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
U1000: The "unused" linting rule has misleading behaviour when interacting with generated code #1333
Comments
I haven't yet verified that behavior, but that would indeed be a bug. If I can see why case 2 might be confusing, but I disagree with the conclusion that
That is not a conclusion that U1000 could make automatically. We don't know whether the unexported function ( |
Thanks for the quick and thoughtful response! Thanks, as well, for all of your work on staticcheck—I find it such a valuable tool.
The special treatment
My instinct would be to have a piece of configuration that toggles between options 1 and 2, since both seem reasonable in different contexts. In the private example that triggered this investigation, we have control of the code generation, so option 2 would have (correctly) flagged an issue in the code generator. If we were at the mercy of uncontrollable code generation, then I think option 1 might feel a bit better. Those are just feelings, though. You'll have a much better sense than I have of what would be best for staticcheck as a tool.
Yes, quite right—if there's a whole subgraph that's unused (as in this case), it's quite right of staticcheck to report not just any leaf nodes but the whole set of nodes in the subgraph. It absolutely should do this: if I'll edit the OP to strike that through. Thanks for setting me straight! |
We've added the exception when generated code had good reasons for having unused functions (or rather objects in general). For example,
We have a flag,
I think this is the only valid option. Internally, U1000 builds a graph of which objects use which; some objects are roots (exported functions, for example). The current way of ignoring unused objects in generated code incorrectly assumes that generated code is self-contained, and simply suppresses the output of a diagnostic; it doesn't affect the shape of the graph. This is why
That feels like an overly specific solution to me, and I hope that it shouldn't be needed once we improve how ignoring in generated code works. |
staticcheck has, in my judgement, some misleading behaviour when it interacts with generated code.
Under some conditions, it produces an error message that doesn't suffice to resolve the underlying problem. (This was first reported here, which led me to here.)
Reproduction
The following example was produced with staticcheck 2022.1.3 (v0.3.3).
The "details" block at the bottom of this section describes the following directory structure (which I've also attached as a tarball here):
First, recreate this directory structure and set
rc.sh
to be executable.If you run
./rc.sh
, then staticcheck will report that the functionfoo
is unused.If you remove the first line from bar.go (that says that the file is generated) and run
./rc.sh
, then staticcheck will report that bothfoo
andbar
are unused.If you replace
bar
withBar
, then, irrespective of the presence of the "first line" in bar.go, no errors are reported.In my view, the messages in 1 and 2 are misleading. In both cases, the issue is thatbar
isn't public (and therefore isn't used);foo
is only unused as a consequence ofbar
not being used. This is especially a problem in 1, because there's no way to resolve the problem from the error message alone.(Ed.: I've changed my mind about 2, so I've struck this through and rewritten it. See the comments below.)
In my view, the message in option 1 is misleading; there's no way to resolve the problem from the error message alone.
The text was updated successfully, but these errors were encountered: