internal/analysisinternal: permit Fix.End slightly beyond EOF

This CL fixes by far the most prolific source of gopls telemetry
field reports: the unreachable analyzer often reports diagnostics
at the end of a truncated file; because of problems with AST
the fix's end position may be beyond EOF, causing ValidateFixes
to fail, and gopls to report a bug.

The mitigation is to make validation tolerant of fixes whose
end is just slightly beyond EOF, and to clamp them to EOF.

+ regression test

Fixes golang/go#71659
Fixes golang/go#71812

Change-Id: Ib68a38d0797c9274eaf3d59fabd45cc120f1dd36
Reviewed-on: https://go-review.googlesource.com/c/tools/+/666675
Auto-Submit: Alan Donovan <[email protected]>
Commit-Queue: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

go/analysis/passes/unreachable/unreachable.go

@@ -188,6 +188,9 @@ func (d *deadState) findDead(stmt ast.Stmt) {
 		case *ast.EmptyStmt:
 			// do not warn about unreachable empty statements
 		default:
+			// (This call to pass.Report is a frequent source
+			// of diagnostics beyond EOF in a truncated file;
+			// see #71659.)
 			d.pass.Report(analysis.Diagnostic{
 				Pos:     stmt.Pos(),
 				End:     stmt.End(),

gopls/internal/cache/analysis.go

@@ -1122,6 +1122,10 @@ func (act *action) exec(ctx context.Context) (any, *actionSummary, error) {
 		ResultOf:     inputs,
 		Report: func(d analysis.Diagnostic) {
 			// Assert that SuggestedFixes are well formed.
+			//
+			// ValidateFixes allows a fix.End to be slightly beyond
+			// EOF to avoid spurious assertions when reporting
+			// fixes as the end of truncated files; see #71659.
 			if err := analysisinternal.ValidateFixes(apkg.pkg.FileSet(), analyzer, d.SuggestedFixes); err != nil {
 				bug.Reportf("invalid SuggestedFixes: %v", err)
 				d.SuggestedFixes = nil

gopls/internal/test/marker/testdata/diagnostics/issue71812.txt

@@ -0,0 +1,17 @@
+This input causes the unreachable analyzer to report a diagnostic
+about the var decl statement. Since the computed End pos of
+ast.StructType is beyond EOF, validation of SuggestedFixes fails.
+This used to trigger an assertion in gopls' analysis driver.
+
+See golang/go#71659 (and also #71812).
+
+-- flags --
+-ignore_extra_diags
+
+-- go.mod --
+module example.com
+go 1.18
+
+-- a/a.go --
+package a
+func _() { return; var x struct{

internal/analysisinternal/analysis.go

@@ -439,11 +439,25 @@ func validateFix(fset *token.FileSet, fix *analysis.SuggestedFix) error {
 		if file == nil {
 			return fmt.Errorf("no token.File for TextEdit.Pos (%v)", edit.Pos)
 		}
+		fileEnd := token.Pos(file.Base() + file.Size())
 		if end := edit.End; end.IsValid() {
 			if end < start {
 				return fmt.Errorf("TextEdit.Pos (%v) > TextEdit.End (%v)", edit.Pos, edit.End)
 			}
 			endFile := fset.File(end)
+			if endFile != file && end < fileEnd+10 {
+				// Relax the checks below in the special case when the end position
+				// is only slightly beyond EOF, as happens when End is computed
+				// (as in ast.{Struct,Interface}Type) rather than based on
+				// actual token positions. In such cases, truncate end to EOF.
+				//
+				// This is a workaround for #71659; see:
+				// https://github.com/golang/go/issues/71659#issuecomment-2651606031
+				// A better fix would be more faithful recording of token
+				// positions (or their absence) in the AST.
+				edit.End = fileEnd
+				continue
+			}
 			if endFile == nil {
 				return fmt.Errorf("no token.File for TextEdit.End (%v; File(start).FileEnd is %d)", end, file.Base()+file.Size())
 			}
@@ -454,7 +468,7 @@ func validateFix(fset *token.FileSet, fix *analysis.SuggestedFix) error {
 		} else {
 			edit.End = start // update the SuggestedFix
 		}
-		if eof := token.Pos(file.Base() + file.Size()); edit.End > eof {
+		if eof := fileEnd; edit.End > eof {
 			return fmt.Errorf("end is (%v) beyond end of file (%v)", edit.End, eof)
 		}