golangci / golangci-lint

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

typecheck errors in vendor files for predeclared any requires go1.18 #4942

Closed m3rolam closed 1 month ago

m3rolam commented 1 month ago

Welcome

Description of the problem

After upgrading to golangci-lint v1.60.2, we are receiving errors for predeclared any typecheck errors for vendored third-party modules.

Example error:

../../go/pkg/mod/github.com/newrelic/go-agent/v3@v3.32.0/newrelic/sql_driver.go:47:37: predeclared any requires go1.18 or later (typecheck)
func sendSecureEventSQL(query, args any) any {
                                    ^
../../go/pkg/mod/github.com/newrelic/go-agent/v3@v3.32.0/newrelic/sql_driver.go:51:43: predeclared any requires go1.18 or later (typecheck)
func sendSecureEventSQLPrepare(query, obj any) {
                                          ^
../../go/pkg/mod/github.com/newrelic/go-agent/v3@v3.32.0/newrelic/sql_driver.go:55:46: predeclared any requires go1.18 or later (typecheck)
func sendSecureEventSQLPrepareArgs(args, obj any) any {
                                             ^

Version of golangci-lint

```console $ golangci-lint --version golangci-lint has version 1.60.2 built with go1.23.0 from f338f3e on 2024-08-20T20:01:24Z ```

Configuration

```console # Configuration file for golangci-lint: https://golangci-lint.run/usage/configuration/ # Install with `brew install golangci-lint` run: # Timeout for analysis, e.g. 30s, 5m. # Default: 1m timeout: 3m linters: disable-all: true enable: # Default Linters - errcheck # checking for unchecked errors, these unchecked errors can be critical bugs in some cases - gosimple # specializes in simplifying a code - govet # reports suspicious constructs, such as Printf calls whose arguments do not align with the format string - ineffassign # detects when assignments to existing variables are not used - staticcheck # is a go vet on steroids, applying a ton of static analysis checks - typecheck # like the front-end of a Go compiler, parses and type-checks Go code - unused # checks for unused constants, variables, functions and types # Optional Linters #- asasalint # checks for pass []any as any in variadic func(...any) #- asciicheck # checks that your code does not contain non-ASCII identifiers #- bidichk # checks for dangerous unicode character sequences #- bodyclose # checks whether HTTP response body is closed successfully #- cyclop # checks function and package cyclomatic complexity #- dupl # tool for code clone detection #- durationcheck # checks for two durations multiplied together #- errname # checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error #- errorlint # finds code that will cause problems with the error wrapping scheme introduced in Go 1.13 #- execinquery # checks query string in Query function which reads your Go src files and warning it finds #- exhaustive # checks exhaustiveness of enum switch statements #- exportloopref # checks for pointers to enclosing loop variables #- forbidigo # forbids identifiers #- funlen # tool for detection of long functions #- gocheckcompilerdirectives # validates go compiler directive comments (//go:) #- gochecknoglobals # checks that no global variables exist #- gochecknoinits # checks that no init functions are present in Go code #- gocognit # computes and checks the cognitive complexity of functions #- goconst # finds repeated strings that could be replaced by a constant #- gocritic # provides diagnostics that check for bugs, performance and style issues #- gocyclo # computes and checks the cyclomatic complexity of functions #- godot # checks if comments end in a period #- goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt #- gomnd # detects magic numbers #- gomoddirectives # manages the use of 'replace', 'retract', and 'excludes' directives in go.mod #- gomodguard # allow and block lists linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations #- goprintffuncname # checks that printf-like functions are named with f at the end #- gosec # inspects source code for security problems #- lll # reports long lines #- loggercheck # checks key value pairs for common logger libraries (kitlog,klog,logr,zap) #- makezero # finds slice declarations with non-zero initial length #- musttag # enforces field tags in (un)marshaled structs #- nakedret # finds naked returns in functions greater than a specified function length #- nestif # reports deeply nested if statements #- nilerr # finds the code that returns nil even if it checks that the error is not nil #- nilnil # checks that there is no simultaneous return of nil error and an invalid value #- noctx # finds sending http request without context.Context #- nolintlint # reports ill-formed or insufficient nolint directives #- nonamedreturns # reports all named returns #- nosprintfhostport # checks for misuse of Sprintf to construct a host with port in a URL #- predeclared # finds code that shadows one of Go's predeclared identifiers #- promlinter # checks Prometheus metrics naming via promlint #- reassign # checks that package variables are not reassigned #- revive # fast, configurable, extensible, flexible, and beautiful linter for Go, drop-in replacement of golint #- rowserrcheck # checks whether Err of rows is checked successfully #- sqlclosecheck # checks that sql.Rows and sql.Stmt are closed #- stylecheck # is a replacement for golint #- tenv # detects using os.Setenv instead of t.Setenv since Go1.17 #- testableexamples # checks if examples are testable (have an expected output) #- testpackage # makes you use a separate _test package #- tparallel # detects inappropriate usage of t.Parallel() method in your Go test codes #- unconvert # removes unnecessary type conversions #- unparam # reports unused function parameters #- usestdlibvars # detects the possibility to use variables/constants from the Go standard library #- wastedassign # finds wasted assignment statements #- whitespace # detects leading and trailing whitespace #- decorder # checks declaration order and count of types, constants, variables and functions #- exhaustruct # [highly recommend to enable] checks if all structure fields are initialized #- gci # controls golang package import order and makes it always deterministic #- ginkgolinter # [if you use ginkgo/gomega] enforces standards of using ginkgo and gomega #- godox # detects FIXME, TODO and other comment keywords #- goheader # checks is file header matches to pattern #- interfacebloat # checks the number of methods inside an interface #- ireturn # accept interfaces, return concrete types #- prealloc # [premature optimization, but can be used in some cases] finds slice declarations that could potentially be preallocated #- varnamelen # [great idea, but too many false positives] checks that the length of a variable's name matches its scope #- wrapcheck # checks that errors returned from external packages are wrapped #- containedctx # detects struct contained context.Context field #- contextcheck # [too many false positives] checks the function whether use a non-inherited context #- depguard # [replaced by gomodguard] checks if package imports are in a list of acceptable packages #- dogsled # checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) #- dupword # [useless without config] checks for duplicate words in the source code #- errchkjson # [don't see profit + I'm against of omitting errors like in the first example https://github.com/breml/errchkjson] checks types passed to the json encoding functions. Reports unsupported types and optionally reports occasions, where the check for the returned error can be omitted #- forcetypeassert # [replaced by errcheck] finds forced type assertions #- goerr113 # [too strict] checks the errors handling expressions #- gofmt # [replaced by goimports] checks whether code was gofmt-ed #- gofumpt # [replaced by goimports, gofumports is not available yet] checks whether code was gofumpt-ed #- grouper # analyzes expression groups #- importas # enforces consistent import aliases #- maintidx # measures the maintainability index of each function #- misspell # [useless] finds commonly misspelled English words in comments #- nlreturn # [too strict and mostly code is not more readable] checks for a new line before return and branch statements to increase code clarity #- paralleltest # [too many false positives] detects missing usage of t.Parallel() method in your Go test #- tagliatelle # checks the struct tags #- thelper # detects golang test helpers without t.Helper() call and checks the consistency of test helpers #- wsl # [too strict and mostly code is not more readable] whitespace linter forces you to use empty lines #- deadcode # [deprecated, replaced by unused] finds unused code #- exhaustivestruct # [deprecated, replaced by exhaustruct] checks if all struct's fields are initialized #- golint # [deprecated, replaced by revive] golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes #- ifshort # [deprecated] checks that your code uses short syntax for if-statements whenever possible #- interfacer # [deprecated] suggests narrower interface types #- maligned # [deprecated, replaced by govet fieldalignment] detects Go structs that would take less memory if their fields were sorted #- nosnakecase # [deprecated, replaced by revive var-naming] detects snake case of variable naming and function name #- scopelint # [deprecated, replaced by exportloopref] checks for unpinned variables in go programs #- structcheck # [deprecated, replaced by unused] finds unused struct fields #- varcheck # [deprecated, replaced by unused] finds unused global variables and constants issues: # Excluding configuration per-path, per-linter, per-text and per-source exclude-rules: - path: _test\.go text: "SA5011: possible nil pointer dereference" - path: _test\.go text: "SA5011\\(related information\\): this check suggests that the pointer can be nil" ```

Go environment

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

Verbose output of running

```console $ golangci-lint cache clean $ golangci-lint run -v INFO golangci-lint has version 1.60.2 built with go1.23.0 from f338f3e on 2024-08-20T20:01:24Z INFO [config_reader] Config search paths: [./ /Users//projects/ /Users//projects /Users/ /Users /] INFO [config_reader] Used config file .golangci.yml INFO [lintersdb] Active 6 linters: [errcheck gosimple govet ineffassign staticcheck unused] INFO [loader] Go packages loading at mode 575 (deps|imports|name|types_sizes|compiled_files|exports_file|files) took 666.462083ms INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 33.818917ms INFO [linters_context/goanalysis] analyzers took 32.545221186s with top 10 stages: buildir: 18.595954443s, inspect: 1.446608119s, fact_deprecated: 1.338733831s, ctrlflow: 1.049334571s, printf: 964.26187ms, unused: 744.802501ms, nilness: 635.918878ms, fact_purity: 593.947696ms, typedness: 412.350572ms, SA5012: 397.634819ms INFO [runner] Issues before processing: 707, after processing: 3 INFO [runner] Processors filtering stat (in/out): source_code: 3/3, severity-rules: 3/3, path_prefixer: 3/3, path_prettifier: 660/660, exclude: 660/660, exclude-rules: 660/660, uniq_by_line: 660/3, max_same_issues: 3/3, sort_results: 3/3, identifier_marker: 660/660, path_shortener: 3/3, fixer: 3/3, filename_unadjuster: 707/707, invalid_issue: 707/660, autogenerated_exclude: 660/660, diff: 3/3, max_per_file_from_linter: 3/3, cgo: 707/707, skip_files: 660/660, skip_dirs: 660/660, nolint: 660/660, max_from_linter: 3/3 INFO [runner] processing took 4.599003ms with stages: identifier_marker: 4.280376ms, path_prettifier: 201µs, source_code: 40.375µs, cgo: 11.542µs, exclude-rules: 10.625µs, nolint: 10.416µs, filename_unadjuster: 9.292µs, uniq_by_line: 9µs, invalid_issue: 8.125µs, skip_dirs: 7.458µs, autogenerated_exclude: 7.209µs, path_shortener: 832ns, max_same_issues: 793ns, max_from_linter: 584ns, max_per_file_from_linter: 376ns, exclude: 209ns, diff: 208ns, sort_results: 207ns, fixer: 167ns, skip_files: 125ns, severity-rules: 84ns, path_prefixer: 0s INFO [runner] linters took 4.647466542s with stages: goanalysis_metalinter: 4.642819166s vendor/github.com/newrelic/go-agent/v3/newrelic/sql_driver.go:47:37: predeclared any requires go1.18 or later (typecheck) func sendSecureEventSQL(query, args any) any { ^ vendor/github.com/newrelic/go-agent/v3/newrelic/sql_driver.go:51:43: predeclared any requires go1.18 or later (typecheck) func sendSecureEventSQLPrepare(query, obj any) { ^ vendor/github.com/newrelic/go-agent/v3/newrelic/sql_driver.go:55:46: predeclared any requires go1.18 or later (typecheck) func sendSecureEventSQLPrepareArgs(args, obj any) any { ^ INFO File cache stats: 1 entries of total size 10.3KiB INFO Memory: 55 samples, avg is 562.5MB, max is 924.4MB INFO Execution took 5.353382958s ```

A minimal reproducible example or link to a public repository

```go package main import "github.com/newrelic/go-agent/v3/newrelic" func main() { _, _ = newrelic.NewApplication( newrelic.ConfigAppName("test"), newrelic.ConfigLicense("test"), newrelic.ConfigAppLogForwardingEnabled(true), ) } ``` ```go module playground go 1.23.0 require github.com/newrelic/go-agent/v3 v3.32.0 require ( github.com/golang/protobuf v1.5.3 // indirect golang.org/x/net v0.9.0 // indirect golang.org/x/sys v0.7.0 // indirect golang.org/x/text v0.9.0 // indirect google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect google.golang.org/grpc v1.56.3 // indirect google.golang.org/protobuf v1.30.0 // indirect ) ```

Validation

Supporter

boring-cyborg[bot] commented 1 month ago

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

m3rolam commented 1 month ago

FWIW: I believe the removal of SetAnalyzerGoVersion in this PR was the cause. https://github.com/golangci/golangci-lint/pull/4907. However, I'm unfamiliar with the project enough to make a meaningful contribution at this time.

ldez commented 1 month ago

The cause is https://github.com/newrelic/go-agent/blob/v3.32.0/v3/newrelic/sql_driver.go#L4-L5

ldez commented 1 month ago

I believe the removal of SetAnalyzerGoVersion in this PR was the cause.

Not really, the problem is that Go is now more strict with Go version used on files.

So the problem is the build tags inside newrelic lib.

m3rolam commented 1 month ago

Ah, that makes sense. Thanks for clarifying!

ldez commented 1 month ago

The build tags have been removed inside v3.34.0 because it was a problem to compile https://github.com/newrelic/go-agent/pull/940

https://github.com/newrelic/go-agent/blob/v3.34.0/v3/newrelic/sql_driver.go

I will close this issue.

m3rolam commented 1 month ago

@ldez I understand that it's been removed in newer versions, but what I'm confused about is why the linter would pop positive on 3rd party dependency issues for typecheck errors. That seems like a unwanted behavior.

ldez commented 1 month ago

golangci-lint and linters rely on Go tooling, if a linter or Go changes the way to handle something, we have no choice.

The change was required for staticcheck but it's not his fault, the change was just here to follow Go new version handling.

The typecheck errors reported are not related to a specific linter but to how Go handles versions: https://github.com/golang/go/blob/d2879efd0227df32d6aeee1be58c325b477f22d4/src/go/types/api.go#L122-L127

So yes, it's unwanted, but there are no other solutions.

skillcoder commented 1 week ago

Any way to suppress this errors? I don't want to update infra libraries right now. Standard way with disable linter or add an exclude-rules seems not working in this case. I've try to remove typecheck from linters.enable And also try to add

issues:
  exclude-rules:
    - source: "vendor/github.com/newrelic/go-agent/v3/newrelic/sql_driver.go"
      text: "typecheck"

With no luck.