From a9aaa40a6a9aaf904dee2fb73aac3ab3c8d152ba Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 22 May 2025 19:23:03 +0200 Subject: [PATCH 1/5] print type coercion hint when appropriate --- compiler/ml/error_message_utils.ml | 49 ++++++++++++++++++- .../expected/primitives1.res.expected | 6 +++ .../expected/primitives6.res.expected | 6 +++ .../expected/subtype_record.res.expected | 16 ++++++ .../expected/subtype_string.res.expected | 16 ++++++ .../super_errors/expected/type1.res.expected | 6 +++ .../super_errors/fixtures/subtype_record.res | 23 +++++++++ .../super_errors/fixtures/subtype_string.res | 7 +++ 8 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/subtype_record.res.expected create mode 100644 tests/build_tests/super_errors/expected/subtype_string.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/subtype_record.res create mode 100644 tests/build_tests/super_errors/fixtures/subtype_string.res diff --git a/compiler/ml/error_message_utils.ml b/compiler/ml/error_message_utils.ml index ebc138284f..84e5176cfd 100644 --- a/compiler/ml/error_message_utils.ml +++ b/compiler/ml/error_message_utils.ml @@ -11,6 +11,8 @@ let with_configured_jsx_module s = module Parser : sig type comment + val extract_text_at_loc : Location.t -> string + val parse_source : (string -> Parsetree.structure * comment list) ref val reprint_source : (Parsetree.structure -> comment list -> string) ref @@ -38,10 +40,13 @@ end = struct let end_offset = end_pos.pos_cnum in String.sub src start_offset (end_offset - start_offset) - let parse_expr_at_loc loc = + let extract_text_at_loc loc = (* TODO: Maybe cache later on *) let src = Ext_io.load_file loc.Location.loc_start.pos_fname in - let sub_src = extract_location_string ~src loc in + extract_location_string ~src loc + + let parse_expr_at_loc loc = + let sub_src = extract_text_at_loc loc in let parsed, comments = !parse_source sub_src in match parsed with | [{Parsetree.pstr_desc = Pstr_eval (exp, _)}] -> Some (exp, comments) @@ -59,6 +64,12 @@ end = struct | None -> None end +(* TODO: Move this to appropriate place where it can be shared with typecore.ml *) +let type_expr ppf typ = + (* print a type and avoid infinite loops *) + Printtyp.reset_and_mark_loops typ; + Printtyp.type_expr ppf typ + type type_clash_statement = FunctionCall type type_clash_context = | SetRecordField @@ -345,6 +356,40 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf single JSX element.@," (with_configured_jsx_module "array") | _ -> ()) + | _, Some (t1, t2) -> + let is_subtype = + try + Ctype.subtype env t1 t2 (); + true + with _ -> false + in + let target_type_string = Format.asprintf "%a" type_expr t2 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) + in + + if is_subtype then ( + fprintf ppf + "@,\ + @,\ + Possible solutions: @,\ + - These types are compatible at runtime. You can use the coercion \ + operator @{:>@} to convert to the expected type @{%s@}." + target_type_string; + match suggested_rewrite with + | Some rewrite -> + fprintf ppf + "@,\ + \ If you want to use coercion, rewrite the highlighted code to: \ + @{%s@}@," + rewrite + | None -> ()) | _ -> () let type_clash_context_from_function sexp sfunct = diff --git a/tests/build_tests/super_errors/expected/primitives1.res.expected b/tests/build_tests/super_errors/expected/primitives1.res.expected index 761f341aba..d392b80c3e 100644 --- a/tests/build_tests/super_errors/expected/primitives1.res.expected +++ b/tests/build_tests/super_errors/expected/primitives1.res.expected @@ -9,5 +9,11 @@ This has type: int But it's expected to have type: float + Possible solutions: + - These types are compatible at runtime. You can use the coercion operator :> to convert to the expected type float. + If you want to use coercion, rewrite the highlighted code to: (2 :> float) + + + You can convert int to float with Int.toFloat. If this is a literal, try a number with a trailing dot (e.g. 20.). \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/primitives6.res.expected b/tests/build_tests/super_errors/expected/primitives6.res.expected index cc665fac7b..17566f03b3 100644 --- a/tests/build_tests/super_errors/expected/primitives6.res.expected +++ b/tests/build_tests/super_errors/expected/primitives6.res.expected @@ -10,5 +10,11 @@ This has type: int But it's expected to have type: float + Possible solutions: + - These types are compatible at runtime. You can use the coercion operator :> to convert to the expected type float. + If you want to use coercion, rewrite the highlighted code to: (10 :> float) + + + You can convert int to float with Int.toFloat. If this is a literal, try a number with a trailing dot (e.g. 20.). \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/subtype_record.res.expected b/tests/build_tests/super_errors/expected/subtype_record.res.expected new file mode 100644 index 0000000000..c4b08d60ce --- /dev/null +++ b/tests/build_tests/super_errors/expected/subtype_record.res.expected @@ -0,0 +1,16 @@ + + We've found a bug for you! + /.../fixtures/subtype_record.res:23:23 + + 21 │ } + 22 │ + 23 │ let x = takesSomeType(v) + 24 │ + + This has type: someOtherType + But this function argument is expecting: SomeModule.someType + + Possible solutions: + - These types are compatible at runtime. You can use the coercion operator :> to convert to the expected type SomeModule.someType. + If you want to use coercion, rewrite the highlighted code to: (v :> SomeModule.someType) + \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/subtype_string.res.expected b/tests/build_tests/super_errors/expected/subtype_string.res.expected new file mode 100644 index 0000000000..faed95761f --- /dev/null +++ b/tests/build_tests/super_errors/expected/subtype_string.res.expected @@ -0,0 +1,16 @@ + + We've found a bug for you! + /.../fixtures/subtype_string.res:7:18-20 + + 5 │ } + 6 │ + 7 │ let x = takesStr(One) + 8 │ + + This has type: variant + But it's expected to have type: string + + Possible solutions: + - These types are compatible at runtime. You can use the coercion operator :> to convert to the expected type string. + If you want to use coercion, rewrite the highlighted code to: (One :> string) + \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/type1.res.expected b/tests/build_tests/super_errors/expected/type1.res.expected index 4ca40ba83b..7eee763594 100644 --- a/tests/build_tests/super_errors/expected/type1.res.expected +++ b/tests/build_tests/super_errors/expected/type1.res.expected @@ -8,5 +8,11 @@ This has type: int But it's expected to have type: float + Possible solutions: + - These types are compatible at runtime. You can use the coercion operator :> to convert to the expected type float. + If you want to use coercion, rewrite the highlighted code to: (2 :> float) + + + You can convert int to float with Int.toFloat. If this is a literal, try a number with a trailing dot (e.g. 20.). \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/subtype_record.res b/tests/build_tests/super_errors/fixtures/subtype_record.res new file mode 100644 index 0000000000..08ebb3f385 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/subtype_record.res @@ -0,0 +1,23 @@ +module SomeModule = { + type someType = { + one: bool, + two: int, + } +} + +type someOtherType = { + ...SomeModule.someType, + three: string, +} + +let v = { + one: true, + two: 1, + three: "hello", +} + +let takesSomeType = (s: SomeModule.someType) => { + s.one +} + +let x = takesSomeType(v) diff --git a/tests/build_tests/super_errors/fixtures/subtype_string.res b/tests/build_tests/super_errors/fixtures/subtype_string.res new file mode 100644 index 0000000000..c41504c5c6 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/subtype_string.res @@ -0,0 +1,7 @@ +type variant = One | Two + +let takesStr = (s: string) => { + s ++ "hello" +} + +let x = takesStr(One) From 6c0c7037fde452efe6fee9cf2152ee7b4a71a145 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 22 May 2025 20:33:58 +0200 Subject: [PATCH 2/5] refactor --- compiler/ml/error_message_utils.ml | 1 - compiler/ml/typecore.ml | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/ml/error_message_utils.ml b/compiler/ml/error_message_utils.ml index 84e5176cfd..444bf57e75 100644 --- a/compiler/ml/error_message_utils.ml +++ b/compiler/ml/error_message_utils.ml @@ -64,7 +64,6 @@ end = struct | None -> None end -(* TODO: Move this to appropriate place where it can be shared with typecore.ml *) let type_expr ppf typ = (* print a type and avoid infinite loops *) Printtyp.reset_and_mark_loops typ; diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index de48f4b2c7..49ed9ea973 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -4168,10 +4168,8 @@ let longident = Printtyp.longident let super_report_unification_error = Printtyp.super_report_unification_error let report_ambiguous_type_error = Printtyp.report_ambiguous_type_error let report_subtyping_error = Printtyp.report_subtyping_error -let type_expr ppf typ = - (* print a type and avoid infinite loops *) - Printtyp.reset_and_mark_loops typ; - Printtyp.type_expr ppf typ + +let type_expr = Error_message_utils.type_expr let report_error env loc ppf error = match error with From 8b11764848e63a74d5983a17120137920ca1ccaf Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 22 May 2025 20:51:29 +0200 Subject: [PATCH 3/5] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b44e383e83..7811976ae2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ - Complete from `RegExp` stdlib module for regexes. https://github.com/rescript-lang/rescript/pull/7425 - Allow oneliner formatting when including module with single type alias. https://github.com/rescript-lang/rescript/pull/7502 - Improve error messages for JSX type mismatches, passing objects where record is expected, passing array literal where tuple is expected, and more. https://github.com/rescript-lang/rescript/pull/7500 +- Show in error messages when coercion can be used to fix a type mismatch. https://github.com/rescript-lang/rescript/pull/7505 # 12.0.0-alpha.13 From 7fe8df8180346fa7902f766cfb666a37e1095083 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 23 May 2025 11:04:51 +0200 Subject: [PATCH 4/5] do not suggest coercion for constant expressions --- compiler/ml/error_message_utils.ml | 11 ++++++++++- .../super_errors/expected/primitives1.res.expected | 6 ------ .../super_errors/expected/primitives6.res.expected | 6 ------ .../super_errors/expected/type1.res.expected | 6 ------ 4 files changed, 10 insertions(+), 19 deletions(-) diff --git a/compiler/ml/error_message_utils.ml b/compiler/ml/error_message_utils.ml index 444bf57e75..660f3cab9b 100644 --- a/compiler/ml/error_message_utils.ml +++ b/compiler/ml/error_message_utils.ml @@ -373,7 +373,16 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf | structure, comments -> Some (!Parser.reprint_source structure comments) in - if is_subtype then ( + (* Suggesting coercion only makes sense for non-constant values. *) + let is_constant = + match !Parser.parse_source target_expr_text with + | ( [{Parsetree.pstr_desc = Pstr_eval ({pexp_desc = Pexp_constant _}, _)}], + _ ) -> + true + | _ -> false + in + + if is_subtype && not is_constant then ( fprintf ppf "@,\ @,\ diff --git a/tests/build_tests/super_errors/expected/primitives1.res.expected b/tests/build_tests/super_errors/expected/primitives1.res.expected index d392b80c3e..761f341aba 100644 --- a/tests/build_tests/super_errors/expected/primitives1.res.expected +++ b/tests/build_tests/super_errors/expected/primitives1.res.expected @@ -9,11 +9,5 @@ This has type: int But it's expected to have type: float - Possible solutions: - - These types are compatible at runtime. You can use the coercion operator :> to convert to the expected type float. - If you want to use coercion, rewrite the highlighted code to: (2 :> float) - - - You can convert int to float with Int.toFloat. If this is a literal, try a number with a trailing dot (e.g. 20.). \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/primitives6.res.expected b/tests/build_tests/super_errors/expected/primitives6.res.expected index 17566f03b3..cc665fac7b 100644 --- a/tests/build_tests/super_errors/expected/primitives6.res.expected +++ b/tests/build_tests/super_errors/expected/primitives6.res.expected @@ -10,11 +10,5 @@ This has type: int But it's expected to have type: float - Possible solutions: - - These types are compatible at runtime. You can use the coercion operator :> to convert to the expected type float. - If you want to use coercion, rewrite the highlighted code to: (10 :> float) - - - You can convert int to float with Int.toFloat. If this is a literal, try a number with a trailing dot (e.g. 20.). \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/type1.res.expected b/tests/build_tests/super_errors/expected/type1.res.expected index 7eee763594..4ca40ba83b 100644 --- a/tests/build_tests/super_errors/expected/type1.res.expected +++ b/tests/build_tests/super_errors/expected/type1.res.expected @@ -8,11 +8,5 @@ This has type: int But it's expected to have type: float - Possible solutions: - - These types are compatible at runtime. You can use the coercion operator :> to convert to the expected type float. - If you want to use coercion, rewrite the highlighted code to: (2 :> float) - - - You can convert int to float with Int.toFloat. If this is a literal, try a number with a trailing dot (e.g. 20.). \ No newline at end of file From fb546a25d2c725a470b934632ff40d536d31768c Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 23 May 2025 11:32:51 +0200 Subject: [PATCH 5/5] tighten wording --- compiler/ml/error_message_utils.ml | 12 +++--------- .../expected/subtype_record.res.expected | 3 +-- .../expected/subtype_string.res.expected | 3 +-- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/compiler/ml/error_message_utils.ml b/compiler/ml/error_message_utils.ml index 660f3cab9b..267ac6c0f6 100644 --- a/compiler/ml/error_message_utils.ml +++ b/compiler/ml/error_message_utils.ml @@ -388,16 +388,10 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf @,\ Possible solutions: @,\ - These types are compatible at runtime. You can use the coercion \ - operator @{:>@} to convert to the expected type @{%s@}." - target_type_string; + operator to convert to the expected type"; match suggested_rewrite with - | Some rewrite -> - fprintf ppf - "@,\ - \ If you want to use coercion, rewrite the highlighted code to: \ - @{%s@}@," - rewrite - | None -> ()) + | Some rewrite -> fprintf ppf ": @{%s@}@," rewrite + | None -> fprintf ppf ": @{:>@}@,") | _ -> () let type_clash_context_from_function sexp sfunct = diff --git a/tests/build_tests/super_errors/expected/subtype_record.res.expected b/tests/build_tests/super_errors/expected/subtype_record.res.expected index c4b08d60ce..4c34154326 100644 --- a/tests/build_tests/super_errors/expected/subtype_record.res.expected +++ b/tests/build_tests/super_errors/expected/subtype_record.res.expected @@ -11,6 +11,5 @@ But this function argument is expecting: SomeModule.someType Possible solutions: - - These types are compatible at runtime. You can use the coercion operator :> to convert to the expected type SomeModule.someType. - If you want to use coercion, rewrite the highlighted code to: (v :> SomeModule.someType) + - These types are compatible at runtime. You can use the coercion operator to convert to the expected type: (v :> SomeModule.someType)  \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/subtype_string.res.expected b/tests/build_tests/super_errors/expected/subtype_string.res.expected index faed95761f..47b413ded0 100644 --- a/tests/build_tests/super_errors/expected/subtype_string.res.expected +++ b/tests/build_tests/super_errors/expected/subtype_string.res.expected @@ -11,6 +11,5 @@ But it's expected to have type: string Possible solutions: - - These types are compatible at runtime. You can use the coercion operator :> to convert to the expected type string. - If you want to use coercion, rewrite the highlighted code to: (One :> string) + - These types are compatible at runtime. You can use the coercion operator to convert to the expected type: (One :> string)  \ No newline at end of file