go/parser: ast.File.File{Start,End} fields are uninitialized when parser bails out early #70162
Assignees
Milestone
Comments
|
Related Issues and Documentation (Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.) |
|
Change https://go.dev/cl/624437 mentions this issue: |
|
Change https://go.dev/cl/624655 mentions this issue: |
gopherbot
pushed a commit
to golang/tools
that referenced
this issue
Nov 4, 2024
This CL fixes a bug (#70149) in gopls/internal/golang/pkgdoc.go in which a call to fset.File(file.Pos()) would return nil because when file points to an empty ast.File, Pos() returns NoPos. Instead, we should use file.FileStart, which is (in principle) always valid even for an empty file. However, there is a separate bug in go1.23 (#70162) that means FileStart is invalid whenever Pos() is. So, this fix only works with go1.24, and there's no real workaround short of the additional logic this CL adds to parsego.Parse, which at least covers all of gopls. Also, we audit all of x/tools for similar faulty uses of Pos() and replace them with FileStart. In future, we should use File.Pos only for its specific meaning related to the package decl. Fixes golang/go#70149 Updates golang/go#70162 Change-Id: Ic8cedfe912e44a0b4eb6e5e6874a6266d4be9076 Reviewed-on: https://go-review.googlesource.com/c/tools/+/624437 Reviewed-by: Robert Findley <[email protected]> Auto-Submit: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
|
Change https://go.dev/cl/631675 mentions this issue: |
gopherbot
pushed a commit
to golang/tools
that referenced
this issue
Nov 25, 2024
This CL adds defensive assertions to ExtractToNewFile, which experiences a crash due to out-of-bounds indexing. The logic looks sound, but clearly something is off. This CL also fixes a mistake in the logic added to parsego.Parse to work around golang/go#70162 (missing File.FileStart in go/parser), but I don't think that bug explains golang/go#70553 because it is concerned only with deeply broken files. Updates golang/go#70553 Updates golang/go#70162 Change-Id: Ie4f6fbbde2046023d7a3b865e5810bbed3be3118 Reviewed-on: https://go-review.googlesource.com/c/tools/+/631675 Reviewed-by: Robert Findley <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When the parser fails to parse even the package declaration, it bails out early, returning a nil
*ast.File. The calling code has a deferred function that replaces this nil with a mininally valid ast.File... except that it fails to populate the FileStart and FileEnd fields (my bad).Example: https://go.dev/play/p/KtgnRPdgyPE
We should populate these fields in all cases:
I will send a CL.
The text was updated successfully, but these errors were encountered: