nolintlint: reports false positive warnings about used nolint directive

[fho]

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

We have staticcheck and nonolint enabled.
Sometimes in our CI builds golangci-lint reports a false warnings from nonolint about unused nolint:staticcheck comments.
All warnings that I checked are wrong, the //nolint:staticcheck directives always silence existing deprecation warnings from staticcheck.

I was not able to reproduce this issue.
In CI we reuse the golangci-lint and go cache when running checks on different branches.
It can happen that the same cache was used previously with older go and/or golangci-lint versions.

I believe #1940 (comment) refers to the same issue.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.49.0 built from cc2d97f3 on 2022-08-24T10:24:37Z

Configuration file

$ cat .golangci.yml
run:
  concurrency: 2
  deadline: 5m

linters:
  disable-all: true
  enable:
    - depguard
    - errcheck
    - exportloopref
    - goimports
    - goprintffuncname
    - ineffassign
    - misspell
    - nolintlint
    - revive
    - rowserrcheck
    - staticcheck
    - typecheck
    - unconvert
    - unused
    - vet

issues:
  exclude-use-default: false

Go environment

$ go version && go env
[/src/go/code/orders]$ go version && go env
go version go1.19.1 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN="/usr/local/bin"
GOCACHE="/var/cache/go-build"
GOENV="/tmp/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS="-tags=timetzdata -trimpath"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/var/cache/go-modcache"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/tmp/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="0"
GOMOD="/src/go/go.mod"
GOWORK=""
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 -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1328424340=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

N/A

Code example or link to a public repository

Example case, output from golangci-lint:

[..]/itemdetails.go:36:32: directive `//nolint:staticcheck // Description field is deprecated` is unused for linter "staticcheck" (nolintlint)
		return item.Description, nil //nolint:staticcheck // Description field is deprecated

The referenced Description field has a deprecation comment set:

// Deprecated: Do not use.
Description      string                 `protobuf:"bytes,4,opt,name=description,proto3" json:"description,omitempty"`

[ldez]

Hello,

I change some elements related to facts (they are used by staticcheck) maybe the problem will be fixed with the next release (v1.50.0)

[fho]

The issue happend again with golangci-lint 1.50.0.

[iwata]

Happened with golangci-lint v1.54.2, too.

[EvgenySerg]

Have this issue also when dealing with switch/case:
nolint:exhaustive` is unused for linter "exhaustive" (nolintlint)
v1.55.2

[ldez]

If you are facing the same problem, the best way to contribute is to provide the following information:

golangci-lint --version

cat .golangci.yml

go version && go env

golangci-lint cache clean
golangci-lint run -v

And a code example or link to a public repository.

[rliebz]

Here's a reliable repro:

#!/bin/sh -e

## setup module
rm -rf nolintrepro
mkdir nolintrepro && cd $_

## go.mod
cat > go.mod <<EOF
module example.com/nolintrepro

go 1.21

require github.com/jackc/pgx/v5 v5.5.3

require (
	github.com/jackc/pgpassfile v1.0.0 // indirect
	github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
	golang.org/x/crypto v0.17.0 // indirect
	golang.org/x/text v0.14.0 // indirect
)
EOF

## main.go
cat > main.go <<EOF
package main

import (
	"fmt"

	"github.com/jackc/pgx/v5"
)

func main() {
	var conn *pgx.Conn
	fmt.Println(conn.PgConn().CheckConn() == nil)
	fmt.Println(conn.PgConn().CheckConn() == nil) //nolint: staticcheck
}
EOF

go mod tidy

## .golangci.yml
cat > .golangci.yml <<EOF
linters:
  disable-all: true
  enable:
    - staticcheck
    - nolintlint
EOF

## Run
golangci-lint run

## Clean
cd ..
rm -rf nolintrepro

version and env

$ go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
$ golangci-lint --version
golangci-lint has version v1.56.2 built with go1.22.0 from (unknown, mod sum: "h1:dgQzlWHgNbCqJjuxRJhFEnHDVrrjuTGQHJ3RIZMpp/o=") on (unknown)

$ go version && go env
go version go1.22.0 darwin/arm64
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/me/Library/Caches/go-build'
GOENV='/Users/me/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/me/.local/share/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/me/.local/share/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.0/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.0/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/me/nolintrepro/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/yd/g6bkd_fd7qs40y6q_8w2fh2w0000gn/T/go-build3765820227=/tmp/go-build -gno-record-gcc-switches -fno-common'

$ golangci-lint cache clean
$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /Users/me/nolintrepro /Users/me /Users /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 2 linters: [nolintlint staticcheck] 
INFO [loader] Go packages loading at mode 575 (deps|name|types_sizes|compiled_files|exports_file|files|imports) took 523.977875ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 161µs 
INFO [linters_context/goanalysis] analyzers took 2.663033296s with top 10 stages: buildir: 2.039481668s, fact_deprecated: 150.111285ms, nilness: 126.620753ms, fact_purity: 125.42288ms, SA5012: 115.692744ms, typedness: 103.558214ms, SA4006: 222µs, isgenerated: 130.292µs, SA4027: 99.542µs, SA9004: 88.75µs 
INFO [runner] Issues before processing: 3, after processing: 1 
INFO [runner] Processors filtering stat (out/in): path_shortener: 1/1, cgo: 3/3, filename_unadjuster: 3/3, uniq_by_line: 1/1, nolint: 1/3, severity-rules: 1/1, fixer: 1/1, skip_files: 3/3, autogenerated_exclude: 3/3, exclude: 3/3, max_from_linter: 1/1, skip_dirs: 3/3, identifier_marker: 3/3, diff: 1/1, max_same_issues: 1/1, source_code: 1/1, path_prefixer: 1/1, sort_results: 1/1, path_prettifier: 3/3, exclude-rules: 3/3, max_per_file_from_linter: 1/1 
INFO [runner] processing took 519.497µs with stages: exclude-rules: 189.292µs, nolint: 94.251µs, autogenerated_exclude: 71.416µs, identifier_marker: 69.291µs, source_code: 52.833µs, path_prettifier: 31.583µs, skip_dirs: 4.959µs, uniq_by_line: 1.417µs, cgo: 1.041µs, max_same_issues: 749ns, path_shortener: 458ns, max_from_linter: 417ns, filename_unadjuster: 292ns, sort_results: 292ns, diff: 209ns, fixer: 208ns, max_per_file_from_linter: 208ns, skip_files: 167ns, severity-rules: 166ns, exclude: 166ns, path_prefixer: 82ns 
INFO [runner] linters took 1.05964275s with stages: goanalysis_metalinter: 1.059068917s 
main.go:11:14: SA1019: conn.PgConn().CheckConn is deprecated: CheckConn is deprecated in favor of Ping. CheckConn cannot detect all types of broken connections where the write would still appear to succeed. Prefer Ping unless on a high latency connection. (staticcheck)
	fmt.Println(conn.PgConn().CheckConn() == nil)
	           ^
INFO File cache stats: 1 entries of total size 213B 
INFO Memory: 19 samples, avg is 131.1MB, max is 309.6MB 
INFO Execution took 1.774828083s          

This is the correct behavior—staticcheck is flagging the line without the nolint directive.

But if we modify the file (which I assume affects the cache), we get the incorrect behavior. I added a newline after var conn *pgx.Conn to make a change that should be equivalent from an AST perspective. Then ran golangci-lint run -v again:

INFO [config_reader] Config search paths: [./ /Users/me/nolintrepro /Users/me /Users /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 2 linters: [nolintlint staticcheck] 
INFO [loader] Go packages loading at mode 575 (exports_file|files|imports|types_sizes|compiled_files|deps|name) took 531.612584ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 198.542µs 
INFO [linters_context/goanalysis] analyzers took 8.812332ms with top 10 stages: buildir: 5.229666ms, SA5012: 532.211µs, fact_purity: 447.049µs, nilness: 416.708µs, fact_deprecated: 405.157µs, typedness: 365.661µs, SA4001: 78.208µs, SA1021: 77.834µs, SA6005: 70.25µs, SA4026: 65.584µs 
INFO [runner] Processors filtering stat (out/in): exclude: 1/1, diff: 1/1, source_code: 1/1, fixer: 1/1, cgo: 1/1, filename_unadjuster: 1/1, skip_dirs: 1/1, autogenerated_exclude: 1/1, uniq_by_line: 1/1, severity-rules: 1/1, skip_files: 1/1, exclude-rules: 1/1, nolint: 1/1, max_same_issues: 1/1, max_from_linter: 1/1, path_shortener: 1/1, path_prettifier: 1/1, identifier_marker: 1/1, max_per_file_from_linter: 1/1, path_prefixer: 1/1, sort_results: 1/1 
INFO [runner] processing took 224.418µs with stages: nolint: 57.251µs, autogenerated_exclude: 51.125µs, source_code: 27.209µs, path_prettifier: 25.876µs, identifier_marker: 25.125µs, exclude-rules: 25.042µs, skip_dirs: 7.542µs, uniq_by_line: 1.042µs, max_same_issues: 1.042µs, max_from_linter: 584ns, cgo: 541ns, path_shortener: 459ns, filename_unadjuster: 292ns, fixer: 208ns, skip_files: 208ns, exclude: 208ns, severity-rules: 207ns, sort_results: 166ns, max_per_file_from_linter: 125ns, diff: 125ns, path_prefixer: 41ns 
INFO [runner] linters took 88.876792ms with stages: goanalysis_metalinter: 88.6125ms 
main.go:13:48: directive `//nolint: staticcheck` is unused for linter "staticcheck" (nolintlint)
	fmt.Println(conn.PgConn().CheckConn() == nil) //nolint: staticcheck
	                                             ^
INFO File cache stats: 1 entries of total size 214B 
INFO Memory: 10 samples, avg is 30.4MB, max is 51.4MB 
INFO Execution took 819.347ms                 

Now it complains about the unused nolint directive, and staticcheck doesn't complain at all.

If you run golangci-lint cache clean, the correct behavior returns until the file is modified again.

[ldez]

Thank you for the repro.

As a workaround for staticcheck, I recommend using the following example instead of nolint directives:

issues:
  exclude-rules:
    - linters:
        - staticcheck
      text: "SA1019: conn.PgConn().CheckConn is deprecated: CheckConn is deprecated in favor of Ping"

Side note: a directive should not contain space after the ::

• nolint: xxx not valid
• nolint:xxx valid.

[stevenh]

Seeing this in combination with ireturn too, as mentioned the error goes away for a time if you clean the cache with golangci-lint cache clean.

pkg/mything.go:9:78: directive `//nolint:ireturn // Returns one of multiple types which implement MyThinger.` is unused for linter "ireturn" (nolintlint)
func Testfunc() MyThinger { //nolint:ireturn // Returns one of multiple types which implement MyThinger.

[ldez]

I think this issue is the same as #4423, I will check my fix #4424 on that. EDIT no it's not the same problem.

May be related to #3476

---

@stevenh If you are facing the same problem, the best way to contribute is to provide the following information:

golangci-lint --version

cat .golangci.yml

go version && go env

golangci-lint cache clean
golangci-lint run -v

And a code example or link to a public repository.