Better error when trying to use a keyword as a record field #7784
Conversation
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
There was a problem hiding this comment.
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 or learn how to get started.
compiler/syntax/src/res_core.ml
Outdated
| @@ -398,6 +398,27 @@ let build_longident words = | |||
| | [] -> assert false | |||
| | hd :: tl -> List.fold_left (fun p s -> Longident.Ldot (p, s)) (Lident hd) tl | |||
|
|
|||
| (* Recovers a keyword used as field name if it's probable that it's a full | |||
There was a problem hiding this comment.
[nitpick] The comment has inconsistent spacing. There should be a space after 'full' to maintain proper spacing before the line break.
Copilot uses AI. Check for mistakes.
compiler/syntax/src/res_core.ml
Outdated
| @@ -398,6 +398,27 @@ let build_longident words = | |||
| | [] -> assert false | |||
| | hd :: tl -> List.fold_left (fun p s -> 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. *) | |||
There was a problem hiding this comment.
[nitpick] The comment should end the sentence with a period before the closing comment marker for consistency.
Copilot uses AI. Check for mistakes.
compiler/syntax/src/res_core.ml
Outdated
| let keyword_txt = Token.to_string p.token in | ||
| let keyword_start = p.Parser.start_pos in | ||
| let keyword_end = p.Parser.end_pos in | ||
| let message = |
There was a problem hiding this comment.
why identical message construction here and a few lines earlier?
There was a problem hiding this comment.
Solved in a3870f7 (was just working on it when you commented).
zth
force-pushed
the
hint-keyword-as-record-field
branch
from
5ce8a8d to
a3870f7
Compare
Closes #7579
Also adds error recovery so the parser can continue and spits out less irrelevant parse errors.