Dedicated error message for ternaries #7804
Conversation
| (* TODO(attributes) Unify the attribute handling in the parser and rest of the compiler. *) | ||
| let is_ternary = | ||
| let rec has_ternary = function | ||
| | [] -> false | ||
| | ({Location.txt = "res.ternary"}, _) :: _ -> true | ||
| | _ :: rest -> has_ternary rest | ||
| in | ||
| has_ternary sexp.pexp_attributes | ||
| in | ||
| let return_context = | ||
| if is_ternary then Some TernaryReturn else Some IfReturn | ||
| in |
There was a problem hiding this comment.
Putting in my TODO to unify all of these attribute names across the syntax and compiler. A lot of them should probably be proper AST nodes.
There was a problem hiding this comment.
Would be great to track this in an issue instead.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a dedicated error message for ternary operators, distinguishing them from general if statement errors. Previously, ternary expressions used the same error messages as if statements since they desugar to if-then-else constructs.
- Adds a new
TernaryReturncontext type to differentiate ternary expressions from if statements - Updates error message formatting to specifically mention ternaries with
?and:operators - Adds test case demonstrating the new error message for type mismatches in ternary branches
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
compiler/ml/typecore.ml |
Detects ternary expressions via attributes and uses TernaryReturn context instead of IfReturn |
compiler/ml/error_message_utils.ml |
Adds TernaryReturn context type and corresponding error message formatting |
tests/build_tests/super_errors/fixtures/ternary_branch_mismatch.res |
Test fixture with ternary type mismatch |
tests/build_tests/super_errors/expected/ternary_branch_mismatch.res.expected |
Expected error output showing the new ternary-specific error message |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let is_ternary = | ||
| let rec has_ternary = function | ||
| | [] -> false | ||
| | ({Location.txt = "res.ternary"}, _) :: _ -> true |
There was a problem hiding this comment.
The magic string "res.ternary" should be defined as a constant to avoid duplication and make it easier to maintain if the attribute name changes.
| let is_ternary = | |
| let rec has_ternary = function | |
| | [] -> false | |
| | ({Location.txt = "res.ternary"}, _) :: _ -> true | |
| let res_ternary_attribute = "res.ternary" in | |
| let is_ternary = | |
| let rec has_ternary = function | |
| | [] -> false | |
| | ({Location.txt = res_ternary_attribute}, _) :: _ -> true |
Copilot uses AI. Check for mistakes.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
4 participants
The general if error was used before (because that's what the ternary desugars to). This gives ternaries a dedicated error.