Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

staticcheck: rare (flaky) U1000 false positive #726

Closed
cespare opened this issue Apr 13, 2020 · 5 comments
Closed

staticcheck: rare (flaky) U1000 false positive #726

cespare opened this issue Apr 13, 2020 · 5 comments

Comments

@cespare
Copy link

cespare commented Apr 13, 2020

(Discussed on IRC but writing it down to have something to refer to.)

We're using staticcheck 2020.1.2 with Go 1.13.3 in our CI environment and we very occasionally see a U1000 false positive on PRs which don't even touch the flagged code.

The symptom is very much like #642, but that bug was clearly fixed by a9480a3.

What we see is U1000 triggering for a bunch of methods, consts, and funcs defined in a particular package P. All of these names are unexported and are not used within the package proper, but are used in the tests. In fact, there's a U1000 warning for every unexported name in P which is used only in the test. (That is, the unused warnings I get are the same as if I rm *_test.go in the package and run staticcheck.)

I only have two examples of this failure but both times staticcheck failed only on package P, and not any other packages. (We run staticcheck over all our Go code at once using staticcheck root/....)

Unfortunately, I'm not having luck reproducing this flake. The approach that worked for #642 hasn't panned out (I've been running many instances of staticcheck in parallel over various combinations of packages, with and without a new cache for each staticcheck, with and without the race detector, etc.) I've thrown a few CPU-years at it but no failures yet, so there's probably a missing ingredient that I'm not incorporating (such as having partially stale cached results from a previous run over different code).

In theory this could be #671, but I doubt it. We would have to be doing something like deleting all the tests on a different branch to get these kinds of errors into the cache. It seems more likely that there's another bug where the package-with-tests variant is accidentally being associated with the errors for the package-without-tests.

If I keep seeing the flake I might disable the cache to see if it makes it go away.

@cespare cespare added false-positive needs-triage Newly filed issue that needs triage labels Apr 13, 2020
@dominikh dominikh removed the needs-triage Newly filed issue that needs triage label Apr 13, 2020
@cespare
Copy link
Author

cespare commented Apr 15, 2020

It turns out this happens a lot more often than I thought. I have around 100 examples from the past month, and as recently as a few hours ago.

It mostly happens for the one package I mentioned above, but I found some examples of it happening for some other packages too.

@cespare
Copy link
Author

cespare commented Apr 15, 2020

I'm disabling the staticcheck cache to see if that makes it go away. Should narrow things down.

@cespare
Copy link
Author

cespare commented Apr 17, 2020

So far no flakes since disabling the staticcheck cache, so it seems likely that doing so suppresses the bug.

@cespare
Copy link
Author

cespare commented Apr 20, 2020

We had multiple flakes per day before disabling the staticcheck cache and none afterwards, so I'm going to declare the workaround successful and just leave it disabled for now. If I can figure out the bug (or if the check is rewritten) we can re-enable the cache later.

@cespare
Copy link
Author

cespare commented Mar 30, 2021

For the record, I'm pretty sure this was fixed by 5cfc85b. We upgraded to a newer version of staticcheck with the new runner, re-enabled the staticcheck cache, and haven't seen any flakes in the last 6 weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants