Deprecate varcheck, structcheck, and deadcode

[ldez]

Is your feature request related to a problem?

Currently, by default golangci-lint runs the following linters about unused/dead code:

• deadcode
• structcheck
• unused (staticcheck)
• varcheck

Those linters, activated by default, are all the linters related to unused/dead code detection.

https://golangci-lint.run/usage/linters/

I created a small sandbox to find the scope of each of these linters.

linter	var	const	function	struct	struct field	method
deadcode			:heavy_check_mark: (1)	:heavy_check_mark: (2)
structcheck					:heavy_check_mark:
varcheck	:heavy_check_mark:	:heavy_check_mark:
unused	:heavy_check_mark:	:heavy_check_mark:	:heavy_check_mark:	:heavy_check_mark:	:heavy_check_mark:	:heavy_check_mark:

• (1) False-negative on function
• (2) See unused exported struct as unused

deadcode

$ golangci-lint run --no-config --disable-all -Edeadcode --print-issued-lines=false
sub/sub.go:11:7: `unexportedConstUnusedLocally` is unused (deadcode)
sub/sub.go:16:5: `unexportedGlobalUnused` is unused (deadcode)
main.go:29:6: `fn6` is unused (deadcode)
main.go:31:6: `unusedStruct` is unused (deadcode)
main.go:41:6: `ExposedUnusedStruct` is unused (deadcode)

structcheck

$ golangci-lint run --no-config --disable-all -Evarcheck --print-issued-lines=false
sub/sub.go:11:7: `unexportedConstUnusedLocally` is unused (varcheck)
sub/sub.go:16:5: `unexportedGlobalUnused` is unused (varcheck)

varcheck

$ golangci-lint run --no-config --disable-all -Estructcheck --print-issued-lines=false
sub/sub.go:19:2: `unexportedField` is unused (structcheck)
sub/sub.go:24:2: `unexportedField` is unused (structcheck)
main.go:32:2: `unexportedField` is unused (structcheck)
main.go:37:2: `unusedField` is unused (structcheck)

unused

$ golangci-lint run --no-config --disable-all -Eunused --print-issued-lines=false
sub/sub.go:19:2: field `unexportedField` is unused (unused)
sub/sub.go:24:2: field `unexportedField` is unused (unused)
sub/sub.go:16:5: var `unexportedGlobalUnused` is unused (unused)
sub/sub.go:11:7: const `unexportedConstUnusedLocally` is unused (unused)
main.go:51:34: func `unusedStructWithMethods.unexportedMethod` is unused (unused)
main.go:55:34: func `unusedStructWithMethods.ExportedMethod` is unused (unused)
main.go:25:6: func `fn4` is unused (unused)
main.go:27:6: func `fn5` is unused (unused)
main.go:42:2: field `unexportedField` is unused (unused)
main.go:64:32: func `usedStructWithMethods.unexportedMethod` is unused (unused)
main.go:37:2: field `unusedField` is unused (unused)
main.go:46:6: type `unusedStructWithMethods` is unused (unused)
main.go:29:6: func `fn6` is unused (unused)
main.go:31:6: type `unusedStruct` is unused (unused)

Describe the solution you'd like

• deprecate varcheck, structcheck, and deadcode.
• remove varcheck, structcheck, and deadcode from the default linters

Describe alternatives you've considered

• deprecate varcheck, structcheck, and deadcode.

or

• remove varcheck, structcheck, and deadcode from the default linters

Additional context

varcheck and structcheck

varcheck and structcheck come from https://github.com/opennota/check.

https://github.com/opennota/check has moved to https://gitlab.com/opennota/check.

And currently, we use a fork https://github.com/golangci/check

The 2 linters are based on the old golang.org/x/tools/go/loader

deadcode

deadcode is also a fork https://github.com/golangci/go-misc

It is based on the old golang.org/x/tools/go/loader

[sayboras]

This is great, I found a lot of similarity/redundancy between deedcode and un-used personally, sometimes it's with gosimple as well. Moving proposed linters out of default list can cause a little bit unpleasant experience for users (I am guessing we will never go to v2). Just curious about the benefit of doing this, the only thing i can think of is to reduce duplicated effort to skip in case of false positive error.

Can you explain a little bit more about deprecate part? While there is overlapping in functionalities with unused, but these linters are quite popular in community, will you plan to remove them completely in golangci-lint after some transition periods (e.g. maybe 2 releases) ?

This actually reminds me of one idea I had in mind long time back. It's about the minimum requirements of new linters in golangci-lint. Currently, there is no requirement for new one (e.g. supporting suggested, extra info, auto fix, etc), and some might be pretty small in scope, which is already available or can be added easily in one of other existing linter. This is not directly related to this github issue though.

[butuzov]

lol. I did litteraly said that on the article review yesterday, and here is the results from last week (made the same comparison).

checking	unused	deadcode	structcheck	varcheck	unparam
variable	x	x		x
func	x	x
method	x
const		x		x
type	x	x*
struct fields			x
interface	x	x
parameter					x

deadcode only was able to find grouped unused constant, while unused was able to find only ungrouped constants that are unused + unused is having better messaging.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tetienne]

Is unused already included within staticcheck where it comes from? (see your comment :) https://github.com/golangci/golangci-lint/discussions/1989#discussioncomment-745004)

[ldez]

no,staticcheck is a binary and a set of rules, those rules are grouped in "category": staticcheck, gosimple, stylecheck and unused. Inside golangci-lint we use the sets of rules, not the binary.

[tetienne]

That's pretty confusing toi have a category named as the binary.

Thx for the clarification, I thought declaring staticcheck within my configuration wrapped the 4 categories. That's was not obvious when you discover golangci

[hakankoklu]

Do we have any updates on this? Seems like folks are keeping unused and removing others in their configuration files. I just did a basic check in a repo and unused was able to find an unused struct field similar to structcheck. I would like to remove structcheck from our codebases due to it not being compatible with generics https://github.com/golangci/golangci-lint/issues/2649

[xerebz]

Why deprecate varcheck if it detects unused const while unused doesn't?

[pierrre]

Why deprecate varcheck if it detects unused const while unused doesn't?

I'm sorry, but this is incorrect. Try this code:

const foo = "bar"

const `foo` is unused (unused)

unused does detect unused constants.

I'm using golangci-lint v1.49.0

[ldez]

varcheck and structcheck have not changed for 3 years. https://github.com/opennota/check / https://gitlab.com/opennota/check They are based on an API deprecated for 3 years.

So the project is abandoned.