x/tools/gopls: safetoken.Offset fails on a Stmt.End value from the parser

[adonovan]

@findleyr reported this error message at tip:

A bug occurred on the server: internal error converting diagnostic from analyzer unreachable:
in SuggestedFixes: end: pos 22988 is not in range for file [3697:22987)

It means that a SuggestedFix emitted by the unreachable analyzer had an end position that was beyond the end of the token.File, by exactly one byte. (The safetoken error message is wrong: it should use a ] to indicate an inclusive interval.)
The unreachable analyzer seems fine: the Start and End positions both come from a Stmt, which should be within the file. The logic in posToLocation also looks sound.

Needs investigating. Also, we should get the safetoken error to include the correct interval, and report the filename.

[gopherbot]

Change https://go.dev/cl/459637 mentions this issue: gopls/internal/lsp/safetoken: fix error message

[rfindley]

@adonovan,
I think your message is intended for @findleyr (rather than @rfindley)

[adonovan]

I suspect the cause is a discrepancy in the length of the file as reported by ast.File.{Pos,End} and by token.File.Size.

See this program for an example:

	src := `package p; func f() { var x struct`
	fset := token.NewFileSet()
	file, _ := parser.ParseFile(fset, "", src, 0)
	tf := fset.File(file.Pos())

	fmt.Printf("file.Pos,End=(%d,%d) End-Pos=%d tf.Size=%d len(src)=%d\n",
		file.Pos(), file.End(), file.End()-file.Pos(), tf.Size(), len(src))

	fn := file.Decls[0].(*ast.FuncDecl)
	stmt := fn.Body.List[0]
	fmt.Printf("stmt.Pos,End=(%d,%d)\n", stmt.Pos(), stmt.End())

Output:

file.Pos,End=(1,36) End-Pos=35 tf.Size=34 len(src)=34
stmt.Pos,End=(23,36)

Observe that the File syntax node reports a length of 35 bytes, including the implicit newline at EOF, whereas the token.File reports only 34 bytes, the true length of the input. So, when the parser sees an incomplete statement such as the struct type here, it performs recovery, effectively synthesizing the LPAREN and RPAREN at the EOF position, which is beyond the token.File size, thus causing the End position of any of the incomplete nodes (StructType, AssignStmt, BlockStmt, FuncDecl) to be invalid as an argument to safetoken.Offset as currently implemented.

The parser source already flags this risk in the some cases (such as ast.Bad{Decl,Stmt,Expr}), as witnessed by this comment, but it seems the problem is more pervasive.

@griesemer

[adonovan]

I think this is a bug in the parser, since it creates Pos values that are beyond the block that it reserves in a call to FileSet.AddFile. AddFile is very tightly specified and can't be changed, but I think we could make the parser reserve one extra byte of space for EOF without breaking anything. We can also add a workaround in safetoken.Offset.

I've reported the parser bug separately as #57490. This issue can be about gopls.

[gopherbot]

Change https://go.dev/cl/459735 mentions this issue: gopls/internal/lsp/safetoken: fix bug in Offset at EOF

[gopherbot]

Change https://go.dev/cl/459736 mentions this issue: gopls/internal/lsp/safetoken: funnel more calls through this package