From f9dedf62f42de497ae221a04e0e5786733d66e71 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 3 Jul 2025 21:52:21 +0200 Subject: [PATCH 1/4] improve a few error messages around polymorphic variants --- compiler/ml/error_message_utils.ml | 23 ++++++--- compiler/ml/printtyp.ml | 47 ++++++++++++++++--- ..._constructor_payload_mismatch.res.expected | 16 +++++++ ..._constructors_mismatch_second.res.expected | 17 +++++++ .../polyvariant_name_formatting.res.expected | 7 ++- .../polyvariants_no_overlap.res.expected | 12 +++++ ...lyvariant_constructor_payload_mismatch.res | 4 ++ ...lyvariant_constructors_mismatch_second.res | 7 +++ .../fixtures/polyvariants_no_overlap.res | 2 + 9 files changed, 122 insertions(+), 13 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/polyvariant_constructor_payload_mismatch.res.expected create mode 100644 tests/build_tests/super_errors/expected/polyvariant_constructors_mismatch_second.res.expected create mode 100644 tests/build_tests/super_errors/expected/polyvariants_no_overlap.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/polyvariant_constructor_payload_mismatch.res create mode 100644 tests/build_tests/super_errors/fixtures/polyvariant_constructors_mismatch_second.res create mode 100644 tests/build_tests/super_errors/fixtures/polyvariants_no_overlap.res diff --git a/compiler/ml/error_message_utils.ml b/compiler/ml/error_message_utils.ml index 478d73ac0b..1505c5699b 100644 --- a/compiler/ml/error_message_utils.ml +++ b/compiler/ml/error_message_utils.ml @@ -493,11 +493,22 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf @{?%s@}" (Parser.extract_text_at_loc loc) | _, Some (t1, t2) -> - let is_subtype = - try - Ctype.subtype env t1 t2 (); - true - with _ -> false + let can_show_coercion_message = + match (t1.desc, t2.desc) with + | Tvariant _, Tvariant _ -> + (* Subtyping polymorphic variants give some weird messages sometimes, + so let's turn it off for now. For an example, turn them on again and try: + ``` + let a: [#Resize | #KeyDown] = #Resize + let b: [#Click] = a + ``` + *) + false + | _ -> ( + 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 @@ -519,7 +530,7 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf | _ -> false in - if is_subtype && not is_constant then ( + if can_show_coercion_message && not is_constant then ( fprintf ppf "@,\ @,\ diff --git a/compiler/ml/printtyp.ml b/compiler/ml/printtyp.ml index 98142b4a93..9766e9ae87 100644 --- a/compiler/ml/printtyp.ml +++ b/compiler/ml/printtyp.ml @@ -1385,17 +1385,52 @@ let explanation unif t3 t4 ppf = (row1.row_fields, row1.row_closed, row2.row_fields, row2.row_closed) with | [], true, [], true -> - fprintf ppf "@,These two variant types have no intersection" + fprintf ppf + "@,\ + @,\ + These polymorphic variants are incompatible - they share no common \ + constructors." | [], true, (_ :: _ as fields), _ -> + let constructors_txt = + if List.length fields = 1 then "constructor" else "constructors" + in fprintf ppf - "@,@[The first variant type does not allow tag(s)@ @[%a@]@]" - print_tags fields + "@,\ + @,\ + The first polymorphic variant is @{closed@} and doesn't include \ + the %s: @{%a@}.@,\ + @,\ + Possible solutions:\n\ + \ - Either make the first variant @{open@} so it can accept \ + constructors it hasn't listed itself. To do this, make sure the type \ + starts with @{[>@} instead of @{[@}\n\ + \ - Or add the missing %s to it." + constructors_txt print_tags fields constructors_txt | (_ :: _ as fields), _, [], true -> + let constructors_txt = + if List.length fields = 1 then "constructor" else "constructors" + in fprintf ppf - "@,@[The second variant type does not allow tag(s)@ @[%a@]@]" - print_tags fields + "@,\ + @,\ + The second polymorphic variant is @{closed@} and doesn't \ + include the %s: @{%a@}.@,\ + @,\ + Possible solutions:\n\ + \ - Either make the second variant @{open@} so it can accept \ + constructors it hasn't listed itself. To do this, make sure the type \ + starts with @{[>@} instead of @{[@}\n\ + \ - Or add the missing %s to it." + constructors_txt print_tags fields constructors_txt | [(l1, _)], true, [(l2, _)], true when l1 = l2 -> - fprintf ppf "@,Types for tag %s are incompatible" + fprintf ppf + "@,\ + @,\ + Both polymorphic variants have the constructor @{%s@}, but \ + their payload types are incompatible.@,\ + Make sure the payload types for @{%s@} match exactly in both \ + polymorphic variants." + (!print_res_poly_identifier l1) (!print_res_poly_identifier l1) | _ -> ()) | _ -> () diff --git a/tests/build_tests/super_errors/expected/polyvariant_constructor_payload_mismatch.res.expected b/tests/build_tests/super_errors/expected/polyvariant_constructor_payload_mismatch.res.expected new file mode 100644 index 0000000000..48eca04f25 --- /dev/null +++ b/tests/build_tests/super_errors/expected/polyvariant_constructor_payload_mismatch.res.expected @@ -0,0 +1,16 @@ + + We've found a bug for you! + /.../fixtures/polyvariant_constructor_payload_mismatch.res:4:16 + + 2 │ type test2 = [#Click(string)] + 3 │ let a: test = #Click(1) + 4 │ let b: test2 = a + 5 │ + + This has type: test + But it's expected to have type: test2 + + Both polymorphic variants have the constructor #Click, but their payload types are incompatible. + Make sure the payload types for #Click match exactly in both polymorphic variants. + + You can convert int to string with Int.toString. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/polyvariant_constructors_mismatch_second.res.expected b/tests/build_tests/super_errors/expected/polyvariant_constructors_mismatch_second.res.expected new file mode 100644 index 0000000000..0494e8d394 --- /dev/null +++ b/tests/build_tests/super_errors/expected/polyvariant_constructors_mismatch_second.res.expected @@ -0,0 +1,17 @@ + + We've found a bug for you! + /.../fixtures/polyvariant_constructors_mismatch_second.res:7:16-22 + + 5 │ } + 6 │ + 7 │ let _ = handle(#Resize) + 8 │ + + This has type: [> #Resize] + But this function argument is expecting: [#Click | #KeyDown] + + The second polymorphic variant is closed and doesn't include the constructor: #Resize. + + Possible solutions: + - Either make the second variant open so it can accept constructors it hasn't listed itself. To do this, make sure the type starts with [> instead of [ + - Or add the missing constructor to it. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/polyvariant_name_formatting.res.expected b/tests/build_tests/super_errors/expected/polyvariant_name_formatting.res.expected index 6cd099ed47..ef5fbf8c8c 100644 --- a/tests/build_tests/super_errors/expected/polyvariant_name_formatting.res.expected +++ b/tests/build_tests/super_errors/expected/polyvariant_name_formatting.res.expected @@ -10,4 +10,9 @@ This pattern matches values of type [? #Invalid] but a pattern was expected which matches values of type polyvariant - The second variant type does not allow tag(s) #Invalid \ No newline at end of file + + The second polymorphic variant is closed and doesn't include the constructor: #Invalid. + + Possible solutions: + - Either make the second variant open so it can accept constructors it hasn't listed itself. To do this, make sure the type starts with [> instead of [ + - Or add the missing constructor to it. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/polyvariants_no_overlap.res.expected b/tests/build_tests/super_errors/expected/polyvariants_no_overlap.res.expected new file mode 100644 index 0000000000..e83ee885ca --- /dev/null +++ b/tests/build_tests/super_errors/expected/polyvariants_no_overlap.res.expected @@ -0,0 +1,12 @@ + + We've found a bug for you! + /.../fixtures/polyvariants_no_overlap.res:2:19 + + 1 │ let a: [#Resize | #KeyDown] = #Resize + 2 │ let b: [#Click] = a + 3 │ + + This has type: [#KeyDown | #Resize] + But it's expected to have type: [#Click] + + These polymorphic variants are incompatible - they share no common constructors. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/polyvariant_constructor_payload_mismatch.res b/tests/build_tests/super_errors/fixtures/polyvariant_constructor_payload_mismatch.res new file mode 100644 index 0000000000..df16786659 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/polyvariant_constructor_payload_mismatch.res @@ -0,0 +1,4 @@ +type test = [#Click(int)] +type test2 = [#Click(string)] +let a: test = #Click(1) +let b: test2 = a diff --git a/tests/build_tests/super_errors/fixtures/polyvariant_constructors_mismatch_second.res b/tests/build_tests/super_errors/fixtures/polyvariant_constructors_mismatch_second.res new file mode 100644 index 0000000000..d8eace4719 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/polyvariant_constructors_mismatch_second.res @@ -0,0 +1,7 @@ +let handle = (ev: [#Click | #KeyDown]) => + switch ev { + | #Click => Js.log("clicked") + | #KeyDown => Js.log("key down") + } + +let _ = handle(#Resize) diff --git a/tests/build_tests/super_errors/fixtures/polyvariants_no_overlap.res b/tests/build_tests/super_errors/fixtures/polyvariants_no_overlap.res new file mode 100644 index 0000000000..9971036067 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/polyvariants_no_overlap.res @@ -0,0 +1,2 @@ +let a: [#Resize | #KeyDown] = #Resize +let b: [#Click] = a From ce9982828739efcf5ade09e4b17280c8fee3d874 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 3 Jul 2025 21:53:48 +0200 Subject: [PATCH 2/4] add note --- compiler/ml/printtyp.ml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/ml/printtyp.ml b/compiler/ml/printtyp.ml index 9766e9ae87..b9299f56a4 100644 --- a/compiler/ml/printtyp.ml +++ b/compiler/ml/printtyp.ml @@ -1391,6 +1391,8 @@ let explanation unif t3 t4 ppf = These polymorphic variants are incompatible - they share no common \ constructors." | [], true, (_ :: _ as fields), _ -> + (* TODO(ai) Future opportunity to provide a way for an LLM to lookup the + full polyvariant type definitions if wanted.*) let constructors_txt = if List.length fields = 1 then "constructor" else "constructors" in From 1b88f550210247905018de92227e5b3fa55729cf Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 3 Jul 2025 21:56:28 +0200 Subject: [PATCH 3/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d70ae26fe..2b807e364d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ - Better error message for when trying to await something that is not a promise. https://github.com/rescript-lang/rescript/pull/7561 - Better error messages for object field missing and object field type mismatches. https://github.com/rescript-lang/rescript/pull/7580 +- Better error messages for when polymorphic variants does not match for various reasons. https://github.com/rescript-lang/rescript/pull/7596 #### :house: Internal From 438c041fc59a0370147fd7600f2f14a97ed07c66 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 4 Jul 2025 16:49:23 +0200 Subject: [PATCH 4/4] wording --- compiler/ml/printtyp.ml | 8 ++++---- .../polyvariant_constructors_mismatch_second.res.expected | 2 +- .../expected/polyvariant_name_formatting.res.expected | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/ml/printtyp.ml b/compiler/ml/printtyp.ml index b9299f56a4..8d04c0fccd 100644 --- a/compiler/ml/printtyp.ml +++ b/compiler/ml/printtyp.ml @@ -1404,8 +1404,8 @@ let explanation unif t3 t4 ppf = @,\ Possible solutions:\n\ \ - Either make the first variant @{open@} so it can accept \ - constructors it hasn't listed itself. To do this, make sure the type \ - starts with @{[>@} instead of @{[@}\n\ + additional constructors. To do this, make sure the type starts with \ + @{[>@} instead of @{[@}\n\ \ - Or add the missing %s to it." constructors_txt print_tags fields constructors_txt | (_ :: _ as fields), _, [], true -> @@ -1420,8 +1420,8 @@ let explanation unif t3 t4 ppf = @,\ Possible solutions:\n\ \ - Either make the second variant @{open@} so it can accept \ - constructors it hasn't listed itself. To do this, make sure the type \ - starts with @{[>@} instead of @{[@}\n\ + additional constructors. To do this, make sure the type starts with \ + @{[>@} instead of @{[@}\n\ \ - Or add the missing %s to it." constructors_txt print_tags fields constructors_txt | [(l1, _)], true, [(l2, _)], true when l1 = l2 -> diff --git a/tests/build_tests/super_errors/expected/polyvariant_constructors_mismatch_second.res.expected b/tests/build_tests/super_errors/expected/polyvariant_constructors_mismatch_second.res.expected index 0494e8d394..16ca255a6e 100644 --- a/tests/build_tests/super_errors/expected/polyvariant_constructors_mismatch_second.res.expected +++ b/tests/build_tests/super_errors/expected/polyvariant_constructors_mismatch_second.res.expected @@ -13,5 +13,5 @@ The second polymorphic variant is closed and doesn't include the constructor: #Resize. Possible solutions: - - Either make the second variant open so it can accept constructors it hasn't listed itself. To do this, make sure the type starts with [> instead of [ + - Either make the second variant open so it can accept additional constructors. To do this, make sure the type starts with [> instead of [ - Or add the missing constructor to it. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/polyvariant_name_formatting.res.expected b/tests/build_tests/super_errors/expected/polyvariant_name_formatting.res.expected index ef5fbf8c8c..25cbe0b2ca 100644 --- a/tests/build_tests/super_errors/expected/polyvariant_name_formatting.res.expected +++ b/tests/build_tests/super_errors/expected/polyvariant_name_formatting.res.expected @@ -14,5 +14,5 @@ The second polymorphic variant is closed and doesn't include the constructor: #Invalid. Possible solutions: - - Either make the second variant open so it can accept constructors it hasn't listed itself. To do this, make sure the type starts with [> instead of [ + - Either make the second variant open so it can accept additional constructors. To do this, make sure the type starts with [> instead of [ - Or add the missing constructor to it. \ No newline at end of file