From c58187dc8a04730a3445b240ecde39843b433e1c Mon Sep 17 00:00:00 2001 From: Reilly Watson Date: Fri, 11 Feb 2022 07:44:58 -0500 Subject: [PATCH] Don't clobber the AST when fixing analysis/unitchecker calls multiple analyzers in parallel, using the same AST. This creates a race where clobbering the AST might result in unexpected behaviour from other linters. In particular, linters that use the buildssa analyzer can panic with an error about a non-constant BasicLit, because it builds up a set of constants and then expects that set to be the same later in its analysis process. Instead make a new CallExpr, and use that for the rewriting step. --- errwrap/errwrap.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/errwrap/errwrap.go b/errwrap/errwrap.go index ac74abe..143f56c 100644 --- a/errwrap/errwrap.go +++ b/errwrap/errwrap.go @@ -140,9 +140,18 @@ func run(pass *analysis.Pass) (interface{}, error) { newFormat[i+1] = 'w' + newCall := &ast.CallExpr{ + Fun: call.Fun, + Args: make([]ast.Expr, len(call.Args)), + Lparen: call.Lparen, + Ellipsis: call.Ellipsis, + Rparen: call.Rparen, + } + copy(newCall.Args, call.Args) + if bl, ok := call.Args[0].(*ast.BasicLit); ok { // replace the expression, keep the arguments the same - call.Args[0] = &ast.BasicLit{ + newCall.Args[0] = &ast.BasicLit{ Value: strconv.Quote(string(newFormat)), ValuePos: bl.ValuePos, Kind: bl.Kind, @@ -155,7 +164,7 @@ func run(pass *analysis.Pass) (interface{}, error) { if sel, ok := expr.Fun.(*ast.SelectorExpr); ok { if id, ok := sel.X.(*ast.Ident); ok { // remove the .String call from the error - call.Args[errIndex] = &ast.BasicLit{ + newCall.Args[errIndex] = &ast.BasicLit{ Value: id.String(), ValuePos: sel.X.Pos(), Kind: token.STRING, @@ -165,7 +174,7 @@ func run(pass *analysis.Pass) (interface{}, error) { } } - newExpr := render(pass.Fset, call) + newExpr := render(pass.Fset, newCall) pass.Report(analysis.Diagnostic{ Pos: call.Pos(),