x/tools/gopls: ChangeSignature: crash in in inliner (likely due to ill-formed variadic call)

[adonovan]

#!stacks
"panicdottypeI" && "inline.(*state).inlineCall:+370"

Issue created by stacks.

				// simplify decl: func(T...) -> func([]T)
				lastParamField := last(calleeDecl.Type.Params.List)
				lastParamField.Type = &ast.ArrayType{
					Elt: lastParamField.Type.(*ast.Ellipsis).Elt,   <---- panicdottypeI
				}

The fix is to do something like what gopls does in gopls/internal/golang/inline.go, but in the inline package itself so that all callers (e.g. ChangeSignature) benefit:

	// The inliner assumes that input is well-typed,
	// but that is frequently not the case within gopls.
	// Until we are able to harden the inliner,
	// report panics as errors to avoid crashing the server.
	bad := func(p *cache.Package) bool { return len(p.ParseErrors())+len(p.TypeErrors()) > 0 }
	if bad(calleePkg) || bad(callerPkg) {
		defer func() {
			if x := recover(); x != nil {
				err = fmt.Errorf("inlining failed (%q), likely because inputs were ill-typed", x)
			}
		}()
	}

This stack nlN-6w was reported by telemetry:

• crash/crash
• runtime.gopanic:+69
• runtime.panicdottypeE:=275
• runtime.panicdottypeI:+5
• golang.org/x/tools/internal/refactor/inline.(*state).inlineCall:+370
• golang.org/x/tools/internal/refactor/inline.(*state).inline:+17
• golang.org/x/tools/internal/refactor/inline.Inline:+13
• golang.org/x/tools/gopls/internal/golang.inlineAllCalls:+171
• golang.org/x/tools/gopls/internal/golang.rewriteCalls:+118
• golang.org/x/tools/gopls/internal/golang.ChangeSignature:+142
• golang.org/x/tools/gopls/internal/server.(*commandHandler).ChangeSignature.func1:+15
• golang.org/x/tools/gopls/internal/server.(*commandHandler).run.func2:+3
• golang.org/x/tools/gopls/internal/server.(*commandHandler).run:+81
• golang.org/x/tools/gopls/internal/server.(*commandHandler).ChangeSignature:+2
• golang.org/x/tools/gopls/internal/protocol/command.Dispatch:+45
• golang.org/x/tools/gopls/internal/server.(*server).ResolveCodeAction:+21
• golang.org/x/tools/gopls/internal/protocol.serverDispatch:+46
• golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3:+5

golang.org/x/tools/gopls@v0.18.1 go1.23.5 darwin/arm64 vscode (2)

[gabyhelp]

Related Issues

• x/tools/internal/refactor/inline: panic in needsParens #71486 (closed)
• x/tools/gopls: "inline: corrupted reference %v" bug in golang.RemoveUnusedParameter #69896 (closed)
• internal/refactor/inline: "args/params mismatch" panic (reported by telemetry) #64558 (closed)
• x/tools/gopls: completion: failed type assertion in inferExpectedTypeArg #70889 (closed)
• x/tools/gopls: panic in inliner (reported by telemetry) #64836 (closed)
• x/tools/gopls: panic in internal/golang.activeParameter #69552 (closed)
• x/tools/gopls: nil *types.Package panic in testKind #73096 (closed)
• x/tools/gopls: nil deref in Snapshot.MetadataForFile #73114
• x/tools/gopls: nil deref panic in go.test code action #72907 (closed)
• x/tools/gopls: Rename: crash (in satisfy) with ill-typed input #73109

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[adonovan]

I'm not sure what kind of input could trigger this: we have sig.Variadic(), meaning the callee's parameter list is known to end in ...T, yet the syntax for the callee's FuncType.Params doesn't end in ast.Ellipsis. Even in ill-typed code, one would expect the type-checker's Variadic flag to be consistent with the FuncType.