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.

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

depguard integration unnecessarily type checks #2670

Closed
4 tasks done
perrydunn opened this issue Mar 21, 2022 · 3 comments · Fixed by #2672
Closed
4 tasks done

depguard integration unnecessarily type checks #2670

perrydunn opened this issue Mar 21, 2022 · 3 comments · Fixed by #2672
Assignees
Labels
bug Something isn't working

Comments

@perrydunn
Copy link

perrydunn commented Mar 21, 2022

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

The depguard golangci-lint integration unnecessarily type checks.

When run standalone depguard does not experience this issue.
When run via golangci-lint this issue is experienced, resulting in typecheck lints (catching the compilation errors).

Example to reproduce: edit test/testdata/depguard.go

 func SpewDebugInfo() {
+       var a A
+       log.Println(A)
        log.Println(gzip.BestCompression)
 }

run go run ./cmd/golangci-lint/ run --no-config --disable-all --enable=depguard ./test/testdata/depguard.go

test/testdata/depguard.go:11:8: undeclared name: `A` (typecheck)
	var a A
	      ^
test/testdata/depguard.go:12:14: undeclared name: `A` (typecheck)
	log.Println(A)
	            ^
exit status 1

Separately you can run depguard directly - to confirm that no errors are produced by depguard itself - by running go run ./cmd/depguard -c ".depguard.json" <path_to>/golangci-lint/test/testdata/depguard.go where .depguard.json is some minimal depguard configuration.

This can be resolved by using LoadModeSyntax and using a modified MakeFakeLoaderProgram that doesn't attempt to access *pass.TypesInfo.

(revive, for example, doesn't experience this issue)

Version of golangci-lint

master
$ golangci-lint --version
golangci-lint has version (devel) built from (unknown, mod sum: "") on (unknown)

Configuration file

N/A (run tests)
$ cat .golangci.yml

Go environment

$ go version && go env
go version go1.17.8 linux/amd64
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/peregrine/.cache/go-build"
GOENV="/home/peregrine/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/peregrine/core3/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/peregrine/core3:/home/peregrine/core3/src/plz-out/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/tm/tools/go/1.17.8/usr/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/tm/tools/go/1.17.8/usr/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.8"
GCCGO="gccgo"
AR="ar"
CC="cc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/peregrine/perrydunn/golangci-lint/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 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1358118786=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /home/peregrine/perrydunn/golangci-lint /home/peregrine/perrydunn /home/peregrine /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 34 linters: [bodyclose deadcode depguard dogsled dupl errcheck exportloopref funlen gochecknoinits goconst gocritic gocyclo gofmt goimports gomnd goprintffuncname gosec gosimple govet ineffassign lll misspell nakedret noctx nolintlint staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck whitespace] 
INFO [loader] Go packages loading at mode 575 (imports|name|compiled_files|exports_file|types_sizes|deps|files) took 627.171864ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 9.054457ms 
INFO [linters context/goanalysis] analyzers took 1m38.383934073s with top 10 stages: gocritic: 24.930130532s, buildir: 20.842760231s, printf: 6.489465218s, ctrlflow: 6.363960598s, nilness: 6.275353763s, fact_deprecated: 6.16640289s, typedness: 5.605870208s, fact_purity: 5.514132124s, SA5012: 4.762908426s, buildssa: 1.180452211s 
INFO [runner/skip dirs] Skipped 3 issues from dir test/testdata_etc/abspath by pattern test/testdata_etc 
INFO [runner/skip dirs] Skipped 18 issues from dir internal/renameio by pattern internal/renameio 
INFO [runner/skip dirs] Skipped 82 issues from dir internal/cache by pattern internal/cache 
INFO [runner/skip dirs] Skipped 3 issues from dir internal/robustio by pattern internal/robustio 
INFO [runner/skip dirs] Skipped 1 issues from dir test/testdata_etc/unused_exported/lib by pattern test/testdata_etc 
INFO [runner] Issues before processing: 478, after processing: 0 
INFO [runner] Processors filtering stat (out/in): filename_unadjuster: 478/478, skip_files: 478/478, autogenerated_exclude: 371/371, cgo: 478/478, path_prettifier: 478/478, exclude: 371/371, nolint: 0/143, skip_dirs: 371/478, identifier_marker: 371/371, exclude-rules: 143/371 
INFO [runner] processing took 24.654164ms with stages: nolint: 9.015158ms, exclude-rules: 7.616803ms, identifier_marker: 3.949303ms, path_prettifier: 1.958529ms, autogenerated_exclude: 1.806398ms, skip_dirs: 263.408µs, cgo: 25.493µs, filename_unadjuster: 15.365µs, max_same_issues: 872ns, diff: 580ns, uniq_by_line: 450ns, source_code: 315ns, skip_files: 285ns, max_from_linter: 228ns, severity-rules: 191ns, max_per_file_from_linter: 180ns, path_shortener: 177ns, exclude: 174ns, sort_results: 170ns, path_prefixer: 85ns 
INFO [runner] linters took 8.893161966s with stages: goanalysis_metalinter: 8.868457234s 
INFO File cache stats: 243 entries of total size 637.9KiB 
INFO Memory: 97 samples, avg is 820.0MB, max is 1623.4MB 
INFO Execution took 9.533104968s

Code example or link to a public repository

//args: -Edepguard
//config_path: testdata/configs/depguard.yml
package testdata

import (
	"compress/gzip" // ERROR "`compress/gzip` is in the blacklist"
	"log"           // ERROR "`log` is in the blacklist: don't use log"
)

func SpewDebugInfo() {
	var a A
	log.Println(A)
	log.Println(gzip.BestCompression)
}
@perrydunn perrydunn added the bug Something isn't working label Mar 21, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Mar 21, 2022

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

@ldez ldez self-assigned this Mar 21, 2022
@ldez
Copy link
Member

ldez commented Mar 21, 2022

Hello,

the use of LoadModeSyntax produces a panic:

[runner] Panic: depguard: package "testdata" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference: goroutine 726 [running]:
runtime/debug.Stack()
	/home/ldez/.gvm/gos/go1.18/src/runtime/debug/stack.go:24 +0x65
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1()
	/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:101 +0x155
panic({0x1362f20, 0x1d41f20})
	/home/ldez/.gvm/gos/go1.18/src/runtime/panic.go:838 +0x207
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.MakeFakeLoaderProgram(...)
	/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/adapters.go:31
github.com/golangci/golangci-lint/pkg/golinters.depGuard.run({0xc0008195c0?, {0xc0002089d0?, 0xc0001ae090?, 0x0?}}, 0xc0002e8270)
	/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/depguard.go:90 +0xd4
github.com/golangci/golangci-lint/pkg/golinters.NewDepguard.func1.1(0x13517e0?)
	/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/depguard.go:41 +0x59
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc0007e2090)
	/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:187 +0x9c4
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
	/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:105 +0x1d
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc000868af0, {0x1499fcf, 0x8}, 0xc00053a748)
	/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:111 +0x4a
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc0002c7a40?)
	/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:104 +0x85
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc0007e2090)
	/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0xb4
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
	/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x1eb
runner] Can't run linter depguard: depguard: depguard: package "testdata" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference

We use an adapter because depguard use the old and deprecated loader package.

@perrydunn
Copy link
Author

@ldez Thanks, your PR makes the change mentioned in the issue description

This can be resolved by using LoadModeSyntax and using a modified MakeFakeLoaderProgram that doesn't attempt to access *pass.TypesInfo.

Awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants