x/tools/gopls: analysis driver spuriously reports invalid suggested fixes due to AST repair

[adonovan]

#!stacks
"bug.Reportf" && "cache.(*action).exec.func3:+3" &&
  "unreachable.(*deadState).findDead:+17"

Issue created by stacks.

This stack indicates that the unreachable analyzer violates the SuggestedFix preconditions now documented and checked by the gopls analysis driver.

This stack Gf7EXQ was reported by telemetry:

• gopls/bug
• golang.org/x/tools/gopls/internal/util/bug.report:+35
• golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
• golang.org/x/tools/gopls/internal/cache.(*action).exec.func3:+3
• golang.org/x/tools/go/analysis/passes/unreachable.(*deadState).findDead:+17
• golang.org/x/tools/go/analysis/passes/unreachable.(*deadState).findDead:+49
• golang.org/x/tools/go/analysis/passes/unreachable.run.func1:+19
• golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder:+20
• golang.org/x/tools/go/analysis/passes/unreachable.run:+7
• golang.org/x/tools/gopls/internal/cache.(*action).exec.func7:+31
• golang.org/x/tools/gopls/internal/cache.(*action).exec:+282
• golang.org/x/tools/gopls/internal/cache.execActions.func1.1:+2
• sync.(*Once).doSlow:+5
• sync.(*Once).Do:=67
• golang.org/x/tools/gopls/internal/cache.execActions.func1:+2
• runtime.goexit:+0

golang.org/x/tools/gopls@v0.18.0-pre.1 go1.23.5 windows/amd64 vscode (1)

Dups: EmaIvQ KznUPA JuVvTw 10HCWA mAqhog S-mQmQ 2q8x9A Vc6s2g yM5t_A Ne_rfg rN06Cw QGVc7g cp-4UQ fb8Dbg bgKp4Q H8EfOg tIGLig s00Qzg wX0s_A EFdz_A _AqRig 2l7h9g 6L5mmA g92Uyw 2e6Ziw B3yvxg SLq6lA eMJmBQ VMn62A nxVngw 4wnTaA H2vDzA S4gjMA -DlRtg Rm3VbQ Zumn2w H0xvEQ -ubuiA eFXWcg U_brvg TqSiJg ipVwYg Zumn1Q WCZHkQ nR04fA kIYX0g 0bITOw VdeswQ XFAqdQ PbQHuw 6voO2Q pZqskQ DA7D_A VH1-DA i9_tJQ n5N7dg tIGkrQ LnzCqA x1y8CA Qp27eQ PFqrmQ 4mILPg UDizTQ b9U4IQ PvdWVA 8wIiMg Bq6TzA YJB1_g F4iflQ iFCmBQ wdx4HQ nKrzAg Yfy4AA 6DjaiQ PzXexg uUz53Q PbQHtQ iL26ow tZ47aA hEZ8jQ _ib_lQ ES4UWQ l5KxTA SiK0wg a1BwtQ Qp27dw sxnjrg pjphnQ CZptaQ _KvAFg uUYSlQ PFqrlw s81mPQ gz-6_Q TnPjWA P45F0g g92UxQ UNskGw nxVnjQ ToZf2A MDBb5A XOUtMg NxSYDQ UrxMYQ s81mMw omHK6Q D42jvg cvb-1Q R2CNEA Ba1Hhg -o---g Lz4aYg S-mQlw 1ajsgA 02S12g tM7wUQ Pq5zVg

[gabyhelp]

Related Issues

• x/tools/gopls: "missing reachable node for %q" bug in evaluatePackageHandle #71352
• x/tools/gopls: "internal error converting diagnostic" (bug reported by telemetry) #64547
• x/tools/gopls: analysisinternal.ZeroValue crash on (presumably) unsafe.Pointer #70585 (closed)
• x/tools/gopls: "diagnostic location is not among files of package: %s" bug (with analyzer SA4023) #70791
• x/tools/gopls: toGobDiagnostic bug reported by telemetry #65695 (closed)
• x/tools/gopls: DidModifyFiles: "non-abs file path %q" bug in port.matches #70603
• x/tools/gopls: bug in analysis importer reported by telemetry #64236 (closed)
• x/tools/gopls: "nil handle" assert in getPackageHandles #71353
• x/tools/gopls: xrefs.Index panics after "can't fail" pgf.PosRange #70446 (closed)
• x/tools/gopls: sigpanic in persistent.(*mapNode).forEach #71115

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

[prattmic]

In v0.17.1, line 1143 was the third line of cache.(*action).exec.func3 (but is not a call to Reportf). Did stacks fetch the wrong version of the source?

https://cs.opensource.google/go/x/tools/+/refs/tags/gopls/v0.17.1:gopls/internal/cache/analysis.go;l=1143

Edit: oops, meant to post this on #71660.

[adonovan]

Yes, I think a version skew is possible since the line numbers are not always off by one by some variable amount, which could correspond to recent changes in the code.

[adonovan]

This stack indicates that the unreachable analyzer violates the SuggestedFix preconditions now documented and checked by the gopls analysis driver.

This call to pass.Report has a diagnostic with a single SuggestedFix of a single TextEdit (a deletion). The various possible errors returned by validateFix are:

• "missing file info for pos (%v)" (File(start) == nil)
• "pos (%v) > end (%v)" (end < start)
• "malformed end position %v" (File(end) == nil, end))
• "edit spans files %v and %v" (endFile != file)
• "end is (%v) beyond end of file (%v)" (edit.End > file.Base() + file.Size()))

("overlapping edits" is not possible, nor is "analyzer %q suggests two fixes with same Message (%s)")

This means that at least one of stmt.Pos(), stmt.End() is invalid (zero, beyond EOF, or not mapped to a token.File). stmt is an element of ast.BlockStmt.List. The logic in the unreachable analyzer is unimpeachable, so I think either the parser recovery or the AST fixing logic must be to blame. Perhaps the input file is incomplete, something like func _() {return; var _ struct (EOF), and the parser's recovery has filled in values for the missing braces that are invalid or beyond EOF. This is especially likely if the Stmt.End() is computed by addition of a constant to the pos of some actual token.

(See #73438)

[adonovan]

Indeed, some variant of func _() {return; var _ struct did reproduce the bug.Report, but unfortunately the failure is silent to the user--the fix is discarded--so I had to check gopls bug to see it... but I lost the actual input as I tried many in a short span of time.

Nonetheless, it confirms that it's a truncated file AST repair issue, in other words another variant of #66790 (comment) (which is perhaps also the root cause of #64547)