From dfe3118e2a17447323760f170ba45195bfb659b1 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 28 Oct 2026 15:49:22 +0000 Subject: [PATCH] Suggest a coercion only for an operand that parses and name the shown type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: The coercion hint wrapped the source text at the expression's location, which leaves out the parentheses around a first or last operand, so the suggestion did not compile (#9105). It also named the expanded target type instead of the one the message shows. Diagnostic modified (coercion hint): Example: type a = {x: int, y: int} type b = {x: int} let v: a = {x: 1, y: 2} let w: b = v->(r => r) Before (diagnostic): - These types are compatible at runtime. You can use the coercion operator to convert to the expected type: v->(r => r :> b) After (diagnostic): - These types are compatible at runtime. You can use the coercion operator to convert to the expected type: :> Diagnostic modified (coercion target): Example: let f = (x: Generator.t<'a, unit, unit>): Iterable.t<'a> => x Before (diagnostic): But it's expected to have type: Iterable.t<'a> (defined as iterable<'a>) … You can use the coercion operator to convert to the expected type: (x :> iterable<'a>) After (diagnostic): But it's expected to have type: Iterable.t<'a> (defined as iterable<'a>) … You can use the coercion operator to convert to the expected type: (x :> Iterable.t<'a>) Review: Why not widen the location to the parentheses? The parser leaves them out of locations on purpose, for comment placement, so a hint that cannot rebuild the operand falls back to the bare operator. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Signed-off-by: Christoph Knittel --- ...coercion-hint-parenthesized-operand.fix.md | 1 + compiler/bsc/rescript_compiler_driver.ml | 6 ++++- compiler/ml/error_message_utils.ml | 26 ++++++++++++------- ...coercion_hint_cut_off_operand.res.expected | 14 ++++++++++ .../coercion_hint_names_alias.res.expected | 13 ++++++++++ .../coercion_hint_cut_off_operand.res | 4 +++ .../fixtures/coercion_hint_names_alias.res | 1 + 7 files changed, 55 insertions(+), 10 deletions(-) create mode 100644 changes/coercion-hint-parenthesized-operand.fix.md create mode 100644 tests/build_tests/super_errors/expected/coercion_hint_cut_off_operand.res.expected create mode 100644 tests/build_tests/super_errors/expected/coercion_hint_names_alias.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/coercion_hint_cut_off_operand.res create mode 100644 tests/build_tests/super_errors/fixtures/coercion_hint_names_alias.res diff --git a/changes/coercion-hint-parenthesized-operand.fix.md b/changes/coercion-hint-parenthesized-operand.fix.md new file mode 100644 index 0000000000..1055761b43 --- /dev/null +++ b/changes/coercion-hint-parenthesized-operand.fix.md @@ -0,0 +1 @@ +- Fix the coercion hint after a parenthesized operand and name the expected type as the error message shows it. diff --git a/compiler/bsc/rescript_compiler_driver.ml b/compiler/bsc/rescript_compiler_driver.ml index c4a5214b58..3aaab5e7cd 100644 --- a/compiler/bsc/rescript_compiler_driver.ml +++ b/compiler/bsc/rescript_compiler_driver.ml @@ -28,7 +28,11 @@ module Error_message_utils_support = struct Res_driver.parse_implementation_from_source ~display_filename:"" ~source in - (res.parsetree, res.comments |> List.map to_comment)); + (* Hints rewrite source text extracted at a location. Text that does + not parse cleanly (e.g. a location that leaves out the parentheses + around an operand) must not be turned into a suggestion. *) + if res.invalid then ([], []) + else (res.parsetree, res.comments |> List.map to_comment)); Error_message_utils.Parser.reprint_source := fun parsetree comments -> diff --git a/compiler/ml/error_message_utils.ml b/compiler/ml/error_message_utils.ml index 4969fa8cbc..6c07a598b3 100644 --- a/compiler/ml/error_message_utils.ml +++ b/compiler/ml/error_message_utils.ml @@ -772,20 +772,28 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf with _ -> false) | None -> false in + (* Name the target type as the message shows it, not its expansion. *) let target_type = - match top_level_types with - | Some (_, t2_top) -> t2_top - | None -> target_type + match trace with + | _ :: (t2_top, _) :: _ -> t2_top + | _ -> target_type in let target_type_string = Format.asprintf "%a" type_expr target_type in let target_expr_text = Parser.extract_text_at_loc loc in let suggested_rewrite = - match - !Parser.parse_source - (Printf.sprintf "(%s :> %s)" target_expr_text target_type_string) - with - | [], _ -> None - | structure, comments -> Some (!Parser.reprint_source structure comments) + (* The location of an expression leaves out the parentheses around its + first or last operand, so the text alone may not be an expression, + even when the wrapped text happens to parse. *) + match Parser.parse_expr_at_loc loc with + | None -> None + | Some _ -> ( + match + !Parser.parse_source + (Printf.sprintf "(%s :> %s)" target_expr_text target_type_string) + with + | [], _ -> None + | structure, comments -> + Some (!Parser.reprint_source structure comments)) in (* Suggesting coercion only makes sense for non-constant values. *) diff --git a/tests/build_tests/super_errors/expected/coercion_hint_cut_off_operand.res.expected b/tests/build_tests/super_errors/expected/coercion_hint_cut_off_operand.res.expected new file mode 100644 index 0000000000..b2bc4649a1 --- /dev/null +++ b/tests/build_tests/super_errors/expected/coercion_hint_cut_off_operand.res.expected @@ -0,0 +1,14 @@ + + We've found a bug for you! + /.../fixtures/coercion_hint_cut_off_operand.res:4:13-34 + + 2 │ type b = {x: int} + 3 │ let v: a = {x: 1, y: 2} + 4 │ let w: b = (true ? v : v)->(r => r) + 5 │ + + This has type: a + But it's expected to have type: b + + Possible solutions: + - These types are compatible at runtime. You can use the coercion operator to convert to the expected type: :> \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/coercion_hint_names_alias.res.expected b/tests/build_tests/super_errors/expected/coercion_hint_names_alias.res.expected new file mode 100644 index 0000000000..8ab76fb180 --- /dev/null +++ b/tests/build_tests/super_errors/expected/coercion_hint_names_alias.res.expected @@ -0,0 +1,13 @@ + + We've found a bug for you! + /.../fixtures/coercion_hint_names_alias.res:1:61 + + 1 │ let f = (x: Generator.t<'a, unit, unit>): Iterable.t<'a> => x + 2 │ + + This has type: Generator.t<'a, unit, unit> + But it's expected to have type: Iterable.t<'a> (defined as iterable<'a>) + + Possible solutions: + - These types are compatible at runtime. You can use the coercion operator to convert to the expected type: (x :> Iterable.t<'a>) + \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/coercion_hint_cut_off_operand.res b/tests/build_tests/super_errors/fixtures/coercion_hint_cut_off_operand.res new file mode 100644 index 0000000000..c35603558d --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/coercion_hint_cut_off_operand.res @@ -0,0 +1,4 @@ +type a = {x: int, y: int} +type b = {x: int} +let v: a = {x: 1, y: 2} +let w: b = (true ? v : v)->(r => r) diff --git a/tests/build_tests/super_errors/fixtures/coercion_hint_names_alias.res b/tests/build_tests/super_errors/fixtures/coercion_hint_names_alias.res new file mode 100644 index 0000000000..732529da2c --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/coercion_hint_names_alias.res @@ -0,0 +1 @@ +let f = (x: Generator.t<'a, unit, unit>): Iterable.t<'a> => x