go-critic icon indicating copy to clipboard operation
go-critic copied to clipboard

`unparam` linter cannot handle parameters with generic placeholders

Open abdusco opened this issue 2 years ago • 3 comments

When working with funcs with parameters that accept type parameters, unparam linter panics.

Minimal example to replicate the issue:

package main

type typeMapper[T any, U any] func(v T) (U, error)

func thisIsOK[T comparable, U comparable](something T, mapperFunc func(v T) (U, error)) {}

func thisPanics[T comparable, U comparable](something T, mapperFunc typeMapper[T, U]) {}

save this as generics.go and run

golangci-lint run --enable unparam generics.go

if you comment out thisPanics, linter passes.

This is the stack trace I get:

> golangci-lint run --config ./.golangci.yml generics.go
ERRO [runner] Panic: gocritic: package "main" (isInitialPkg: true, needAnalyzeSource: true): unhandled expr: goroutine 591 [running]:
runtime/debug.Stack()
        /opt/homebrew/Cellar/go/1.18/libexec/src/runtime/debug/stack.go:24 +0x68
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1()
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:101 +0x104
panic({0x104da9b60, 0x104f37a58})
        /opt/homebrew/Cellar/go/1.18/libexec/src/runtime/panic.go:838 +0x204
github.com/go-toolsmith/astcopy.copyExpr({0x104f46518?, 0x140023fe880?})
        /Users/user/go/pkg/mod/github.com/go-toolsmith/[email protected]/astcopy.go:889 +0x730
github.com/go-toolsmith/astcopy.Field(0x140023fe900)
        /Users/user/go/pkg/mod/github.com/go-toolsmith/[email protected]/astcopy.go:314 +0x1f0
github.com/go-toolsmith/astcopy.FieldList(0x140024c4810)
        /Users/user/go/pkg/mod/github.com/go-toolsmith/[email protected]/astcopy.go:331 +0x154
github.com/go-toolsmith/astcopy.FuncType(0x1400007d0a0)
        /Users/user/go/pkg/mod/github.com/go-toolsmith/[email protected]/astcopy.go:344 +0xac
github.com/go-critic/go-critic/checkers.(*paramTypeCombineChecker).optimizeFuncType(0x14000607758?, 0x1400007d0a0)
        /Users/user/go/pkg/mod/github.com/go-critic/[email protected]/checkers/paramTypeCombine_checker.go:42 +0x2c
github.com/go-critic/go-critic/checkers.(*paramTypeCombineChecker).VisitFuncDecl(0x14000e8f748?, 0x140024c4870)
        /Users/user/go/pkg/mod/github.com/go-critic/[email protected]/checkers/paramTypeCombine_checker.go:35 +0x30
github.com/go-critic/go-critic/checkers/internal/astwalk.(*funcDeclWalker).WalkFile(0x14000a7acf0, 0x140000d2200)
        /Users/user/go/pkg/mod/github.com/go-critic/[email protected]/checkers/internal/astwalk/func_decl_walker.go:21 +0xf0
github.com/go-critic/go-critic/framework/linter.(*Checker).Check(...)
        /Users/user/go/pkg/mod/github.com/go-critic/[email protected]/framework/linter/linter.go:130
github.com/golangci/golangci-lint/pkg/golinters.runGocriticOnFile(0x140022d22a0, 0x140008e81d3?, {0x140006d7400, 0x2b, 0x104df6fe0?})
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/gocritic.go:178 +0xa4
github.com/golangci/golangci-lint/pkg/golinters.runGocriticOnPackage(0x140022d22a0, {0x140006d7400, 0x2b, 0x40}, {0x14002680440, 0x1, 0x140005a3508?})
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/gocritic.go:166 +0x15c
github.com/golangci/golangci-lint/pkg/golinters.(*goCriticWrapper).run(0x1400024c7c0, 0x14000478340)
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/gocritic.go:126 +0x1cc
github.com/golangci/golangci-lint/pkg/golinters.NewGoCritic.func1(0x104deee80?)
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/gocritic.go:49 +0x3c
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0x14000384360)
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:187 +0x96c
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:105 +0x24
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0x1400025fb80, {0x104abdfd7, 0x8}, 0x140004e1730)
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/timeutils/stopwatch.go:111 +0x48
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0x0?)
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:104 +0x78
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0x14000384360)
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0xb4
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
        /Users/user/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x16c 

My env:

> golangci-lint version
golangci-lint has version v1.49.0 built from (unknown, mod sum: "h1:I8WHOavragDttlLHtSraHn/h39C+R60bEQ5NoGcHQr8=") on (unknown)

> go version
go version go1.18 darwin/arm64


github.com/quasilyte/go-ruleguard/cmd/[email protected]

abdusco avatar Sep 14 '22 11:09 abdusco

This will be fixed in the next release that includes astcopy v1.0.2.

quasilyte avatar Sep 16 '22 21:09 quasilyte

Actually, I think it should be included in gocritic v0.6.5. It looks like golangci-lint version you're using is based on gocritic v0.6.4.

quasilyte avatar Sep 16 '22 21:09 quasilyte

I'm using https://github.com/golangci/golangci-lint/releases/tag/v1.49.0 atm, which installs gocritic v0.6.4, so I think I'll have to wait until that gets updated.

Does it make sense to move/duplicate this issue under https://github.com/golangci/golangci-lint repo?

abdusco avatar Sep 19 '22 09:09 abdusco

I got the same error with the newest version of gocritic with the newest version of golangci-lint Issue link - https://github.com/golangci/golangci-lint/issues/3357

golangci-lint version: golangci-lint has version 1.50.1 built from 8926a95 on 2022-10-22T10:48:48Z gocritic version: [email protected]

Command

gocritic check -enableAll ./...

Output

panic: assertion failed [recovered]
        panic: assertion failed

goroutine 1582 [running]:
github.com/go-critic/go-critic/framework/lintmain/internal/check.(*program).checkFile.func1.1()
        /Users/nipunapathirana/go/pkg/mod/github.com/go-critic/[email protected]/framework/lintmain/internal/check/check.go:160 +0x11e
panic({0x43fefe0, 0x4522d10})
        /usr/local/go/src/runtime/panic.go:838 +0x207
go/types.assert(...)
        /usr/local/go/src/go/types/errors.go:21
go/types.(*StdSizes).Alignof(0xc00023c210, {0x4525b30?, 0xc001068930})
        /usr/local/go/src/go/types/sizes.go:72 +0x25f
go/types.(*StdSizes).Offsetsof(0x4525a18?, {0xc0006160d0, 0x1, 0x0?})
        /usr/local/go/src/go/types/sizes.go:101 +0xdd
go/types.(*StdSizes).Sizeof(0xc00023c210, {0x4525a18?, 0xc00024a070})
        /usr/local/go/src/go/types/sizes.go:154 +0x1c5
github.com/go-critic/go-critic/framework/linter.(*CheckerContext).SizeOf(...)
        /Users/nipunapathirana/go/pkg/mod/github.com/go-critic/[email protected]/framework/linter/linter.go:325
github.com/go-critic/go-critic/checkers.(*hugeParamChecker).checkParams(0xc00103c348, {0xc000616048, 0x1, 0x0?})
        /Users/nipunapathirana/go/pkg/mod/github.com/go-critic/[email protected]/checkers/hugeParam_checker.go:56 +0x107
github.com/go-critic/go-critic/checkers.(*hugeParamChecker).VisitFuncDecl(0x0?, 0xc001068360)
        /Users/nipunapathirana/go/pkg/mod/github.com/go-critic/[email protected]/checkers/hugeParam_checker.go:44 +0x38
github.com/go-critic/go-critic/checkers/internal/astwalk.(*funcDeclWalker).WalkFile(0xc00026bcc0, 0xc000b84000)
        /Users/nipunapathirana/go/pkg/mod/github.com/go-critic/[email protected]/checkers/internal/astwalk/func_decl_walker.go:21 +0xda
github.com/go-critic/go-critic/framework/linter.(*Checker).Check(...)
        /Users/nipunapathirana/go/pkg/mod/github.com/go-critic/[email protected]/framework/linter/linter.go:130
github.com/go-critic/go-critic/framework/lintmain/internal/check.(*program).checkFile.func1(0x2b, 0xc0002f4500)
        /Users/nipunapathirana/go/pkg/mod/github.com/go-critic/[email protected]/framework/lintmain/internal/check/check.go:164 +0xc2
created by github.com/go-critic/go-critic/framework/lintmain/internal/check.(*program).checkFile
        /Users/nipunapathirana/go/pkg/mod/github.com/go-critic/[email protected]/framework/lintmain/internal/check/check.go:146 +0xa5

senpathi avatar Nov 11 '22 18:11 senpathi

/Users/nipunapathirana/go/pkg/mod/github.com/go-critic/[email protected]/framework/linter/linter.go:325 github.com/go-critic/go-critic/checkers.(*hugeParamChecker).checkParams(0xc00103c348, {0xc000616048, 0x1, 0x0?})

It seems more a problem of hugeParam Problem disappeared when disabling hugeParam

Begi avatar Jan 25 '23 21:01 Begi

Fixed in https://github.com/go-critic/go-critic/pull/1309

cristaloleg avatar Feb 10 '23 18:02 cristaloleg