Background: Some ast.Node types, such as ast.StructType and ast.InterfaceType, compute their End position based on assumptions about where "}" braces appear in well-formed input. However, in a truncated file--a common input to gopls when one is composing a new file--the brace may be missing and the computed End position may be beyond EOF; or it may be zero. Both have been a widespread source of bugs (e.g. 71659).
Proposal: We propose to establish (a) the invariant that, in all ast.File trees produced by the parser, every Node has a non-zero Pos and End value, and (b) the inequality File.FileStart <= Node.Pos <= Node.End <= and File.End.
This will likely require the addition of new fields to record token.Pos values (or their absence) in the AST.
@jba @findleyr @griesemer
Comment From: findleyr
Thank you for filing this issue. It should be a proposal, since it will require adding new fields to the AST.
However, I consider it a bug that End overflows the file. It should be the case that if the parser accepts a struct
keyword as a struct type, with no braces, then it must record the absence of those braces.
The alternative would be to use BadExpr for a dangling struct
keyword. The only downside I see there is that we could not record valuable information about struct{ Fields...
with no closing brace, although given the current poor recovery in the presence of mismatching delimiters, I'm not sure that makes much difference.
My preference would be to:
1. Add LBrace
and RBrace
to StructType
.
2. Document the invariant.
Note that InterfaceType
should be fine since FieldList records its Opening
and Closing
position.
Comment From: adonovan
I agree it will need a proposal, but the first step is to investigate what changes are needed to the parser and go/ast. Once that's done, we can turn this into a proposal.
A complete audit is required; StructType is just one of the known problematic cases. It's possible that StructType can be fixed with only a change to the logic in End. For example:
struct
EOF -> Pos + len("struct")struct {
EOF -> s.Fields.Opening + 1
Comment From: gabyhelp
Related Issues
- go/ast: record start and end of file in File.FileStart, File.FileEnd #53202 (closed)
- go/parser: StructType.End() can overflow the File #48300
- go/parser: StructType.End() can underflow the File #66683
- go/parser: ast.File.File{Start,End} fields are uninitialized when parser bails out early #70162 (closed)
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
Comment From: adonovan
@jba, I am stealing this from you.
Comment From: gopherbot
Change https://go.dev/cl/668238 mentions this issue: go/ast: enforce FileStart <= Node.Pos <= Node.End <= FileEnd
Comment From: gopherbot
Change https://go.dev/cl/668677 mentions this issue: x/tools: prepare for go1.25 parser changes
Comment From: gopherbot
Change https://go.dev/cl/672055 mentions this issue: x/tools: various cleanups related to planned parser changes