Repository: go-critic/go-critic

Issue:
slice out of bounds
Using gocritic via golangci-lint:

WARN [runner] Can't run linter goanalysis_metalinter: goanalysis_metalinter: gocritic: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: slice bounds out of range [2:1] 
ERRO Running error: can't run linter goanalysis_metalinter
goanalysis_metalinter: gocritic: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: slice bounds out of range [2:1] 


Discussion:
@cristaloleg: Oof. Do you have a full stacktrace?@ptman: ```
ERRO [runner] Panic: gocritic: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: slice bounds out of range [2:1]: goroutine 48858 [running]:
runtime/debug.Stack()
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/runtime/debug/stack.go:26 +0x5e
github.com/golangci/golangci-lint/v2/pkg/goanalysis.(*action).analyzeSafe.func1()
	$GOTPATH/pkg/mod/github.com/golangci/golangci-lint/v2@v2.6.0/pkg/goanalysis/runner_action.go:55 +0x25b
panic({0x15d44a0?, 0xc001e9a300?})
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/runtime/panic.go:783 +0x132
github.com/go-critic/go-critic/checkers.(*dupOptionChecker).getVariadicArgs(0x2709720?, 0xc02761b380)
	$GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.0/checkers/dupOption_checker.go:79 +0x14c
github.com/go-critic/go-critic/checkers.(*dupOptionChecker).VisitExpr(0xc0067a6cb0, {0x1bb68b0?, 0xc02761b380?})
	$GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.0/checkers/dupOption_checker.go:42 +0x66
github.com/go-critic/go-critic/checkers/internal/astwalk.(*exprWalker).WalkFile.func1({0x1bb0880?, 0xc02761b380?})
	$GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.0/checkers/internal/astwalk/expr_walker.go:25 +0x5b
go/ast.inspector.Visit(0xc01d4b0e40, {0x1bb0880?, 0xc02761b380?})
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/go/ast/walk.go:361 +0x2b
go/ast.Walk({0x1ba93e0?, 0xc01d4b0e40?}, {0x1bb0880, 0xc02761b380})
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/go/ast/walk.go:34 +0x4c
go/ast.walkList[...](...)
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/go/ast/walk.go:21
go/ast.Walk({0x1ba93e0?, 0xc01d4b0e40?}, {0x1bb0880, 0xc02761b400})
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/go/ast/walk.go:121 +0x26b4
go/ast.walkList[...](...)
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/go/ast/walk.go:21
go/ast.Walk({0x1ba93e0?, 0xc01d4b0e40?}, {0x1bb0b28, 0xc02761b440})
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/go/ast/walk.go:194 +0x1eae
go/ast.walkList[...](...)
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/go/ast/walk.go:21
go/ast.Walk({0x1ba93e0?, 0xc01d4b0e40?}, {0x1bb0740, 0xc0138c1e60})
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/go/ast/walk.go:211 +0x1d6e
go/ast.Walk({0x1ba93e0?, 0xc01d4b0e40?}, {0x1bb0df8, 0xc0138c1e90})
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/go/ast/walk.go:332 +0x6bc
go/ast.Inspect(...)
	/nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/share/go/src/go/ast/walk.go:377
github.com/go-critic/go-critic/checkers/internal/astwalk.(*exprWalker).WalkFile(0xc0067a6cd0, 0xc00c2b06e0)
	$GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.0/checkers/internal/astwalk/expr_walker.go:23 +0x139
github.com/go-critic/go-critic/linter.(*Checker).Check(...)
	$GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.0/linter/linter.go:160
github.com/golangci/golangci-lint/v2/pkg/golinters/gocritic.runOnFile(0xc00c531b20, 0xc00c2b06e0, {0xc03108c908, 0x6a, 0x7f885e0f8f30?})
	$GOPATH/pkg/mod/github.com/golangci/golangci-lint/v2@v2.6.0/pkg/golinters/gocritic/gocritic.go:146 +0x96
github.com/golangci/golangci-lint/v2/pkg/golinters/gocritic.(*goCriticWrapper).run(0xc001142090, 0xc00c531b20)
	$GOPATH/pkg/mod/github.com/golangci/golangci-lint/v2@v2.6.0/pkg/golinters/gocritic/gocritic.go:111 +0x2fe
github.com/golangci/golangci-lint/v2/pkg/golinters/gocritic.New.func1(0x14bb5a0?)
	$GOPATH/pkg/mod/github.com/golangci/golangci-lint/v2@v2.6.0/pkg/golinters/gocritic/gocritic.go:43 +0x1d
github.com/golangci/golangci-lint/v2/pkg/goanalysis.(*action).analyze.func3(...)
	$GOPATH/pkg/mod/github.com/golangci/golangci-lint/v2@v2.6.0/pkg/goanalysis/runner_checker.go:183
github.com/golangci/golangci-lint/v2/pkg/goanalysis.(*action).analyze(0xc0051a5cc0)
	$GOPATH/pkg/mod/github.com/golangci/golangci-lint/v2@v2.6.0/pkg/goanalysis/runner_checker.go:209 +0xb62
github.com/golangci/golangci-lint/v2/pkg/timeutils.(*Stopwatch).TrackStage(0xc00288d1d0, {0x1670a63, 0x8}, 0xc00da53f10)
	$GOPATH/pkg/mod/github.com/golangci/golangci-lint/v2@v2.6.0/pkg/timeutils/stopwatch.go:111 +0x44
github.com/golangci/golangci-lint/v2/pkg/goanalysis.(*action).analyzeSafe(0xc0051a5cc0?)
	$GOPATH/pkg/mod/github.com/golangci/golangci-lint/v2@v2.6.0/pkg/goanalysis/runner_action.go:59 +0x71
github.com/golangci/golangci-lint/v2/pkg/goanalysis.(*loadingPackage).analyze.func2()
	$GOPATH/pkg/mod/github.com/golangci/golangci-lint/v2@v2.6.0/pkg/goanalysis/runner_loadingpackage.go:111 +0x65
golang.org/x/sync/errgroup.(*Group).Go.func1()
	$GOPATH/pkg/mod/golang.org/x/sync@v0.17.0/errgroup/errgroup.go:93 +0x50
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 784
	$GOPATH/pkg/mod/golang.org/x/sync@v0.17.0/errgroup/errgroup.go:78 +0x95 
WARN [runner] Can't run linter goanalysis_metalinter: goanalysis_metalinter: gocritic: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: slice bounds out of range [2:1] 
ERRO Running error: can't run linter goanalysis_metalinter
goanalysis_metalinter: gocritic: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: slice bounds out of range [2:1] 
```@cristaloleg: Hopefully this will be fixed in https://github.com/go-critic/go-critic/releases/tag/v0.14.1 (regular `x/` bump fixes such failures).@ldez: As the panic happens inside golangci-lint, a bump of `golang.org/x/tools` will have no impact: we already use the latest version.

@ptman can you provide a reproducible example?@ptman:     go get -tool github.com/go-critic/go-critic/cmd/go-critic@v0.14.1
    go tool go-critic check -enableAll ./...

still fails:

```
dupOption: error: runtime error: slice bounds out of range [2:1]
panic: runtime error: slice bounds out of range [2:1] [recovered, repanicked]

goroutine 2910 [running]:
main.(*program).checkFile.func1.1()
        $GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.1/cmd/go-critic/check.go:163 +0x19d       
panic({0x893200?, 0xc008b40330?})
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/runtime/panic.go:783 +0x132
github.com/go-critic/go-critic/checkers.(*dupOptionChecker).getVariadicArgs(0xc220a0?, 0xc002e1ac00)
        $GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/dupOption_checker.go:79 +0x14c       
github.com/go-critic/go-critic/checkers.(*dupOptionChecker).VisitExpr(0xc003ca43d0, {0x965cf8?, 0xc002e1ac00?})
        $GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/dupOption_checker.go:42 +0x66       
github.com/go-critic/go-critic/checkers/internal/astwalk.(*exprWalker).WalkFile.func1({0x964238?, 0xc002e1ac00?})
        $GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/internal/astwalk/expr_walker.go:25 +0x5b       
go/ast.inspector.Visit(0xc00975a230, {0x964238?, 0xc002e1ac00?})
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/go/ast/walk.go:361 +0x2b
go/ast.Walk({0x962760?, 0xc00975a230?}, {0x964238, 0xc002e1ac00})
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/go/ast/walk.go:34 +0x4c
go/ast.walkList[...](...)
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/go/ast/walk.go:21
go/ast.Walk({0x962760?, 0xc00975a230?}, {0x964238, 0xc002e1ac80})
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/go/ast/walk.go:121 +0x26b4
go/ast.walkList[...](...)
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/go/ast/walk.go:21
go/ast.Walk({0x962760?, 0xc00975a230?}, {0x9644e0, 0xc002e1ad40})
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/go/ast/walk.go:194 +0x1eae
go/ast.walkList[...](...)
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/go/ast/walk.go:21
go/ast.Walk({0x962760?, 0xc00975a230?}, {0x9640f8, 0xc0020831d0})
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/go/ast/walk.go:211 +0x1d6e
go/ast.Walk({0x962760?, 0xc00975a230?}, {0x9647b0, 0xc002083200})
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/go/ast/walk.go:332 +0x6bc
go/ast.Inspect(...)
        /nix/store/x322dk8vnsfbr9p31175zhqcc9534iql-go-1.25.0/share/go/src/go/ast/walk.go:377
github.com/go-critic/go-critic/checkers/internal/astwalk.(*exprWalker).WalkFile(0xc003ca43e0, 0xc0021101e0)
        $GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/internal/astwalk/expr_walker.go:23 +0x139       
github.com/go-critic/go-critic/linter.(*Checker).Check(...)
        $GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.1/linter/linter.go:160       
main.(*program).checkFile.func1()
        $GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.1/cmd/go-critic/check.go:170 +0xa5       
created by main.(*program).checkFile in goroutine 1
        $GOPATH/pkg/mod/github.com/go-critic/go-critic@v0.14.1/cmd/go-critic/check.go:151 +0xe5
```
@ldez: I don't reproduce the problem:

```console
$ go run ./cmd/go-critic/ check -enableAll ./...
$
$ go install github.com/go-critic/go-critic/cmd/go-critic@v0.14.1
$ go-critic check -enableAll ./...
$
```
@ldez: can you provide a reproducible example?@ptman: I have no idea which part of our private codebase triggers it. 0.13.0 worked. 0.14.0 doesn't.@ptman: Ah, dupOption is new in 0.14.0, so that's clearly the cause. https://github.com/go-critic/go-critic/blob/5182ddd4c454f34f0fdf991887bbd7ae16223bbc/checkers/dupOption_checker.go#L79@ptman: @alingse @alexandear @ldez: Obviously, this is the cause, but we need to understand why.

Without a reproducible example, it will take time to find the right fix.
@alingse: the `last := sign.Params().Len() - 1` is 2 but the call.Args length is 1, it might be call a options func without options.@ldez: I already tried without options, and there is no panic.@alingse: try this code, the compile is not completed, so the real call.Args's length is 1

```go
package main

type Config struct {}
type Option func(*Config)

func NewService(name string, op Option, options ...Option) *Config {
    return nil
}

func main() {
    // This call will trigger the go-critic panic
    NewService("worker")
}
```
and
```
go-critic check -enableAll ./...
```

and it cause panic

```
dupOption: error: runtime error: slice bounds out of range [2:1]
panic: runtime error: slice bounds out of range [2:1] [recovered]
        panic: runtime error: slice bounds out of range [2:1]

goroutine 2158 [running]:
main.(*program).checkFile.func1.1()
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/cmd/go-critic/check.go:163 +0x19d
panic({0x8874c0?, 0xc000d86018?})
        /usr/local/go/src/runtime/panic.go:787 +0x132
github.com/go-critic/go-critic/checkers.(*dupOptionChecker).getVariadicArgs(0xbfdf10?, 0xc000f60c40)
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/dupOption_checker.go:79 +0x150
github.com/go-critic/go-critic/checkers.(*dupOptionChecker).VisitExpr(0xc0003dc880, {0x955560?, 0xc000f60c40?})
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/dupOption_checker.go:42 +0x66
github.com/go-critic/go-critic/checkers/internal/astwalk.(*exprWalker).WalkFile.func1({0x952c38?, 0xc000f60c40?})
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/internal/astwalk/expr_walker.go:25 +0x5b
go/ast.inspector.Visit(0xc001994160, {0x952c38?, 0xc000f60c40?})
        /usr/local/go/src/go/ast/walk.go:361 +0x2b
go/ast.Walk({0x951240?, 0xc001994160?}, {0x952c38, 0xc000f60c40})
        /usr/local/go/src/go/ast/walk.go:34 +0x4c
go/ast.Walk({0x951240?, 0xc001994160?}, {0x952e68, 0xc0014e36f0})
        /usr/local/go/src/go/ast/walk.go:183 +0x3016
go/ast.walkList[...](...)
        /usr/local/go/src/go/ast/walk.go:21
go/ast.Walk({0x951240?, 0xc001994160?}, {0x952af8, 0xc00074bda0})
        /usr/local/go/src/go/ast/walk.go:211 +0x23dd
go/ast.Walk({0x951240?, 0xc001994160?}, {0x9531b0, 0xc00074be00})
        /usr/local/go/src/go/ast/walk.go:332 +0x59c
go/ast.Inspect(...)
        /usr/local/go/src/go/ast/walk.go:372
github.com/go-critic/go-critic/checkers/internal/astwalk.(*exprWalker).WalkFile(0xc0003dc890, 0xc0006fc640)
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/internal/astwalk/expr_walker.go:23 +0x139
github.com/go-critic/go-critic/linter.(*Checker).Check(...)
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/linter/linter.go:160
main.(*program).checkFile.func1()
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/cmd/go-critic/check.go:170 +0xa5
created by main.(*program).checkFile in goroutine 1
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/cmd/go-critic/check.go:151 +0xe5
```
@ldez: yes but this code doesn't compile.@cristaloleg: ```
    linttest.go:105: ~/go-critic/checkers/testdata/dupOption/positive_tests.go:149:21: not enough arguments in call to NewService
```

(can be ran with `make test-checker dupOption`)@alingse: maybe the tool cause panic first before it report compile problem

```
dupOption: error: runtime error: slice bounds out of range [2:1]
panic: runtime error: slice bounds out of range [2:1] [recovered]
        panic: runtime error: slice bounds out of range [2:1]

goroutine 2158 [running]:
main.(*program).checkFile.func1.1()
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/cmd/go-critic/check.go:163 +0x19d
panic({0x8874c0?, 0xc000d86018?})
        /usr/local/go/src/runtime/panic.go:787 +0x132
github.com/go-critic/go-critic/checkers.(*dupOptionChecker).getVariadicArgs(0xbfdf10?, 0xc000f60c40)
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/dupOption_checker.go:79 +0x150
github.com/go-critic/go-critic/checkers.(*dupOptionChecker).VisitExpr(0xc0003dc880, {0x955560?, 0xc000f60c40?})
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/dupOption_checker.go:42 +0x66
github.com/go-critic/go-critic/checkers/internal/astwalk.(*exprWalker).WalkFile.func1({0x952c38?, 0xc000f60c40?})
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/internal/astwalk/expr_walker.go:25 +0x5b
go/ast.inspector.Visit(0xc001994160, {0x952c38?, 0xc000f60c40?})
        /usr/local/go/src/go/ast/walk.go:361 +0x2b
go/ast.Walk({0x951240?, 0xc001994160?}, {0x952c38, 0xc000f60c40})
        /usr/local/go/src/go/ast/walk.go:34 +0x4c
go/ast.Walk({0x951240?, 0xc001994160?}, {0x952e68, 0xc0014e36f0})
        /usr/local/go/src/go/ast/walk.go:183 +0x3016
go/ast.walkList[...](...)
        /usr/local/go/src/go/ast/walk.go:21
go/ast.Walk({0x951240?, 0xc001994160?}, {0x952af8, 0xc00074bda0})
        /usr/local/go/src/go/ast/walk.go:211 +0x23dd
go/ast.Walk({0x951240?, 0xc001994160?}, {0x9531b0, 0xc00074be00})
        /usr/local/go/src/go/ast/walk.go:332 +0x59c
go/ast.Inspect(...)
        /usr/local/go/src/go/ast/walk.go:372
github.com/go-critic/go-critic/checkers/internal/astwalk.(*exprWalker).WalkFile(0xc0003dc890, 0xc0006fc640)
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/checkers/internal/astwalk/expr_walker.go:23 +0x139
github.com/go-critic/go-critic/linter.(*Checker).Check(...)
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/linter/linter.go:160
main.(*program).checkFile.func1()
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/cmd/go-critic/check.go:170 +0xa5
created by main.(*program).checkFile in goroutine 1
        /home/xxxx/go/pkg/mod/github.com/go-critic/go-critic@v0.14.1/cmd/go-critic/check.go:151 +0xe5
```@ptman: our codebase compiles@cristaloleg: @ptman can you bisect failure by packages, please? I assume there is no that many option-like definitions and probably calls.@cristaloleg: @alingse most (if not all) linters assume that the input code is syntactically correct and compilable.

Without this assumption, checking the code’s logic is impossible. So if the code doesn’t compile without errors, expecting meaningful results from a linter is asking too much.@ldez: And inside golangci-lint, the compilation error is detected:

```console
 $ golangci-lint run               
main.go:1: : # github.com/golangci/sandbox
./main.go:11:13: not enough arguments in call to NewService                                                                                                                                                                           
        have (string)                                                                                                                                                                                                                 
        want (string, Option, ...Option) (typecheck)                                                                                                                                                                                  
package main
1 issues:
* typecheck: 1
```
@cristaloleg: @ptman do you've a build tags in your project? There might be a case when project compiles BUT there is also a code under build tag that doesn't but it's still checked by the linters.@ldez: I found how to reproduce 🎉 @ldez: I will open a PR.@ldez: This is tricky: you need to use a function A that returns the parameters of the function B.
@ldez: At the end, the fix is trivial, but finding a reproducible example was complex.

The PR -> #1516

Solution patch:
diff --git a/.gitignore b/.gitignore
index 850602c..70f8464 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,6 +6,7 @@
 /go-critic
 .idea
 pkg/
+/bin/
 
 # Test binary, build with `go test -c`
 *.test
diff --git a/checkers/dupOption_checker.go b/checkers/dupOption_checker.go
index 2f45d01..5e7eeda 100644
--- a/checkers/dupOption_checker.go
+++ b/checkers/dupOption_checker.go
@@ -75,6 +75,11 @@ func (c *dupOptionChecker) getVariadicArgs(call *ast.CallExpr) ([]ast.Expr, type
 	if !ok {
 		return nil, nil
 	}
+
+	if last > len(call.Args) {
+		return nil, nil
+	}
+
 	argType := sliceType.Elem()
 	return call.Args[last:], argType
 }



Test patch:
diff --git a/checkers/testdata/dupOption/negative_tests.go b/checkers/testdata/dupOption/negative_tests.go
index fe9ae4d..e4972a9 100644
--- a/checkers/testdata/dupOption/negative_tests.go
+++ b/checkers/testdata/dupOption/negative_tests.go
@@ -10,3 +10,18 @@ func doSome2(w, h int) {
 		withImage(nil, ""),
 	)
 }
+
+func f(a int, b string, c ...float64) {}
+
+func g() (int, string, float64) {
+	return 42, "hello", 3.14
+}
+
+func h() (int, string) {
+	return 42, "hello"
+}
+
+func _() {
+	f(g())
+	f(h())
+}