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

Unparam broken with generics since x/tools update to v0.4.0 #3426

Closed
4 tasks done
mgabeler-lee-6rs opened this issue Dec 19, 2022 · 4 comments · Fixed by #3429 or #3452
Closed
4 tasks done

Unparam broken with generics since x/tools update to v0.4.0 #3426

mgabeler-lee-6rs opened this issue Dec 19, 2022 · 4 comments · Fixed by #3429 or #3452
Assignees
Labels
bug Something isn't working dependencies Relates to an upstream dependency

Comments

@mgabeler-lee-6rs
Copy link
Contributor

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 (e.g., gocritic, go vet, etc.). (https://golangci-lint.run/usage/linters/)

Description of the problem

unparam is broken again since updating golang.org/x/tools to v0.4.0 (#3411). It seems that with tools v0.3.0, the generic functions never got passed to this linter. But with v0.4.0, they do, the code unparam has to check the sizes of parameters panics when the parameter uses a type parameter.

Running unparam standalone doesn't hit this because it uses x/tools v0.1.x

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version v1.50.2-0.20221218170326-1926748b44fb built from (unknown, mod sum: "h1:Ufcq4g/u6bCKnDFwedDJwlv3q5a+V11yG0ZwoqSnhpk=") on (unknown)

Configuration file

None, just enable unparam on the command line

Go environment

$ go version && go env
go version go1.19.4 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/mgl/.cache/go-build"
GOENV="/home/mgl/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/mgl/.go/pkg/mod"
GONOPROXY="go.6river.tech/*,github.com/6RiverSystems/*"
GONOSUMDB="go.6river.tech/*,github.com/6RiverSystems/*"
GOOS="linux"
GOPATH="/home/mgl/.go"
GOPRIVATE="go.6river.tech/*,github.com/6RiverSystems/*"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.19"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.19/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.4"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/tmp.oKMDP6sSNO/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 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3052654776=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v --enable unparam 
INFO [config_reader] Config search paths: [./ /tmp/tmp.oKMDP6sSNO /tmp / /home/mgl] 
INFO [lintersdb] Active 8 linters: [errcheck gosimple govet ineffassign staticcheck typecheck unparam unused] 
INFO [loader] Go packages loading at mode 575 (compiled_files|deps|exports_file|files|imports|name|types_sizes) took 28.410091ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 131.981µs 
INFO [linters_context/goanalysis] analyzers took 660.329986ms with top 10 stages: buildir: 520.245685ms, inspect: 33.324979ms, ctrlflow: 22.029599ms, fact_deprecated: 17.627415ms, printf: 17.454851ms, SA5012: 14.74235ms, fact_purity: 10.153983ms, nilness: 7.838822ms, typedness: 6.710241ms, buildssa: 6.457902ms 
ERRO [runner] Panic: unparam: package "example" (isInitialPkg: true, needAnalyzeSource: true): /usr/lib/go-1.19/src/go/types/sizes.go:181: assertion failed: goroutine 804 [running]:
runtime/debug.Stack()
	/usr/lib/go-1.19/src/runtime/debug/stack.go:24 +0x65
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1()
	/home/mgl/.go/pkg/mod/github.com/golangci/golangci-lint@v1.50.2-0.20221218170326-1926748b44fb/pkg/golinters/goanalysis/runner_action.go:102 +0x155
panic({0x150daa0, 0xc0079b6220})
	/usr/lib/go-1.19/src/runtime/panic.go:884 +0x212
go/types.assert(0xf0?)
	/usr/lib/go-1.19/src/go/types/errors.go:27 +0x5b
go/types.(*StdSizes).Sizeof(0xc000045230, {0x195e1f0?, 0xc00196a930})
	/usr/lib/go-1.19/src/go/types/sizes.go:181 +0x32e
mvdan.cc/unparam/check.(*Checker).checkFunc(0xc001601720, 0xc000ed9200, 0xc000e17ae0?)
	/home/mgl/.go/pkg/mod/mvdan.cc/unparam@v0.0.0-20220706161116-678bad134442/check/check.go:598 +0x989
mvdan.cc/unparam/check.(*Checker).Check(0xc001601720)
	/home/mgl/.go/pkg/mod/mvdan.cc/unparam@v0.0.0-20220706161116-678bad134442/check/check.go:378 +0x78b
github.com/golangci/golangci-lint/pkg/golinters.runUnparam(0xc00273e780, 0xc000260ac0)
	/home/mgl/.go/pkg/mod/github.com/golangci/golangci-lint@v1.50.2-0.20221218170326-1926748b44fb/pkg/golinters/unparam.go:75 +0x1dc
github.com/golangci/golangci-lint/pkg/golinters.NewUnparam.func1(0x15771e0?)
	/home/mgl/.go/pkg/mod/github.com/golangci/golangci-lint@v1.50.2-0.20221218170326-1926748b44fb/pkg/golinters/unparam.go:28 +0x33
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc0004c6bd0)
	/home/mgl/.go/pkg/mod/github.com/golangci/golangci-lint@v1.50.2-0.20221218170326-1926748b44fb/pkg/golinters/goanalysis/runner_action.go:188 +0x9df
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
	/home/mgl/.go/pkg/mod/github.com/golangci/golangci-lint@v1.50.2-0.20221218170326-1926748b44fb/pkg/golinters/goanalysis/runner_action.go:106 +0x1d
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc000176d20, {0x16de461, 0x7}, 0xc0017a6f48)
	/home/mgl/.go/pkg/mod/github.com/golangci/golangci-lint@v1.50.2-0.20221218170326-1926748b44fb/pkg/timeutils/stopwatch.go:111 +0x4a
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0x1?)
	/home/mgl/.go/pkg/mod/github.com/golangci/golangci-lint@v1.50.2-0.20221218170326-1926748b44fb/pkg/golinters/goanalysis/runner_action.go:105 +0x85
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc0004c6bd0)
	/home/mgl/.go/pkg/mod/github.com/golangci/golangci-lint@v1.50.2-0.20221218170326-1926748b44fb/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0xb4
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
	/home/mgl/.go/pkg/mod/github.com/golangci/golangci-lint@v1.50.2-0.20221218170326-1926748b44fb/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x1eb 
WARN [runner] Can't run linter goanalysis_metalinter: goanalysis_metalinter: unparam: package "example" (isInitialPkg: true, needAnalyzeSource: true): /usr/lib/go-1.19/src/go/types/sizes.go:181: assertion failed 
INFO [runner] processing took 11.036µs with stages: source_code: 3.631µs, max_same_issues: 1.402µs, skip_dirs: 849ns, max_from_linter: 642ns, nolint: 591ns, cgo: 455ns, filename_unadjuster: 404ns, autogenerated_exclude: 330ns, skip_files: 325ns, identifier_marker: 324ns, uniq_by_line: 302ns, exclude: 291ns, path_prettifier: 287ns, sort_results: 207ns, path_shortener: 180ns, diff: 166ns, max_per_file_from_linter: 164ns, exclude-rules: 164ns, severity-rules: 161ns, path_prefixer: 161ns 
INFO [runner] linters took 601.417117ms with stages: goanalysis_metalinter: 601.293255ms 
ERRO Running error: 1 error occurred:
	* can't run linter goanalysis_metalinter: goanalysis_metalinter: unparam: package "example" (isInitialPkg: true, needAnalyzeSource: true): /usr/lib/go-1.19/src/go/types/sizes.go:181: assertion failed
 
INFO Memory: 8 samples, avg is 150.7MB, max is 226.3MB 
INFO Execution took 633.937881ms                  

Code example or link to a public repository

Lightly adapted from the real code that is panicking for me. Verbose output above is from running against this example code.
package example

import (
    "context"
    "fmt"
    "time"
)

var RetryDelay = time.Minute

func runWithRetries[T, U any](
	ctx context.Context,
	f func(ctx context.Context, value T) (U, error),
	value T,
) (U, error) {
	for {
		if result, err := f(ctx, value); err == nil {
			return result, nil
		} else {
			fmt.Printf("waiting %v to retry: %v\n", RetryDelay, err)
			delay := time.After(RetryDelay)
			select {
			case <-ctx.Done():
				return result, ctx.Err()
			case <-delay:
				// continue
			}
		}
	}
}
@mgabeler-lee-6rs mgabeler-lee-6rs added the bug Something isn't working label Dec 19, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Dec 19, 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 Dec 19, 2022
@ldez ldez added the dependencies Relates to an upstream dependency label Dec 19, 2022
@ldez
Copy link
Member

ldez commented Dec 19, 2022

Hello,

have you tried to update golang.org/x/tools inside mvdan.cc/unparam?

@mgabeler-lee-6rs
Copy link
Contributor Author

mgabeler-lee-6rs commented Dec 19, 2022

Yep, that just makes it panic in the same way.

Edit: Submitted mvdan/unparam#70 with a fix.

@mgabeler-lee-6rs
Copy link
Contributor Author

mvdan/unparam#70 is merged, so if golangci-lint updates its unparam dep, this should be fixed

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

Successfully merging a pull request may close this issue.

2 participants