unused: lint unused exported members in the `./internal` directory

[qdm12]

Since Go code in the ./internal cannot be imported by other projects, it would be nice to lint project-locally-unused Go types/variables/constants located in the ./internal directory.

[dominikh]

In terms of implementation difficulty, this is identical to detecting any kind of unused exported object. The proposed idea is a filter to only flag those that are in internal packages, not all checked packages.

There's no concrete timeline for supporting finding unused exported objects, although the work on it has started.

[qdm12]

In terms of implementation difficulty, this is identical to detecting any kind of unused exported object.

I suppose this is due to the checks crossing packages, whereas for unexported objects it's only package local right?

Would you think now would be a good time to start PR-ing for this feature, or do you have already opened PRs / local work waiting on your end?

Thanks!

[dominikh]

I suppose this is due to the checks crossing packages, whereas for unexported objects it's only package local right?

Crossing them in the wrong direction, at that. Normal analyzes in Staticcheck (and the go/analysis framework) propagate information from dependencies to dependents. But figuring out which objects are used needs information to flow in the opposite direction.

But that's not the only difficulty. There are also a lot of edge cases involving interfaces and reflection. A method might only be used dynamically, for example.

Would you think now would be a good time to start PR-ing for this feature, or do you have already opened PRs / local work waiting on your end?

There's some existing work already commited/pushed but not exposed, because it is incomplete. Generally speaking, however, I would advise against sending PRs. I usually take even longer to review PRs than I take implementing things myself :grimacing: This task in particular also requires a lot of intimate knowledge of how unused works, and unfortunately most of that is only recorded in my mind.

The issue tracker has a fair amount of prior discussions on the matter of whole-program unusedness.

[qdm12]

Thank you for taking the time explaining all this 👍

I usually take even longer to review PRs than I take implementing things myself

Oh I feel you 😄 The ugly side of pull requests I guess 😸

The issue tracker has a fair amount of prior discussions on the matter of whole-program unusedness.

Feel free to close mine if you feel it's duplicating another issue or could be bundled in another issue.

But that's not the only difficulty. There are also a lot of edge cases involving interfaces and reflection. A method might only be used dynamically, for example.

Wouldn't it be worth it to have an imperfect linter and use for example golangci-lint's //nolint:unused on such niche cases? 🤔

[dominikh]

Feel free to close mine if you feel it's duplicating another issue or could be bundled in another issue.

I'm leaving this issue open because it documents an interesting UI middle-ground between flagging all or no exported objects.

Wouldn't it be worth it to have an imperfect linter and use for example golangci-lint's //nolint:unused on such niche cases?

The guiding principle for Staticcheck is to err on the side of false negatives, not false positives. This is especially true for a check as impactful as this one, which tells you to delete your code and where it might not be obvious that the suggestion is wrong — removing code required for dynamic calls will cause failures at run time, not at compile time.

[alingse]

• we need this to delete old codes.