Skip to content
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.

Sign up for GitHub

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

Jump to bottom

structcheck returns false positives, for variables defined in different file, in the same package #537

Closed
moolibdensplk opened this issue May 14, 2019 · 7 comments · May be fixed by golangci/check#1
Closed

structcheck returns false positives, for variables defined in different file, in the same package #537

moolibdensplk opened this issue May 14, 2019 · 7 comments · May be fixed by golangci/check#1
Labels
false positive An error is reported when one does not exist stale No recent correspondence or work activity

Comments

@moolibdensplk
Copy link

moolibdensplk commented May 14, 2019 •
edited
Loading

Thank you for creating the issue!

Please include the following information:

  1. Version of golangci-lint: golangci-lint --version (or git commit if you don't use binary distribution)

commit: 692dacb (master branch).

  1. Config file: cat .golangci.yml

no config, running manually using:
golangci run --disable-all -E structcheck

  1. Go environment: go version && go env
$ go version
go version go1.12.4 darwin/amd64

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/REDACTED/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/REDACTED/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.4/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="0"
GOMOD="/Users/REDACTED/cloud/REDACTED/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/yq/bwmkdmt135q2565hzm6qy57c0000gn/T/go-build639278964=/tmp/go-build -gno-record-gcc-switches -fno-common"
  1. Verbose output of running: golangci-lint run -v
$ golangci-lint run -v --disable-all -E structcheck
INFO [config_reader] Config search paths: [./ /Users/REDACTED/cloud/REDACTED /Users/REDACTED/cloud /Users/REDACTED /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 1 linters: [structcheck]
INFO [loader] Go packages loading at mode load types and syntax took 1.36938593s
INFO [loader] SSA repr building timing: packages building 20.752897ms, total 439.769658ms
INFO [runner] worker.8 took 2.787µs
INFO [runner] worker.12 took 2.049µs
INFO [runner] worker.3 took 685ns
INFO [runner] worker.5 took 1.272µs
INFO [runner] worker.1 took 1.935µs
INFO [runner] worker.4 took 782ns
INFO [runner] worker.2 took 1.328µs
INFO [runner] worker.9 took 1.218µs
INFO [runner] worker.6 took 1.885µs
INFO [runner] worker.11 took 1.732µs
INFO [runner] worker.10 took 2.2µs
INFO [runner] worker.7 took 2.913397ms with stages: structcheck: 2.90531ms
INFO [runner] Workers idle times: #1: 2.812024ms, #2: 2.799165ms, #3: 2.83959ms, #4: 2.794031ms, #5: 2.821053ms, #6: 2.768656ms, #8: 2.874194ms, #9: 2.78048ms, #10: 2.733417ms, #11: 2.73826ms, #12: 2.849321ms
INFO [runner] processing took 897.568µs with stages: identifier_marker: 233.772µs, skip_dirs: 232.073µs, exclude: 187.111µs, source_code: 95.93µs, autogenerated_exclude: 53.575µs, cgo: 23.62µs, nolint: 21.673µs, path_prettifier: 12.031µs, max_same_issues: 11.942µs, uniq_by_line: 7.61µs, filename_unadjuster: 4.097µs, max_from_linter: 4.093µs, max_per_file_from_linter: 3.4µs, replacement_builder: 3.133µs, path_shortener: 2.899µs, diff: 254ns, skip_files: 191ns, exclude-rules: 164ns

[...]
internal/definitions.go:154:2: `lfTf` is unused (structcheck)
	lfTf              *lockfile.Lockfile 
	^
internal/definitions.go:156:2: `lpId` is unused (structcheck)
	lpId              string
	^
[....]
INFO File cache stats: 1 entries of total size 6.5KiB
INFO Memory: 21 samples, avg is 309.9MB, max is 675.7MB
INFO Execution took 1.923498231s

The problem happens when the given variables / types are defined in one file, and used in another in the same package.

I did a quick test to asses whether these were really unused, and here is the logic:
a. I took one of these, reported as unused:

internal/definitions.go:154:2: `lfTf` is unused (structcheck)
	lfTf              *lockfile.Lockfile 
	^

b. commented it out in the file where it is defined
c. ran the linter again:
As expected, that returned errors about lfTf being undeclared:

$ golangci-lint run -E structcheck
internal/module1.go:376:20: mod1cfg.lfTf undefined (type SomeType has no field or method lfTf) (typecheck)
	unlockFile(mod1cfg.lfTf)
	                  ^
internal/module2go:852:20: mod2cfg.lfTf undefined (type SomeType has no field or method lfTf) (typecheck)
	unlockFile(mod2cfg.lfTf)
	                  ^
internal/module3.go:228:20: mod3cfg.lfTf undefined (type  SomeType no field or method lfTf) (typecheck)
	unlockFile(blahcfg.lfTf)

I was able to recreate the issue for all of the objects reported by first check.
I noticed that megacheck used to have a similar problem and that you guys have fixed it previously:
#148

Would you mind looking at structcheck as well ?
Cause at the moment, I had to disable this linter in our pipeline, as it turns out all errors coming from it, happen to be false positive in our scenario (where we have some structure definitions in one file, that are then used in different files within the same package).

Kind regerads
moolibdensplk
@moolibdensplk moolibdensplk changed the title structcheck seems to return false positives for unused structcheck returns false positives, for variables defined in different file, in the same package May 20, 2019
@arthurkiller
Copy link

same with me

@tpounds tpounds added the false positive An error is reported when one does not exist label Sep 25, 2019
@parvathi-nair
Copy link

Would there be a fix soon? In My case, it is not a different file, but the variables are used from two different structs like this . A { name string } B{ A, age int}, C{A , color string}. Both B and C use A which uses name. But the golangci says name is not used.

@0xch4z
Copy link

0xch4z commented Feb 14, 2020

I've found this to happen when using files with periods in the name with as recent as v1.23.6

I changed a file add.test.go to add_test.go and the package-level variables were resolved.

@h4yfans
Copy link

h4yfans commented Dec 22, 2020

Still same with me

@jingyugao jingyugao mentioned this issue Feb 7, 2021
Open
@mittachaitu mittachaitu mentioned this issue May 17, 2021
Merged
7 tasks
@stale
Copy link

stale bot commented Jan 8, 2022

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.

@stale stale bot added the stale No recent correspondence or work activity label Jan 8, 2022
@olix0r
Copy link

olix0r commented Feb 14, 2022

This issue still appears to be present in the golangci-lint v1.44.0. Should, perhaps, structcheck be removed from the default set of enabled linters?

@stale stale bot removed the stale No recent correspondence or work activity label Feb 14, 2022
@ldez
Copy link
Member

ldez commented Feb 14, 2022

@olix0r take a look at #1841

@ldez ldez added the stale No recent correspondence or work activity label Feb 14, 2022
@stale stale bot closed this as completed Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
false positive An error is reported when one does not exist stale No recent correspondence or work activity
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix structcheck returns false positives for select field from embedded type jingyugao/check
8 participants
@tpounds @olix0r @ldez @parvathi-nair @arthurkiller @h4yfans @0xch4z @moolibdensplk