Report undefined start symbols - #804
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
find_symbol_by_id! now assumes id.location is non-nil, but tokens can be constructed without a location, which can reintroduce a NoMethodError on unresolved lookups.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves diagnostics for undefined start symbols and other unresolved grammar symbols by preserving token source locations when converting strings to identifiers and by emitting the standard “symbol … is used, but is not defined …” message with proper file/line/column context.
Changes:
- Preserve
STRINGtoken location when reducingstring_as_idinto anIdenttoken. - Update symbol resolution failure to use
Location#generate_error_messagefor consistent, source-located diagnostics. - Add/adjust specs to assert the new error message formatting for undefined
%startand unresolved symbols.
File summaries
| File | Description |
|---|---|
| spec/lrama/parser_spec.rb | Adds an expectation for a source-located error when %start names an undefined symbol. |
| spec/lrama/grammar/symbols/resolver_spec.rb | Updates resolver error expectations to the standard diagnostic format with carets. |
| parser.y | Preserves STRING token location when converting to Ident. |
| lib/lrama/parser.rb | Updates the generated reduce action to pass through location. |
| lib/lrama/grammar/symbols/resolver.rb | Changes unresolved-symbol raising to use generate_error_message for consistent diagnostics. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The changes are small, consistent across generator source and generated output, and are covered by new/updated specs that validate the improved diagnostics and location handling.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
An undefined
%startsymbol could leak an internalNoMethodErrorwhen a string alias appeared as the first symbol on a rule's right-hand side. Thestring_as_idparser action discarded the original token location, and unresolved symbols otherwise produced a generic internal resolver error.Preserve the string token's location when converting it to an identifier, and report unresolved symbols with their source location and the standard grammar diagnostic.