- Notifications
You must be signed in to change notification settings - Fork471
Better error when trying to use a keyword as a record field#7784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
pkg-pr-newbot commentedAug 22, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
rescript@rescript/darwin-arm64@rescript/darwin-x64@rescript/linux-arm64@rescript/linux-x64@rescript/win32-x64commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR improves error handling when developers attempt to use language keywords as record field names in ReScript. The change provides clearer error messages and better error recovery to continue parsing after encountering such errors.
- Adds helpful error messages suggesting alternatives (e.g.,
type_) and the@asannotation for runtime compatibility - Implements error recovery to continue parsing and reduce cascading parse errors
- Covers all record field contexts: type definitions, expressions, and patterns
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| res_core.ml | Implements keyword detection and recovery logic for record fields with improved error messages |
| recordFieldKeywordInType.res | Test case for keyword usage in record type definitions |
| recordFieldKeywordInExpr.res | Test case for keyword usage in record expressions |
| recordFieldKeywordInPattern.res | Test case for keyword usage in record patterns |
| expected/*.txt | Expected error outputs showing improved error messages and recovery |
| CHANGELOG.md | Documents the enhancement in the changelog |
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
compiler/syntax/src/res_core.ml Outdated
| |[] ->assertfalse | ||
| |hd ::tl ->List.fold_left (funps ->Longident.Ldot (p, s)) (Lident hd) tl | ||
| (* Recovers a keyword used as field name if it's probable that it's a full |
CopilotAIAug 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[nitpick] The comment has inconsistent spacing. There should be a space after 'full' to maintain proper spacing before the line break.
compiler/syntax/src/res_core.ml Outdated
| |hd ::tl ->List.fold_left (funps ->Longident.Ldot (p, s)) (Lident hd) tl | ||
| (* Recovers a keyword used as field name if it's probable that it's a full | ||
| field name (not punning etc), by checking if there's a colon after it.*) |
CopilotAIAug 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[nitpick] The comment should end the sentence with a period before the closing comment marker for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Much cleaner.
Left some nitpick.
compiler/syntax/src/res_core.ml Outdated
| let keyword_txt=Token.to_string p.tokenin | ||
| let keyword_start= p.Parser.start_posin | ||
| let keyword_end= p.Parser.end_posin | ||
| let message= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why identical message construction here and a few lines earlier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Solved ina3870f7 (was just working on it when you commented).
5ce8a8d toa3870f7Comparefc7226b intomasterUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Closes#7579
Also adds error recovery so the parser can continue and spits out less irrelevant parse errors.