search

golangci / golangci-lint

Fast linters runner for Go
https://golangci-lint.run
GNU General Public License v3.0
15.65k stars 1.39k forks source link

Spooky season false positive on `unused` for an unexported method bound to a struct with generics 🎃 #4176

Closed qfornaguera closed 1 year ago

qfornaguera commented 1 year ago

Welcome

Description of the problem

Lint says:

INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 7 linters: [dupl godot gofmt goimports ineffassign revive unused] 
INFO [loader] Go packages loading at mode 575 (exports_file|imports|name|types_sizes|compiled_files|deps|files) took 522.225917ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 2.476166ms 
INFO [linters_context/goanalysis] analyzers took 50.929376ms with top 10 stages: goimports: 14.475333ms, dupl: 10.215ms, gofmt: 9.673833ms, unused: 6.572707ms, the_only_name: 6.44775ms, ineffassign: 1.510001ms, godot: 1.046917ms, isgenerated: 574.292µs, directives: 375.585µs, typecheck: 37.958µs 
INFO [runner] Issues before processing: 3, after processing: 1 
INFO [runner] Processors filtering stat (out/in): max_per_file_from_linter: 1/1, source_code: 1/1, sort_results: 1/1, autogenerated_exclude: 3/3, uniq_by_line: 1/1, path_prettifier: 3/3, max_same_issues: 1/1, max_from_linter: 1/1, path_shortener: 1/1, path_prefixer: 1/1, cgo: 3/3, filename_unadjuster: 3/3, diff: 1/1, exclude-rules: 1/3, nolint: 1/1, identifier_marker: 3/3, exclude: 3/3, severity-rules: 1/1, fixer: 1/1, skip_files: 3/3, skip_dirs: 3/3 
INFO [runner] processing took 410.87µs with stages: nolint: 151.001µs, path_prettifier: 134.25µs, identifier_marker: 48.791µs, autogenerated_exclude: 43.624µs, source_code: 17.666µs, skip_dirs: 5.584µs, exclude-rules: 4.958µs, cgo: 958ns, max_from_linter: 625ns, uniq_by_line: 584ns, max_same_issues: 458ns, path_shortener: 416ns, filename_unadjuster: 376ns, fixer: 291ns, severity-rules: 249ns, max_per_file_from_linter: 249ns, skip_files: 208ns, diff: 167ns, sort_results: 166ns, exclude: 166ns, path_prefixer: 83ns 
INFO [runner] linters took 81.318ms with stages: goanalysis_metalinter: 80.866541ms 
apps/iam/services/authentication/internal/domain/account/account_builder.go:101:32: func `(*accBuilder[Builder]).buildAccount` is unused (unused)
func (ab *accBuilder[Builder]) buildAccount() (*account, error) {
                               ^
INFO File cache stats: 1 entries of total size 3.9KiB 
INFO Memory: 8 samples, avg is 30.5MB, max is 56.5MB 
INFO Execution took 615.027125ms

But it is actually used...

Screenshot 2023-11-02 at 09 56 03

As a hint. Exporting the method fixes lint issues 👇 

func (ab *accBuilder[Builder]) BuildAccount() (*account, error) {
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 7 linters: [dupl godot gofmt goimports ineffassign revive unused] 
INFO [loader] Go packages loading at mode 575 (exports_file|imports|types_sizes|deps|files|name|compiled_files) took 570.225417ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 2.74ms 
INFO [linters_context/goanalysis] analyzers took 47.400667ms with top 10 stages: goimports: 14.303ms, dupl: 9.145083ms, gofmt: 8.440916ms, the_only_name: 6.938918ms, unused: 5.102542ms, ineffassign: 1.611625ms, godot: 844.708µs, isgenerated: 620.75µs, directives: 366.292µs, typecheck: 26.833µs 
INFO [runner] Issues before processing: 2, after processing: 0 
INFO [runner] Processors filtering stat (out/in): cgo: 2/2, filename_unadjuster: 2/2, identifier_marker: 2/2, skip_dirs: 2/2, skip_files: 2/2, exclude-rules: 0/2, path_prettifier: 2/2, autogenerated_exclude: 2/2, exclude: 2/2 
INFO [runner] processing took 223.96µs with stages: path_prettifier: 131.541µs, identifier_marker: 46.167µs, autogenerated_exclude: 32.042µs, skip_dirs: 8.166µs, exclude-rules: 1.583µs, cgo: 917ns, max_same_issues: 459ns, filename_unadjuster: 458ns, fixer: 417ns, severity-rules: 334ns, nolint: 333ns, exclude: 251ns, skip_files: 249ns, uniq_by_line: 208ns, max_from_linter: 167ns, max_per_file_from_linter: 166ns, source_code: 126ns, path_shortener: 125ns, path_prefixer: 84ns, sort_results: 84ns, diff: 83ns 
INFO [runner] linters took 80.364916ms with stages: goanalysis_metalinter: 80.102959ms 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 8 samples, avg is 32.4MB, max is 56.3MB 
INFO Execution took 666.799833ms

Further details...

the struct implements this unexported interface

type accountBuilder[Builder any] interface {

(งツ)ว

Not in a rush because I can go along exporting the method. But just to let you know it is happening 👀

Version of golangci-lint

```console golangci-lint has version 1.55.1 built with go1.21.3 from 9b20d49d on 2023-10-25T10:03:43Z ```

Configuration

```console # options for analysis running run: # timeout for analysis, e.g. 30s, 5m, default is 1m timeout: 10m # which dirs to skip: they won't be analyzed; # can use regexp here: generated.*, regexp is applied on full path; # default value is empty list, but next dirs are always skipped independently # from this option's value: # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ skip-dirs: - protos - service/legacy/stateprovider - third_party - game/slots/internal/infrastructure/spinner/go-client-generated # include test files or not, default is true tests: true # all available settings of specific linters linters-settings: # Settings for revive linter. revive: rules: - name: exported errcheck: # report about not checking of errors in type assetions: `a := b.(MyStruct)`; # default is false: such cases aren't reported by default. check-type-assertions: true # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`; # default is false: such cases aren't reported by default. check-blank: true # path to a file containing a list of functions to exclude from checking # see https://github.com/kisielk/errcheck#excluding-functions for details exclude: .errcheck govet: # report about shadowed variables check-shadowing: true # settings per analyzer settings: printf: # analyzer name, run `go tool vet help` to see all analyzers funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf golint: # minimal confidence for issues, default is 0.8 min-confidence: 0.8 gofmt: # simplify code: gofmt with `-s` option, true by default simplify: true gocyclo: # minimal code complexity to report, 30 by default (but we recommend 10-20) min-complexity: 20 # TODO: Lower this maligned: # print struct with more effective memory layout or not, false by default suggest-new: true dupl: # tokens count to trigger issue, 150 by default threshold: 200 goconst: # minimal length of string constant, 3 by default min-len: 3 # minimal occurrences count to trigger, 3 by default min-occurrences: 3 depguard: list-type: blacklist include-go-root: false packages: - github.com/davecgh/go-spew/spew misspell: # Correct spellings using locale preferences for US or UK. # Default is to use a neutral variety of English. # Setting locale to US will correct the British spelling of 'colour' to 'color'. locale: US ignore-words: - someword lll: # max line length, lines longer will be reported. Default is 120. # '\t' is counted as 1 character by default, and can be changed with the tab-width option line-length: 120 # tab width in spaces. Default to 1. tab-width: 1 unused: # treat code as a program (not a library) and report unused exported identifiers; default is false. # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: # if it's called for subdir of a project it can't find funcs usages. All text editor integrations # with golangci-lint call it on a directory with the changed file. check-exported: false unparam: # Inspect exported functions, default is false. Set to true if no external program/library imports your code. # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors: # if it's called for subdir of a project it can't find external interfaces. All text editor integrations # with golangci-lint call it on a directory with the changed file. check-exported: false nakedret: # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 max-func-lines: 30 prealloc: # XXX: we don't recommend using this linter before doing performance profiling. # For most programs usage of prealloc will be a premature optimization. # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them. # True by default. simple: true range-loops: true # Report preallocation suggestions on range loops, true by default for-loops: false # Report preallocation suggestions on for loops, false by default gocritic: # Which checks should be enabled; can't be combined with 'disabled-checks'; # See https://go-critic.github.io/overview#checks-overview # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run` # By default list of stable checks is used. enabled-checks: # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty disabled-checks: # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint` run to see all tags and checks. # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". enabled-tags: - performance settings: # settings passed to gocritic captLocal: # must be valid enabled check name paramsOnly: true hugeParam: sizeThreshold: 300 rangeValCopy: sizeThreshold: 32 linters: disable-all: true enable: - revive - godot - gofmt - goimports - dupl - ineffassign - unused #- gosimple #- wsl #- gocritic issues: # Independently from option `exclude` we use default exclude patterns, # it can be disabled by this option. To list all # excluded by default patterns execute `golangci-lint run --help`. # Default value for this option is true. exclude-use-default: false # Maximum issues count per one linter. Set to 0 to disable. Default is 50. max-per-linter: 0 # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. max-same-issues: 0 # Show only new issues: if there are unstaged changes or untracked files, # only those changes are analyzed, else only changes in HEAD~ are analyzed. # It's a super-useful option for integration of golangci-lint into existing # large codebase. It's not practical to fix all existing issues at the moment # of integration: much better don't allow issues in new code. # Default is false. new: false # # List of regexps of issue texts to exclude, empty list by default. # # But independently from this option we use default exclude patterns, # # it can be disabled by `exclude-use-default: false`. To list all # # excluded by default patterns execute `golangci-lint run --help` # exclude: # - abcdef # Excluding configuration per-path, per-linter, per-text and per-source exclude-rules: - path: _test\.go linters: - dupl - linters: - staticcheck text: "SA1019:" - linters: - govet text: "copylocks:" include: - EXC0012 # EXC0012 revive: Annoying issue about not having a comment. The rare codebase has such comments - EXC0014 # EXC0014 revive: Annoying issue about not having a comment. The rare codebase has such comments ```

Go environment

```console go version go1.21.3 darwin/arm64 GO111MODULE='on' GOARCH='arm64' GOBIN='' GOCACHE='/Users/quim.fornaguera/Library/Caches/go-build' GOENV='/Users/quim.fornaguera/Library/Application Support/go/env' GOEXE='' GOEXPERIMENT='' GOFLAGS='' GOHOSTARCH='arm64' GOHOSTOS='darwin' GOINSECURE='' GOMODCACHE='/Users/quim.fornaguera/Go/pkg/mod' GONOPROXY='' GONOSUMDB='' GOOS='darwin' GOPATH='/Users/quim.fornaguera/Go' GOPRIVATE='' GOPROXY='https://proxy.golang.org,direct' GOROOT='/usr/local/go' GOSUMDB='sum.golang.org' GOTMPDIR='' GOTOOLCHAIN='auto' GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64' GOVCS='' GOVERSION='go1.21.3' GCCGO='gccgo' AR='ar' CC='clang' CXX='clang++' CGO_ENABLED='1' GOMOD='/Users/quim.fornaguera/Projects/appocalypse/go.mod' GOWORK='' CGO_CFLAGS='-O2 -g' CGO_CPPFLAGS='' CGO_CXXFLAGS='-O2 -g' CGO_FFLAGS='-O2 -g' CGO_LDFLAGS='-O2 -g' PKG_CONFIG='pkg-config' GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/wm/14pqy6212092z8l7g7sqk9hr0000gp/T/go-build4035768897=/tmp/go-build -gno-record-gcc-switches -fno-common' ```

Verbose output of running

Outputs in the description are indeed with -v

A minimal reproducible example or link to a public repository

This is an abstraction of my src code that reproduces the case 👇 ```go type builder[B any] interface { // WithSomething mocks build setting. WithSomething() B build() struct{} } type baseBuilder[B any] struct { builder B } func newBaseBuilder[B any](b B) *baseBuilder[B] { return &baseBuilder[B]{ builder: b, } } func (b *baseBuilder[B]) WithSomething() B { return b.builder } func (b *baseBuilder[B]) build() struct{} { return struct{}{} } // SomeBuilder is a builder for Some. type SomeBuilder interface { builder[SomeBuilder] Build() struct{} } type someBuilder struct { builder[SomeBuilder] } // NewSomeBuilder creates a new SomeBuilder. func NewSomeBuilder() SomeBuilder { b := &someBuilder{} b.builder = newBaseBuilder[SomeBuilder](b) return b } func (s *someBuilder) Build() struct{} { return s.builder.build() } ``` Funny fact! if you remove `WithSomething` from the interface, false positive also disappear ¯\\\_(ツ)\_/¯

Validation

boring-cyborg[bot] commented 1 year ago

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

ldez commented 1 year ago

hello,

unused is a part of staticcheck.

$ staticcheck -version                                
staticcheck 2023.1.6 (v0.4.6)
$ staticcheck ./...                                   
main.go:33:26: func (*baseBuilder[B]).build is unused (U1000)

The problem seems to be described here: https://github.com/dominikh/go-tools/issues/1294

qfornaguera commented 1 year ago

wow such a mistake from my part, sry to bother you. I've kinda read that checkbox paying not much attention and misunderstood it. Great that is already reported. I'll add any useful information in that link. Thanks for the rapid response! Cheers!