'unused' check should not reason about control flow

[adonovan]

Welcome

• Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
• Yes, I've searched similar issues on GitHub and didn't find any.
• Yes, I've included all information below (version, config, etc).
• Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

Adding a call to (*testing.T).Skip into a test function causes the 'unused' analysis to report various functions as unused, specifically, those whose sole call is made dynamically unreachable by the Skip call:

func Test (*testing.T) {
    t.Skip()
    f() // func `f` is unused (unused)
}
func f() { ... }

While it is indeed clever that the 'unused' analysis knows that Skip never returns and is able to make deductions based on the control-flow graph, it not helpful to the user, who must now find a devious way to work around the check, such as this:

    // This tautological condition is necessary to defeat the too-clever
    // 'unused' golangci-lint analyzer into thinking that f is reachable.
    // See https://github.com/golangci/golangci-lint/issues/2515.
    if true {
        t.Skip()
    }

'unused' should use the static reference graph. It should not care about control flow.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.42.1 built from 54f4301 on 2021-09-04T14:39:09Z

Configuration file

$ cat .golangci.yml
# paste output here

Go environment

$ go version && go env
# paste output here

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
# paste output here

Code example or link to a public repository

// add your code here

[boring-cyborg]

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

[ldez]

Hello,

Thank you for your report, unused is a part of staticcheck, golangci-lint just wraps this linter.

https://github.com/dominikh/go-tools/blob/master/unused/unused.go

So I recommend opening an issue on staticcheck.

ping @dominikh

[dominikh]

This is dominikh/go-tools#633 (and changing how unused works on a broader scale is an open todo item of mine.)

[ldez]

Thank you @dominikh 👍

I will close this issue as the problem is already declared on staticcheck.

[adonovan]

Thanks. Perhaps the bug report template could remind ill-informed bug reporters such as me to consider reporting the bug upstream for certain components, with a link to details.

[ldez]

Perhaps the bug report template could remind ill-informed bug reporters such as me to consider reporting the bug upstream for certain components, with a link to details.

We already have a checkbox about that 😸 and you checked this box 😸

[x] Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Maybe the sentence is not enough clear.

[adonovan]

We already have a checkbox about that 😸 and you checked this box 😸

Right you are! Sorry for missing that. For some reason I interpreted that to mean: did I run the command-line tool directly against a micro test case (as opposed to e.g. seeing a failure in a CI system's logs)?

Perhaps it could be phrased something like this:

[x] This is a problem in the golangci-lint driver and not one of its linter plugins.

It might be worth a paragraph in the main README file explaining whether this repo hosts original linters, or merely repackages upstream ones in a new driver, or does a mix of both, and link to the two sets (internal, external).