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 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..8d04c0fccd 100644 --- a/compiler/ml/printtyp.ml +++ b/compiler/ml/printtyp.ml @@ -1385,17 +1385,54 @@ 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), _ -> + (* 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 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 \ + 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 -> + 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 \ + 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 -> - 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..16ca255a6e --- /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 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 6cd099ed47..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 @@ -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 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/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