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

staticcheck lint:ignore directives not honored #2671

Closed
4 tasks done
scop opened this issue Mar 21, 2022 · 3 comments
Closed
4 tasks done

staticcheck lint:ignore directives not honored #2671

scop opened this issue Mar 21, 2022 · 3 comments
Labels
area: nolint Related to nolint directive and nolintlint duplicate This issue or pull request already exists

Comments

@scop
Copy link
Contributor

scop 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

As noted in #1658 (comment), staticcheck's lint:ignore directives are not honored within golangci-lint.

At least some other linters' directives do work, including gosec's //#nosec and revive's //revive:disable:....

Would be great to have this work for staticcheck, too, as linter specific directives provide means for more fine grained filtering than //nolint:somelintercompletely.

Haven't dug through if this is something that would need to be fixed in staticcheck or golangci-lint, but starting by reporting it here :)

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version v1.45.0 built from (unknown, mod sum: "h1:T2oCVkYoeckBxcNS6DTYiSXN2QcTNuAWaHyLGfqzMlU=") on (unknown)

Configuration file

$ cat .golangci.yml
cat: .golangci.yml: No such file or directory

Go environment

$ go version && go env
go version go1.17.8 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/scop/.cache/go-build"
GOENV="/home/scop/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/scop/go/pkg/mod"
GONOPROXY="*.example.com"
GONOSUMDB="*.example.com"
GOOS="linux"
GOPATH="/home/scop/go"
GOPRIVATE="*.example.com"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/scop/.goenv/versions/1.17.8"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/scop/.goenv/versions/1.17.8/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.8"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build1470473724=/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: [./ /tmp/ihi /tmp / /home/scop] 
INFO [lintersdb] Active 10 linters: [deadcode errcheck gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck] 
INFO [loader] Go packages loading at mode 575 (types_sizes|exports_file|files|imports|compiled_files|deps|name) took 85.083961ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 73.188µs 
INFO [linters context/goanalysis] analyzers took 2.943121ms with top 10 stages: buildir: 430.099µs, fact_deprecated: 310.708µs, printf: 300.551µs, ctrlflow: 173.958µs, fact_purity: 123.318µs, typedness: 120.53µs, nilness: 115.63µs, SA5012: 111.277µs, directives: 49.762µs, unused: 47.842µs 
INFO [runner] Processors filtering stat (out/in): path_prettifier: 1/1, nolint: 1/1, max_same_issues: 1/1, severity-rules: 1/1, sort_results: 1/1, autogenerated_exclude: 1/1, identifier_marker: 1/1, exclude: 1/1, max_per_file_from_linter: 1/1, source_code: 1/1, cgo: 1/1, skip_files: 1/1, skip_dirs: 1/1, uniq_by_line: 1/1, max_from_linter: 1/1, path_shortener: 1/1, path_prefixer: 1/1, filename_unadjuster: 1/1, exclude-rules: 1/1, diff: 1/1 
INFO [runner] processing took 395.069µs with stages: nolint: 96.518µs, identifier_marker: 67.793µs, autogenerated_exclude: 58.307µs, source_code: 50.394µs, path_prettifier: 43.23µs, exclude-rules: 34.573µs, skip_dirs: 18.936µs, uniq_by_line: 6.94µs, max_same_issues: 4.373µs, max_from_linter: 2.896µs, path_shortener: 2.618µs, cgo: 2.283µs, filename_unadjuster: 1.598µs, max_per_file_from_linter: 1.371µs, skip_files: 692ns, sort_results: 667ns, severity-rules: 518ns, exclude: 515ns, diff: 494ns, path_prefixer: 353ns 
INFO [runner] linters took 14.997923ms with stages: goanalysis_metalinter: 14.371884ms 
main.go:5:2: SA9003: empty branch (staticcheck)
	if "foo" != "" {
	^
INFO File cache stats: 1 entries of total size 100B 
INFO Memory: 3 samples, avg is 44.3MB, max is 44.5MB 
INFO Execution took 130.39769ms                   

For comparison staticcheck ./... is silent (the lint:ignore took place).

Code example or link to a public repository

package main

func main() {
	//lint:ignore SA9003 for demonstration purposes
	if "foo" != "" {
	}
}
@scop scop 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 added enhancement New feature or improvement and removed bug Something isn't working labels Mar 21, 2022
@ldez ldez added the area: nolint Related to nolint directive and nolintlint label Mar 21, 2024
@owenwaller
Copy link

Has there been any progress on this issue?

I've just run into this issue myself, when staticcheck started failing on the use of ast.Package with Go 1.23.

ast.Package was deprecated in Go 1.23, but as it is part of the Go standard library it falls under the Go v1 compatability promise. This makes the failure a false positive, as ast.package will always exist and function, even if new code should not use it going forward.

I tried an //lint:ignore comment, but that didn't l work for the reasons outlined above. So I ended up with a golangci-lint //nolint:staticcheck comment, which works.

But as the ast.Package is passed as a function argument the //nolint:staticcheck disables all of the checks in staticcheck for the function as a whole, which I'd rather not do if I can avoid it.

Supporting the staticcheck lint:ignore directive would solve this.

Thanks

Owen

@ldez
Copy link
Member

ldez commented Aug 26, 2024

duplicate of #1658

@ldez ldez closed this as completed Aug 26, 2024
@ldez ldez added duplicate This issue or pull request already exists and removed enhancement New feature or improvement labels Aug 26, 2024
owenwaller added a commit to owenwaller/vugu that referenced this issue Aug 27, 2024
Using a golangci-lint directive of the form:

//nolint:staticcheck // reason why staticcheck has been disabled

We can disable the use of the staticcheck linter completely at the blocks
or lines where is is currently reporting the use of the deprecated
ast.Package stuct.

This approach is documented here:
https://golangci-lint.run/usage/false-positives/#nolint-directive

At present golangci-lint will ignore staticheck //lint:ignore style
directives. This issues has been raised with the golangci-lint team. See:

golangci/golangci-lint#2671 (comment)
and here:
golangci/golangci-lint#1658 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: nolint Related to nolint directive and nolintlint duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants