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

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

Closed
7 tasks done
m3rolam opened this issue Aug 22, 2024 · 9 comments
Closed
7 tasks done

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

m3rolam opened this issue Aug 22, 2024 · 9 comments
Assignees
Labels
question Further information is requested

Comments

@m3rolam
Copy link

m3rolam commented Aug 22, 2024

Welcome

  • Yes, I'm using a binary release within 2 latest releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've read the typecheck section of the FAQ.
  • Yes, I've tried with the standalone linter if available (e.g., gocritic, go vet, etc.).
  • I agree to follow this project's Code of Conduct

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/[email protected]/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/[email protected]/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/[email protected]/newrelic/sql_driver.go:55:46: predeclared any requires go1.18 or later (typecheck)
func sendSecureEventSQLPrepareArgs(args, obj any) any {
                                             ^

Version of golangci-lint

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

Configuration

# 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

$ go version && go env
go version go1.22.6 darwin/arm64
GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/<user>/Library/Caches/go-build'
GOENV='/Users/<user>/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/<user>/go/pkg/mod'
GONOPROXY='github.com/<org>'
GONOSUMDB='github.com/<org>'
GOOS='darwin'
GOPATH='/Users/<user>/go'
GOPRIVATE='github.com/<org>'
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/<user>/projects/<project>/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

$ 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/<user>/projects/<project> /Users/<user>/projects /Users/<user> /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

package main

import "github.com/newrelic/go-agent/v3/newrelic"

func main() {
	_, _ = newrelic.NewApplication(
		newrelic.ConfigAppName("test"),
		newrelic.ConfigLicense("test"),
		newrelic.ConfigAppLogForwardingEnabled(true),
	)
}
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

  • Yes, I've included all information above (version, config, etc.).

Supporter

@m3rolam m3rolam added the bug Something isn't working label Aug 22, 2024
Copy link

boring-cyborg bot commented Aug 22, 2024

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 Aug 22, 2024
@m3rolam
Copy link
Author

m3rolam commented Aug 22, 2024

FWIW: I believe the removal of SetAnalyzerGoVersion in this PR was the cause. #4907. However, I'm unfamiliar with the project enough to make a meaningful contribution at this time.

@ldez
Copy link
Member

ldez commented Aug 22, 2024

@ldez
Copy link
Member

ldez commented Aug 22, 2024

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
Copy link
Author

m3rolam commented Aug 22, 2024

Ah, that makes sense. Thanks for clarifying!

@ldez
Copy link
Member

ldez commented Aug 22, 2024

The build tags have been removed inside v3.34.0 because it was a problem to compile newrelic/go-agent#940

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

I will close this issue.

@ldez ldez closed this as completed Aug 22, 2024
@ldez ldez added question Further information is requested and removed bug Something isn't working labels Aug 22, 2024
@m3rolam
Copy link
Author

m3rolam commented Aug 22, 2024

@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
Copy link
Member

ldez commented Aug 22, 2024

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

This comment was marked as off-topic.

@golangci golangci locked as resolved and limited conversation to collaborators Sep 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants