From 6f972d7efd49541ae039e3a249e7926565a6add2 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 26 Sep 2023 22:18:46 +0200 Subject: [PATCH] Error messages: Improve "Somewhere wanted" messages (#6410) * first batch of changes trying to improve the Somewhere wanted error messages * tweaks * add more fixtures * remove redundant text * add specialized errors for array items and setting record fields * tweak set record field message * hint about tuple in array message * move message handling to own file * cleanup * cleanup * cleanup * cleanup * changelog * add example of tuple to error message --- CHANGELOG.md | 1 + .../array_item_type_mismatch.res.expected | 17 ++ .../expected/collections.res.expected | 2 +- .../expected/comparison_operator.res.expected | 13 ++ .../function_argument_mismatch.res.expected | 13 ++ .../function_call_mismatch.res.expected | 12 ++ .../function_return_mismatch.res.expected | 12 ++ .../expected/highlighting1.res.expected | 2 +- .../expected/highlighting2.res.expected | 2 +- .../expected/highlighting3.res.expected | 2 +- .../expected/highlighting5.res.expected | 2 +- .../expected/if_branch_mismatch.res.expected | 16 ++ .../if_condition_mismatch.res.expected | 12 ++ .../math_operator_constant.res.expected | 20 ++ .../expected/math_operator_float.res.expected | 20 ++ .../expected/math_operator_int.res.expected | 20 ++ .../expected/primitives1.res.expected | 10 +- .../expected/primitives11.res.expected | 2 +- .../expected/primitives2.res.expected | 2 +- .../expected/primitives6.res.expected | 2 +- .../expected/primitives7.res.expected | 2 +- .../expected/primitives9.res.expected | 2 +- .../record_type_spreads_deep_sub.res.expected | 2 +- .../set_record_field_type_match.res.expected | 13 ++ .../switch_different_types.res.expected | 14 ++ .../expected/switch_guard.res.expected | 14 ++ .../expected/syntaxErrors4.res.expected | 2 +- .../super_errors/expected/type1.res.expected | 10 +- .../super_errors/expected/type2.res.expected | 2 +- .../expected/unicode_location.res.expected | 2 +- .../expected/warnings1.res.expected | 4 +- .../expected/warnings2.res.expected | 2 +- .../fixtures/array_item_type_mismatch.res | 1 + .../fixtures/comparison_operator.res | 3 + .../fixtures/function_argument_mismatch.res | 3 + .../fixtures/function_call_mismatch.res | 8 + .../fixtures/function_return_mismatch.res | 10 + .../fixtures/if_branch_mismatch.res | 5 + .../fixtures/if_condition_mismatch.res | 3 + .../fixtures/math_operator_constant.res | 3 + .../fixtures/math_operator_float.res | 3 + .../fixtures/math_operator_int.res | 3 + .../fixtures/set_record_field_type_match.res | 11 + .../fixtures/switch_different_types.res | 9 + .../super_errors/fixtures/switch_guard.res | 9 + jscomp/ml/error_message_utils.ml | 199 ++++++++++++++++++ jscomp/ml/typecore.ml | 109 +++++----- jscomp/ml/typecore.mli | 2 +- 48 files changed, 558 insertions(+), 74 deletions(-) create mode 100644 jscomp/build_tests/super_errors/expected/array_item_type_mismatch.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/comparison_operator.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/function_argument_mismatch.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/function_call_mismatch.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/function_return_mismatch.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/if_branch_mismatch.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/if_condition_mismatch.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/math_operator_constant.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/math_operator_float.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/math_operator_int.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/set_record_field_type_match.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/switch_different_types.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/switch_guard.res.expected create mode 100644 jscomp/build_tests/super_errors/fixtures/array_item_type_mismatch.res create mode 100644 jscomp/build_tests/super_errors/fixtures/comparison_operator.res create mode 100644 jscomp/build_tests/super_errors/fixtures/function_argument_mismatch.res create mode 100644 jscomp/build_tests/super_errors/fixtures/function_call_mismatch.res create mode 100644 jscomp/build_tests/super_errors/fixtures/function_return_mismatch.res create mode 100644 jscomp/build_tests/super_errors/fixtures/if_branch_mismatch.res create mode 100644 jscomp/build_tests/super_errors/fixtures/if_condition_mismatch.res create mode 100644 jscomp/build_tests/super_errors/fixtures/math_operator_constant.res create mode 100644 jscomp/build_tests/super_errors/fixtures/math_operator_float.res create mode 100644 jscomp/build_tests/super_errors/fixtures/math_operator_int.res create mode 100644 jscomp/build_tests/super_errors/fixtures/set_record_field_type_match.res create mode 100644 jscomp/build_tests/super_errors/fixtures/switch_different_types.res create mode 100644 jscomp/build_tests/super_errors/fixtures/switch_guard.res create mode 100644 jscomp/ml/error_message_utils.ml diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a88a8c76c..e8015963a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ - A little performance improvement for JSX V4 runtime helper by removing one object allocation for components with key prop. https://github.com/rescript-lang/rescript-compiler/pull/6376 - The error message for "toplevel expressions should evaluate to unit" has been revamped and improved. https://github.com/rescript-lang/rescript-compiler/pull/6407 +- Improve "Somewhere wanted" error messages by changing wording and adding more context + suggested solutions to the error messages where appropriate. https://github.com/rescript-lang/rescript-compiler/pull/6410 # 11.0.0-rc.3 diff --git a/jscomp/build_tests/super_errors/expected/array_item_type_mismatch.res.expected b/jscomp/build_tests/super_errors/expected/array_item_type_mismatch.res.expected new file mode 100644 index 0000000000..54c99ad256 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/array_item_type_mismatch.res.expected @@ -0,0 +1,17 @@ + + We've found a bug for you! + /.../fixtures/array_item_type_mismatch.res:1:16-22 + + 1 │ let x = [1, 2, "hello"] + 2 │ + + This array item has type: string + But this array is expected to have items of type: int + + Arrays can only contain items of the same type. + + Possible solutions: + - Convert all values in the array to the same type. + - Use a tuple, if your array is of fixed length. Tuples can mix types freely, and compiles to a JavaScript array. Example of a tuple: `let myTuple = (10, "hello", 15.5, true) + + You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/collections.res.expected b/jscomp/build_tests/super_errors/expected/collections.res.expected index ef09145333..41b190b6f8 100644 --- a/jscomp/build_tests/super_errors/expected/collections.res.expected +++ b/jscomp/build_tests/super_errors/expected/collections.res.expected @@ -7,6 +7,6 @@ 3 │ This has type: string - Somewhere wanted: int + But it's expected to have type: int You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/comparison_operator.res.expected b/jscomp/build_tests/super_errors/expected/comparison_operator.res.expected new file mode 100644 index 0000000000..b2988d3431 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/comparison_operator.res.expected @@ -0,0 +1,13 @@ + + We've found a bug for you! + /.../fixtures/comparison_operator.res:3:17 + + 1 │ let f = Some(0) + 2 │ + 3 │ let x = 100 === f + 4 │ + + This has type: option + But it's being compared to something of type: int + + You can only compare things of the same type. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/function_argument_mismatch.res.expected b/jscomp/build_tests/super_errors/expected/function_argument_mismatch.res.expected new file mode 100644 index 0000000000..3905f123f6 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/function_argument_mismatch.res.expected @@ -0,0 +1,13 @@ + + We've found a bug for you! + /.../fixtures/function_argument_mismatch.res:3:28-30 + + 1 │ let makeName = (s, i) => s ++ i + 2 │ + 3 │ let name = makeName("123", 123) + 4 │ + + This has type: int + But this function argument is expecting: string + + You can convert int to string with Belt.Int.toString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/function_call_mismatch.res.expected b/jscomp/build_tests/super_errors/expected/function_call_mismatch.res.expected new file mode 100644 index 0000000000..d3cee635d3 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/function_call_mismatch.res.expected @@ -0,0 +1,12 @@ + + We've found a bug for you! + /.../fixtures/function_call_mismatch.res:6:3-10 + + 4 │ + 5 │ let cloneInTemp = (temp: string): string => { + 6 │ cd(temp) + 7 │ exec("git clone git@github.com:myorg/myrepo.git") + 8 │ } + + This function call returns: string + But it's expected to return: unit \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/function_return_mismatch.res.expected b/jscomp/build_tests/super_errors/expected/function_return_mismatch.res.expected new file mode 100644 index 0000000000..dc0dcc30a0 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/function_return_mismatch.res.expected @@ -0,0 +1,12 @@ + + We've found a bug for you! + /.../fixtures/function_return_mismatch.res:9:3-5 + + 7 │ + 8 │ let x = fnExpectingCleanup(() => { + 9 │ 123 + 10 │ }) + 11 │ + + This has type: int + But it's expected to have type: cleanup (defined as unit => unit) \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/highlighting1.res.expected b/jscomp/build_tests/super_errors/expected/highlighting1.res.expected index b6efe46d31..3e79ccf230 100644 --- a/jscomp/build_tests/super_errors/expected/highlighting1.res.expected +++ b/jscomp/build_tests/super_errors/expected/highlighting1.res.expected @@ -8,6 +8,6 @@ 4 │ This has type: string - Somewhere wanted: int + But it's expected to have type: int You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/highlighting2.res.expected b/jscomp/build_tests/super_errors/expected/highlighting2.res.expected index 28cd73a5af..ddce9938c6 100644 --- a/jscomp/build_tests/super_errors/expected/highlighting2.res.expected +++ b/jscomp/build_tests/super_errors/expected/highlighting2.res.expected @@ -9,6 +9,6 @@ 5 ┆ This has type: string - Somewhere wanted: int + But it's expected to have type: int You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/highlighting3.res.expected b/jscomp/build_tests/super_errors/expected/highlighting3.res.expected index 04a948db88..ad49ca150d 100644 --- a/jscomp/build_tests/super_errors/expected/highlighting3.res.expected +++ b/jscomp/build_tests/super_errors/expected/highlighting3.res.expected @@ -9,6 +9,6 @@ 5 │ This has type: string - Somewhere wanted: int + But it's expected to have type: int You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/highlighting5.res.expected b/jscomp/build_tests/super_errors/expected/highlighting5.res.expected index 5db1a55595..6e6ab03a30 100644 --- a/jscomp/build_tests/super_errors/expected/highlighting5.res.expected +++ b/jscomp/build_tests/super_errors/expected/highlighting5.res.expected @@ -8,6 +8,6 @@ 3 │ This has type: string - Somewhere wanted: int + But it's expected to have type: int You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/if_branch_mismatch.res.expected b/jscomp/build_tests/super_errors/expected/if_branch_mismatch.res.expected new file mode 100644 index 0000000000..714a6f074f --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/if_branch_mismatch.res.expected @@ -0,0 +1,16 @@ + + We've found a bug for you! + /.../fixtures/if_branch_mismatch.res:4:3-5 + + 2 │ "123" + 3 │ } else { + 4 │ 123 + 5 │ } + 6 │ + + This has type: int + But this if statement is expected to return: string + + if expressions must return the same type in all branches (if, else if, else). + + You can convert int to string with Belt.Int.toString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/if_condition_mismatch.res.expected b/jscomp/build_tests/super_errors/expected/if_condition_mismatch.res.expected new file mode 100644 index 0000000000..f81ffca952 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/if_condition_mismatch.res.expected @@ -0,0 +1,12 @@ + + We've found a bug for you! + /.../fixtures/if_condition_mismatch.res:1:12-18 + + 1 │ let x = if "horse" { + 2 │ () + 3 │ } + + This has type: string + But if conditions must always be of type: bool + + To fix this, change the highlighted code so it evaluates to a bool. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/math_operator_constant.res.expected b/jscomp/build_tests/super_errors/expected/math_operator_constant.res.expected new file mode 100644 index 0000000000..f2251eee15 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/math_operator_constant.res.expected @@ -0,0 +1,20 @@ + + We've found a bug for you! + /.../fixtures/math_operator_constant.res:3:15-17 + + 1 │ let num = 0 + 2 │ + 3 │ let x = num + 12. + 4 │ + + This value has type: float + But it's being used with the + operator, which works on: int + + Floats and ints have their own mathematical operators. This means you cannot add a float and an int without converting between the two. + + Possible solutions: + - Ensure all values in this calculation has the type int. You can convert between floats and ints via Belt.Float.toInt and Belt.Int.fromFloat. + - Make 12. an int by removing the dot or explicitly converting to int + + You can convert float to int with Belt.Float.toInt. + If this is a literal, try a number without a trailing dot (e.g. 20). \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/math_operator_float.res.expected b/jscomp/build_tests/super_errors/expected/math_operator_float.res.expected new file mode 100644 index 0000000000..cd0fffa1c1 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/math_operator_float.res.expected @@ -0,0 +1,20 @@ + + We've found a bug for you! + /.../fixtures/math_operator_float.res:3:9-11 + + 1 │ let num = 0 + 2 │ + 3 │ let x = num +. 12. + 4 │ + + This has type: int + But it's being used with the +. operator, which works on: float + + Floats and ints have their own mathematical operators. This means you cannot add a float and an int without converting between the two. + + Possible solutions: + - Ensure all values in this calculation has the type float. You can convert between floats and ints via Belt.Float.toInt and Belt.Int.fromFloat. + - Change the operator to +, which works on int + + You can convert int to float with Belt.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/jscomp/build_tests/super_errors/expected/math_operator_int.res.expected b/jscomp/build_tests/super_errors/expected/math_operator_int.res.expected new file mode 100644 index 0000000000..ebccfbecb7 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/math_operator_int.res.expected @@ -0,0 +1,20 @@ + + We've found a bug for you! + /.../fixtures/math_operator_int.res:3:9-11 + + 1 │ let num = 0. + 2 │ + 3 │ let x = num + 12. + 4 │ + + This has type: float + But it's being used with the + operator, which works on: int + + Floats and ints have their own mathematical operators. This means you cannot add a float and an int without converting between the two. + + Possible solutions: + - Ensure all values in this calculation has the type int. You can convert between floats and ints via Belt.Float.toInt and Belt.Int.fromFloat. + - Change the operator to +., which works on float + + You can convert float to int with Belt.Float.toInt. + If this is a literal, try a number without a trailing dot (e.g. 20). \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/primitives1.res.expected b/jscomp/build_tests/super_errors/expected/primitives1.res.expected index b262b9eae4..b1a303eccb 100644 --- a/jscomp/build_tests/super_errors/expected/primitives1.res.expected +++ b/jscomp/build_tests/super_errors/expected/primitives1.res.expected @@ -6,8 +6,14 @@ 2 │ 2. + 2 3 │ - This has type: float - Somewhere wanted: int + This value has type: float + But it's being used with the + operator, which works on: int + + Floats and ints have their own mathematical operators. This means you cannot add a float and an int without converting between the two. + + Possible solutions: + - Ensure all values in this calculation has the type int. You can convert between floats and ints via Belt.Float.toInt and Belt.Int.fromFloat. + - Make 2. an int by removing the dot or explicitly converting to int You can convert float to int with Belt.Float.toInt. If this is a literal, try a number without a trailing dot (e.g. 20). \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/primitives11.res.expected b/jscomp/build_tests/super_errors/expected/primitives11.res.expected index 00803de068..6123d58b2c 100644 --- a/jscomp/build_tests/super_errors/expected/primitives11.res.expected +++ b/jscomp/build_tests/super_errors/expected/primitives11.res.expected @@ -8,7 +8,7 @@ 6 │ This has type: b (defined as option) - Somewhere wanted: a (defined as option) + But it's expected to have type: a (defined as option) The incompatible parts: bb (defined as option) vs aa (defined as option) diff --git a/jscomp/build_tests/super_errors/expected/primitives2.res.expected b/jscomp/build_tests/super_errors/expected/primitives2.res.expected index 08de7d5219..8677b188ff 100644 --- a/jscomp/build_tests/super_errors/expected/primitives2.res.expected +++ b/jscomp/build_tests/super_errors/expected/primitives2.res.expected @@ -7,6 +7,6 @@ 3 │ This has type: int - Somewhere wanted: string + But this function argument is expecting: string You can convert int to string with Belt.Int.toString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/primitives6.res.expected b/jscomp/build_tests/super_errors/expected/primitives6.res.expected index 7ca89f6a34..6e46bb560a 100644 --- a/jscomp/build_tests/super_errors/expected/primitives6.res.expected +++ b/jscomp/build_tests/super_errors/expected/primitives6.res.expected @@ -8,7 +8,7 @@ 4 │ This has type: int - Somewhere wanted: float + But it's expected to have type: float You can convert int to float with Belt.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/jscomp/build_tests/super_errors/expected/primitives7.res.expected b/jscomp/build_tests/super_errors/expected/primitives7.res.expected index b0038464c5..0f7375f708 100644 --- a/jscomp/build_tests/super_errors/expected/primitives7.res.expected +++ b/jscomp/build_tests/super_errors/expected/primitives7.res.expected @@ -8,7 +8,7 @@ 4 │ This has type: list - Somewhere wanted: list + But this function argument is expecting: list The incompatible parts: int vs float diff --git a/jscomp/build_tests/super_errors/expected/primitives9.res.expected b/jscomp/build_tests/super_errors/expected/primitives9.res.expected index 110a085ce7..b97f1b023d 100644 --- a/jscomp/build_tests/super_errors/expected/primitives9.res.expected +++ b/jscomp/build_tests/super_errors/expected/primitives9.res.expected @@ -6,6 +6,6 @@ 2 │ This has type: string - Somewhere wanted: int + But it's expected to have type: int You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/record_type_spreads_deep_sub.res.expected b/jscomp/build_tests/super_errors/expected/record_type_spreads_deep_sub.res.expected index da6b670d0d..cdbe9281a9 100644 --- a/jscomp/build_tests/super_errors/expected/record_type_spreads_deep_sub.res.expected +++ b/jscomp/build_tests/super_errors/expected/record_type_spreads_deep_sub.res.expected @@ -9,6 +9,6 @@ 10 │ This has type: string - Somewhere wanted: int + But it's expected to have type: int You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/set_record_field_type_match.res.expected b/jscomp/build_tests/super_errors/expected/set_record_field_type_match.res.expected new file mode 100644 index 0000000000..d45880d7d2 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/set_record_field_type_match.res.expected @@ -0,0 +1,13 @@ + + We've found a bug for you! + /.../fixtures/set_record_field_type_match.res:11:13-14 + + 9 │ } + 10 │ + 11 │ user.name = 12 + 12 │ + + You're assigning something to this field that has type: int + But this record field is of type: string + + You can convert int to string with Belt.Int.toString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/switch_different_types.res.expected b/jscomp/build_tests/super_errors/expected/switch_different_types.res.expected new file mode 100644 index 0000000000..14e96e8964 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/switch_different_types.res.expected @@ -0,0 +1,14 @@ + + We've found a bug for you! + /.../fixtures/switch_different_types.res:7:10-23 + + 5 │ switch foo { + 6 │ | "world" => () + 7 │ | _ => someFunction() + 8 │ } + 9 │ } + + This has type: string + But this switch is expected to return: unit + + All branches in a switch must return the same type. To fix this, change your branch to return the expected type. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/switch_guard.res.expected b/jscomp/build_tests/super_errors/expected/switch_guard.res.expected new file mode 100644 index 0000000000..f04ff7dd76 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/switch_guard.res.expected @@ -0,0 +1,14 @@ + + We've found a bug for you! + /.../fixtures/switch_guard.res:6:16-22 + + 4 │ let bar = () => { + 5 │ switch foo { + 6 │ | "world" if "horse" => () + 7 │ | _ => someFunction() + 8 │ } + + This has type: string + But if conditions must always be of type: bool + + To fix this, change the highlighted code so it evaluates to a bool. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/syntaxErrors4.res.expected b/jscomp/build_tests/super_errors/expected/syntaxErrors4.res.expected index 22f86e8282..5cf63f8107 100644 --- a/jscomp/build_tests/super_errors/expected/syntaxErrors4.res.expected +++ b/jscomp/build_tests/super_errors/expected/syntaxErrors4.res.expected @@ -13,6 +13,6 @@ 23 │ /* */ This has type: string - Somewhere wanted: int + But it's expected to have type: int You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/type1.res.expected b/jscomp/build_tests/super_errors/expected/type1.res.expected index aca8c5af6a..036daa2550 100644 --- a/jscomp/build_tests/super_errors/expected/type1.res.expected +++ b/jscomp/build_tests/super_errors/expected/type1.res.expected @@ -5,8 +5,14 @@ 1 │ let x = 2. + 2 2 │ - This has type: float - Somewhere wanted: int + This value has type: float + But it's being used with the + operator, which works on: int + + Floats and ints have their own mathematical operators. This means you cannot add a float and an int without converting between the two. + + Possible solutions: + - Ensure all values in this calculation has the type int. You can convert between floats and ints via Belt.Float.toInt and Belt.Int.fromFloat. + - Make 2. an int by removing the dot or explicitly converting to int You can convert float to int with Belt.Float.toInt. If this is a literal, try a number without a trailing dot (e.g. 20). \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/type2.res.expected b/jscomp/build_tests/super_errors/expected/type2.res.expected index 3e0bba689f..1b189ab2be 100644 --- a/jscomp/build_tests/super_errors/expected/type2.res.expected +++ b/jscomp/build_tests/super_errors/expected/type2.res.expected @@ -9,6 +9,6 @@ 8 │ This has type: string - Somewhere wanted: int + But this function argument is expecting: int You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/unicode_location.res.expected b/jscomp/build_tests/super_errors/expected/unicode_location.res.expected index e21c17cd39..a8e39cf3ca 100644 --- a/jscomp/build_tests/super_errors/expected/unicode_location.res.expected +++ b/jscomp/build_tests/super_errors/expected/unicode_location.res.expected @@ -7,6 +7,6 @@ │ (unicode symbols of length 2) This has type: int - Somewhere wanted: string + But this function argument is expecting: string You can convert int to string with Belt.Int.toString. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/warnings1.res.expected b/jscomp/build_tests/super_errors/expected/warnings1.res.expected index 56d98ff174..4d911382ed 100644 --- a/jscomp/build_tests/super_errors/expected/warnings1.res.expected +++ b/jscomp/build_tests/super_errors/expected/warnings1.res.expected @@ -8,5 +8,5 @@ 4 │ 10 5 │ } - This has type: int => int - Somewhere wanted: unit \ No newline at end of file + This function call returns: int => int + But it's expected to return: unit \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/warnings2.res.expected b/jscomp/build_tests/super_errors/expected/warnings2.res.expected index 5c3ab48512..324f794b04 100644 --- a/jscomp/build_tests/super_errors/expected/warnings2.res.expected +++ b/jscomp/build_tests/super_errors/expected/warnings2.res.expected @@ -8,4 +8,4 @@ 4 │ } This has type: int - Somewhere wanted: unit \ No newline at end of file + But it's expected to have type: unit \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/fixtures/array_item_type_mismatch.res b/jscomp/build_tests/super_errors/fixtures/array_item_type_mismatch.res new file mode 100644 index 0000000000..541be0e3ce --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/array_item_type_mismatch.res @@ -0,0 +1 @@ +let x = [1, 2, "hello"] diff --git a/jscomp/build_tests/super_errors/fixtures/comparison_operator.res b/jscomp/build_tests/super_errors/fixtures/comparison_operator.res new file mode 100644 index 0000000000..c9c4270d4f --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/comparison_operator.res @@ -0,0 +1,3 @@ +let f = Some(0) + +let x = 100 === f diff --git a/jscomp/build_tests/super_errors/fixtures/function_argument_mismatch.res b/jscomp/build_tests/super_errors/fixtures/function_argument_mismatch.res new file mode 100644 index 0000000000..895aa91ff9 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/function_argument_mismatch.res @@ -0,0 +1,3 @@ +let makeName = (s, i) => s ++ i + +let name = makeName("123", 123) diff --git a/jscomp/build_tests/super_errors/fixtures/function_call_mismatch.res b/jscomp/build_tests/super_errors/fixtures/function_call_mismatch.res new file mode 100644 index 0000000000..1d3bafce1a --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/function_call_mismatch.res @@ -0,0 +1,8 @@ +@module("shelljs") +external cd: string => string = "cd" +external exec: string => string = "exec" + +let cloneInTemp = (temp: string): string => { + cd(temp) + exec("git clone git@github.com:myorg/myrepo.git") +} diff --git a/jscomp/build_tests/super_errors/fixtures/function_return_mismatch.res b/jscomp/build_tests/super_errors/fixtures/function_return_mismatch.res new file mode 100644 index 0000000000..7907de2b39 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/function_return_mismatch.res @@ -0,0 +1,10 @@ +type cleanup = unit => unit + +let fnExpectingCleanup = (cb: unit => cleanup) => { + let cleanup = cb() + cleanup() +} + +let x = fnExpectingCleanup(() => { + 123 +}) diff --git a/jscomp/build_tests/super_errors/fixtures/if_branch_mismatch.res b/jscomp/build_tests/super_errors/fixtures/if_branch_mismatch.res new file mode 100644 index 0000000000..19112ea047 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/if_branch_mismatch.res @@ -0,0 +1,5 @@ +let x = if true { + "123" +} else { + 123 +} diff --git a/jscomp/build_tests/super_errors/fixtures/if_condition_mismatch.res b/jscomp/build_tests/super_errors/fixtures/if_condition_mismatch.res new file mode 100644 index 0000000000..221d18bc70 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/if_condition_mismatch.res @@ -0,0 +1,3 @@ +let x = if "horse" { + () +} diff --git a/jscomp/build_tests/super_errors/fixtures/math_operator_constant.res b/jscomp/build_tests/super_errors/fixtures/math_operator_constant.res new file mode 100644 index 0000000000..3934ae091e --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/math_operator_constant.res @@ -0,0 +1,3 @@ +let num = 0 + +let x = num + 12. diff --git a/jscomp/build_tests/super_errors/fixtures/math_operator_float.res b/jscomp/build_tests/super_errors/fixtures/math_operator_float.res new file mode 100644 index 0000000000..4e791f2f20 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/math_operator_float.res @@ -0,0 +1,3 @@ +let num = 0 + +let x = num +. 12. diff --git a/jscomp/build_tests/super_errors/fixtures/math_operator_int.res b/jscomp/build_tests/super_errors/fixtures/math_operator_int.res new file mode 100644 index 0000000000..cc57609be4 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/math_operator_int.res @@ -0,0 +1,3 @@ +let num = 0. + +let x = num + 12. diff --git a/jscomp/build_tests/super_errors/fixtures/set_record_field_type_match.res b/jscomp/build_tests/super_errors/fixtures/set_record_field_type_match.res new file mode 100644 index 0000000000..f280f42300 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/set_record_field_type_match.res @@ -0,0 +1,11 @@ +type record = { + mutable name: string, + age: int, +} + +let user = { + name: "Test", + age: 99, +} + +user.name = 12 diff --git a/jscomp/build_tests/super_errors/fixtures/switch_different_types.res b/jscomp/build_tests/super_errors/fixtures/switch_different_types.res new file mode 100644 index 0000000000..de9a693b92 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/switch_different_types.res @@ -0,0 +1,9 @@ +@val external foo: string = "foo" +external someFunction: unit => string = "someFunction" + +let bar = () => { + switch foo { + | "world" => () + | _ => someFunction() + } +} diff --git a/jscomp/build_tests/super_errors/fixtures/switch_guard.res b/jscomp/build_tests/super_errors/fixtures/switch_guard.res new file mode 100644 index 0000000000..7c3786b4b3 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/switch_guard.res @@ -0,0 +1,9 @@ +@val external foo: string = "foo" +external someFunction: unit => string = "someFunction" + +let bar = () => { + switch foo { + | "world" if "horse" => () + | _ => someFunction() + } +} diff --git a/jscomp/ml/error_message_utils.ml b/jscomp/ml/error_message_utils.ml new file mode 100644 index 0000000000..f7ab333bb8 --- /dev/null +++ b/jscomp/ml/error_message_utils.ml @@ -0,0 +1,199 @@ +type typeClashStatement = FunctionCall +type typeClashContext = + | SetRecordField + | ArrayValue + | FunctionReturn + | MaybeUnwrapOption + | IfCondition + | IfReturn + | Switch + | StringConcat + | ComparisonOperator + | MathOperator of { + forFloat: bool; + operator: string; + isConstant: string option; + } + | FunctionArgument + | Statement of typeClashStatement + +let fprintf = Format.fprintf + +let errorTypeText ppf typeClashContext = + let text = + match typeClashContext with + | Some (Statement FunctionCall) -> "This function call returns:" + | Some (MathOperator {isConstant = Some _}) -> "This value has type:" + | Some ArrayValue -> "This array item has type:" + | Some SetRecordField -> + "You're assigning something to this field that has type:" + | _ -> "This has type:" + in + fprintf ppf "%s" text + +let errorExpectedTypeText ppf typeClashContext = + match typeClashContext with + | Some FunctionArgument -> + fprintf ppf "But this function argument is expecting:" + | Some ComparisonOperator -> + fprintf ppf "But it's being compared to something of type:" + | Some Switch -> fprintf ppf "But this switch is expected to return:" + | Some IfCondition -> + fprintf ppf "But @{if@} conditions must always be of type:" + | Some IfReturn -> + fprintf ppf "But this @{if@} statement is expected to return:" + | Some ArrayValue -> + fprintf ppf "But this array is expected to have items of type:" + | Some SetRecordField -> fprintf ppf "But this record field is of type:" + | Some (Statement FunctionCall) -> fprintf ppf "But it's expected to return:" + | Some (MathOperator {operator}) -> + fprintf ppf + "But it's being used with the @{%s@} operator, which works on:" + operator + | Some FunctionReturn -> + fprintf ppf "But this function is expecting you to return:" + | _ -> fprintf ppf "But it's expected to have type:" + +let printExtraTypeClashHelp ppf trace typeClashContext = + match typeClashContext with + | Some (MathOperator {forFloat; operator; isConstant}) -> ( + let operatorForOtherType = + match operator with + | "+" -> "+." + | "+." -> "+" + | "/" -> "/." + | "/." -> "/" + | "-" -> "-." + | "*" -> "*." + | "*." -> "*" + | v -> v + in + let operatorText = + match operator.[0] with + | '+' -> "add" + | '-' -> "subtract" + | '/' -> "divide" + | '*' -> "multiply" + | _ -> "compute" + in + (* TODO check int vs float explicitly before showing this *) + fprintf ppf + "\n\n\ + \ Floats and ints have their own mathematical operators. This means you \ + cannot %s a float and an int without converting between the two.\n\n\ + \ Possible solutions:\n\ + \ - Ensure all values in this calculation has the type @{%s@}. \ + You can convert between floats and ints via @{Belt.Float.toInt@} \ + and @{Belt.Int.fromFloat@}." + operatorText + (if forFloat then "float" else "int"); + match (isConstant, trace) with + | Some constant, _ -> + if forFloat then + fprintf ppf + "\n\ + \ - Make @{%s@} a @{float@} by adding a trailing dot: \ + @{%s.@}" + constant constant + else + fprintf ppf + "\n\ + \ - Make @{%s@} an @{int@} by removing the dot or \ + explicitly converting to int" + constant + | ( _, + [ + ({Types.desc = Tconstr (p1, _, _)}, _); + ({desc = Tconstr (p2, _, _)}, _); + ] ) -> ( + match (Path.name p1, Path.name p2) with + | "float", "int" | "int", "float" -> + fprintf ppf + "\n\ + \ - Change the operator to @{%s@}, which works on @{%s@}" + operatorForOtherType + (if forFloat then "int" else "float") + | _ -> ()) + | _ -> ()) + | Some Switch -> + fprintf ppf + "\n\n\ + \ All branches in a @{switch@} must return the same type. To fix \ + this, change your branch to return the expected type." + | Some IfCondition -> + fprintf ppf + "\n\n\ + \ To fix this, change the highlighted code so it evaluates to a \ + @{bool@}." + | Some IfReturn -> + fprintf ppf + "\n\n\ + \ @{if@} expressions must return the same type in all branches \ + (@{if@}, @{else if@}, @{else@})." + | Some MaybeUnwrapOption -> + fprintf ppf + "\n\n\ + \ Possible solutions:\n\ + \ - Unwrap the option to its underlying value using \ + `yourValue->Belt.Option.getWithDefault(someDefaultValue)`" + | Some ComparisonOperator -> + fprintf ppf "\n\n You can only compare things of the same type." + | Some ArrayValue -> + fprintf ppf + "\n\n\ + \ Arrays can only contain items of the same type.\n\n\ + \ Possible solutions:\n\ + \ - Convert all values in the array to the same type.\n\ + \ - Use a tuple, if your array is of fixed length. Tuples can mix types \ + freely, and compiles to a JavaScript array. Example of a tuple: `let \ + myTuple = (10, \"hello\", 15.5, true)" + | _ -> () + +let typeClashContextFromFunction sexp sfunct = + let isConstant = + match sexp.Parsetree.pexp_desc with + | Pexp_constant (Pconst_integer (txt, _) | Pconst_float (txt, _)) -> + Some txt + | _ -> None + in + match sfunct.Parsetree.pexp_desc with + | Pexp_ident + {txt = Lident ("=" | "==" | "<>" | "!=" | ">" | ">=" | "<" | "<=")} -> + Some ComparisonOperator + | Pexp_ident {txt = Lident "++"} -> Some StringConcat + | Pexp_ident {txt = Lident (("/." | "*." | "+." | "-.") as operator)} -> + Some (MathOperator {forFloat = true; operator; isConstant}) + | Pexp_ident {txt = Lident (("/" | "*" | "+" | "-") as operator)} -> + Some (MathOperator {forFloat = false; operator; isConstant}) + | _ -> Some FunctionArgument + +let typeClashContextForFunctionArgument typeClashContext sarg0 = + match typeClashContext with + | Some (MathOperator {forFloat; operator}) -> + Some + (MathOperator + { + forFloat; + operator; + isConstant = + (match sarg0.Parsetree.pexp_desc with + | Pexp_constant (Pconst_integer (txt, _) | Pconst_float (txt, _)) + -> + Some txt + | _ -> None); + }) + | typeClashContext -> typeClashContext + +let typeClashContextMaybeOption ty_expected ty_res = + match (ty_expected, ty_res) with + | ( {Types.desc = Tconstr (expectedPath, _, _)}, + {Types.desc = Tconstr (typePath, _, _)} ) + when Path.same Predef.path_option typePath + && Path.same expectedPath Predef.path_option = false -> + Some MaybeUnwrapOption + | _ -> None + +let typeClashContextInStatement sexp = + match sexp.Parsetree.pexp_desc with + | Pexp_apply _ -> Some (Statement FunctionCall) + | _ -> None diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 2c1e471ce7..111ac9fe9e 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -22,6 +22,7 @@ open Types open Typedtree open Btype open Ctype +open Error_message_utils type error = Polymorphic_label of Longident.t @@ -31,7 +32,7 @@ type error = | Or_pattern_type_clash of Ident.t * (type_expr * type_expr) list | Multiply_bound_variable of string | Orpat_vars of Ident.t * Ident.t list - | Expr_type_clash of (type_expr * type_expr) list + | Expr_type_clash of (type_expr * type_expr) list * (typeClashContext option) | Apply_non_function of type_expr | Apply_wrong_label of arg_label * type_expr | Label_multiply_defined of string @@ -335,14 +336,14 @@ let unify_pat_types loc env ty ty' = raise(Typetexp.Error(loc, env, Typetexp.Variant_tags (l1, l2))) (* unification inside type_exp and type_expect *) -let unify_exp_types loc env ty expected_ty = +let unify_exp_types ?typeClashContext loc env ty expected_ty = (* Format.eprintf "@[%a@ %a@]@." Printtyp.raw_type_expr exp.exp_type Printtyp.raw_type_expr expected_ty; *) try unify env ty expected_ty with Unify trace -> - raise(Error(loc, env, Expr_type_clash(trace))) + raise(Error(loc, env, Expr_type_clash(trace, typeClashContext))) | Tags(l1,l2) -> raise(Typetexp.Error(loc, env, Typetexp.Variant_tags (l1, l2))) @@ -663,7 +664,7 @@ let rec collect_missing_arguments env type1 type2 = match type1 with end | _ -> None -let print_expr_type_clash env trace ppf = begin +let print_expr_type_clash ?typeClashContext env trace ppf = begin (* this is the most frequent error. We should do whatever we can to provide specific guidance to this generic error before giving up *) let bottom_aliases_result = bottom_aliases trace in @@ -711,9 +712,10 @@ let print_expr_type_clash env trace ppf = begin Printtyp.super_report_unification_error ppf env trace (function ppf -> - fprintf ppf "This has type:") + errorTypeText ppf typeClashContext) (function ppf -> - fprintf ppf "Somewhere wanted:"); + errorExpectedTypeText ppf typeClashContext); + printExtraTypeClashHelp ppf trace typeClashContext; show_extra_help ppf env trace; end @@ -1690,7 +1692,7 @@ let rec type_approx env sexp = let ty = type_approx env e in let ty1 = approx_type env sty in begin try unify env ty ty1 with Unify trace -> - raise(Error(sexp.pexp_loc, env, Expr_type_clash trace)) + raise(Error(sexp.pexp_loc, env, Expr_type_clash (trace, None))) end; ty1 | Pexp_coerce (e, sty1, sty2) -> @@ -1702,7 +1704,7 @@ let rec type_approx env sexp = and ty1 = approx_ty_opt sty1 and ty2 = approx_type env sty2 in begin try unify env ty ty1 with Unify trace -> - raise(Error(sexp.pexp_loc, env, Expr_type_clash trace)) + raise(Error(sexp.pexp_loc, env, Expr_type_clash (trace, None))) end; ty2 | _ -> newvar () @@ -1932,9 +1934,9 @@ let rec name_pattern default = function (* Typing of expressions *) -let unify_exp env exp expected_ty = +let unify_exp ?typeClashContext env exp expected_ty = let loc = proper_exp_loc exp in - unify_exp_types loc env exp.exp_type expected_ty + unify_exp_types ?typeClashContext loc env exp.exp_type expected_ty let is_ignore funct env = @@ -1984,23 +1986,23 @@ let rec type_exp ?recarg env sexp = In the principal case, [type_expected'] may be at generic_level. *) -and type_expect ?in_function ?recarg env sexp ty_expected = +and type_expect ?typeClashContext ?in_function ?recarg env sexp ty_expected = let previous_saved_types = Cmt_format.get_saved_types () in let exp = Builtin_attributes.warning_scope sexp.pexp_attributes (fun () -> - type_expect_ ?in_function ?recarg env sexp ty_expected + type_expect_ ?typeClashContext ?in_function ?recarg env sexp ty_expected ) in Cmt_format.set_saved_types (Cmt_format.Partial_expression exp :: previous_saved_types); exp -and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = +and type_expect_ ?typeClashContext ?in_function ?(recarg=Rejected) env sexp ty_expected = let loc = sexp.pexp_loc in (* Record the expression type before unifying it with the expected type *) let rue exp = - unify_exp env (re exp) (instance env ty_expected); + unify_exp ?typeClashContext env (re exp) (instance env ty_expected); exp in let process_optional_label (id, ld, e) = @@ -2138,7 +2140,8 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = Ext_list.exists sexp.pexp_attributes (fun ({txt },_) -> txt = "res.uapp") && not @@ Ext_list.exists sexp.pexp_attributes (fun ({txt },_) -> txt = "res.partial") && not @@ is_automatic_curried_application env funct in - let (args, ty_res, fully_applied) = type_application uncurried env funct sargs in + let typeClashContext = typeClashContextFromFunction sexp sfunct in + let (args, ty_res, fully_applied) = type_application ?typeClashContext uncurried env funct sargs in end_def (); unify_var env (newvar()) funct.exp_type; @@ -2190,9 +2193,9 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = empty pattern matching can be generated by Camlp4 with its revised syntax. Let's accept it for backward compatibility. *) let val_cases, partial = - type_cases env arg.exp_type ty_expected true loc val_caselist in + type_cases ~rootTypeClashContext:Switch env arg.exp_type ty_expected true loc val_caselist in let exn_cases, _ = - type_cases env Predef.type_exn ty_expected false loc exn_caselist in + type_cases ~rootTypeClashContext:Switch env Predef.type_exn ty_expected false loc exn_caselist in re { exp_desc = Texp_match(arg, val_cases, exn_cases, partial); exp_loc = loc; exp_extra = []; @@ -2447,7 +2450,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = let (record, label, opath) = type_label_access env srecord lid in let ty_record = if opath = None then newvar () else record.exp_type in let (label_loc, label, newval) = - type_label_exp false env loc ty_record (lid, label, snewval) in + type_label_exp ~typeClashContext:SetRecordField false env loc ty_record (lid, label, snewval) in unify_exp env record ty_record; if label.lbl_mut = Immutable then raise(Error(loc, env, Label_not_mutable lid.txt)); @@ -2463,7 +2466,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = let ty = newgenvar() in let to_unify = Predef.type_array ty in unify_exp_types loc env to_unify ty_expected; - let argl = List.map (fun sarg -> type_expect env sarg ty) sargl in + let argl = List.map (fun sarg -> type_expect ~typeClashContext:ArrayValue env sarg ty) sargl in re { exp_desc = Texp_array argl; exp_loc = loc; exp_extra = []; @@ -2471,10 +2474,10 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = exp_attributes = sexp.pexp_attributes; exp_env = env } | Pexp_ifthenelse(scond, sifso, sifnot) -> - let cond = type_expect env scond Predef.type_bool in + let cond = type_expect ~typeClashContext:IfCondition env scond Predef.type_bool in begin match sifnot with None -> - let ifso = type_expect env sifso Predef.type_unit in + let ifso = type_expect ~typeClashContext:IfReturn env sifso Predef.type_unit in rue { exp_desc = Texp_ifthenelse(cond, ifso, None); exp_loc = loc; exp_extra = []; @@ -2482,10 +2485,10 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = exp_attributes = sexp.pexp_attributes; exp_env = env } | Some sifnot -> - let ifso = type_expect env sifso ty_expected in - let ifnot = type_expect env sifnot ty_expected in + let ifso = type_expect ~typeClashContext:IfReturn env sifso ty_expected in + let ifnot = type_expect ~typeClashContext:IfReturn env sifnot ty_expected in (* Keep sharing *) - unify_exp env ifnot ifso.exp_type; + unify_exp ~typeClashContext:IfReturn env ifnot ifso.exp_type; re { exp_desc = Texp_ifthenelse(cond, ifso, Some ifnot); exp_loc = loc; exp_extra = []; @@ -2573,7 +2576,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = let tv = newvar () in let gen = generalizable tv.level arg.exp_type in (try unify_var env tv arg.exp_type with Unify trace -> - raise(Error(arg.exp_loc, env, Expr_type_clash trace))); + raise(Error(arg.exp_loc, env, Expr_type_clash (trace, typeClashContext)))); gen end else true in @@ -2963,7 +2966,7 @@ and type_label_access env srecord lid = (* Typing format strings for printing or reading. These formats are used by functions in modules Printf, Format, and Scanf. (Handling of * modifiers contributed by Thorsten Ohl.) *) -and type_label_exp create env loc ty_expected +and type_label_exp ?typeClashContext create env loc ty_expected (lid, label, sarg) = (* Here also ty_expected may be at generic_level *) begin_def (); @@ -2995,7 +2998,7 @@ and type_label_exp create env loc ty_expected raise (Error(lid.loc, env, Private_label(lid.txt, ty_expected))); let arg = let snap = if vars = [] then None else Some (Btype.snapshot ()) in - let arg = type_argument env sarg ty_arg (instance env ty_arg) in + let arg = type_argument ?typeClashContext env sarg ty_arg (instance env ty_arg) in end_def (); try check_univars env (vars <> []) "field value" arg label.lbl_arg vars; @@ -3015,7 +3018,7 @@ and type_label_exp create env loc ty_expected in (lid, label, {arg with exp_type = instance env arg.exp_type}) -and type_argument ?recarg env sarg ty_expected' ty_expected = +and type_argument ?typeClashContext ?recarg env sarg ty_expected' ty_expected = (* ty_expected' may be generic *) let no_labels ty = let ls, tvar = list_labels env ty in @@ -3095,8 +3098,8 @@ and type_argument ?recarg env sarg ty_expected' ty_expected = func let_var) } end | _ -> - let texp = type_expect ?recarg env sarg ty_expected' in - unify_exp env texp ty_expected; + let texp = type_expect ?typeClashContext ?recarg env sarg ty_expected' in + unify_exp ?typeClashContext env texp ty_expected; texp and is_automatic_curried_application env funct = (* When a curried function is used with uncurried application, treat it as a curried application *) @@ -3104,7 +3107,7 @@ and is_automatic_curried_application env funct = match (expand_head env funct.exp_type).desc with | Tarrow _ -> true | _ -> false -and type_application uncurried env funct (sargs : sargs) : targs * Types.type_expr * bool = +and type_application ?typeClashContext uncurried env funct (sargs : sargs) : targs * Types.type_expr * bool = (* funct.exp_type may be generic *) let result_type omitted ty_fun = List.fold_left @@ -3219,7 +3222,7 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex in type_unknown_args max_arity ~args:((l1, Some arg1) :: args) omitted ty2 sargl in - let rec type_args max_arity args omitted ~ty_fun ty_fun0 ~(sargs : sargs) = + let rec type_args ?typeClashContext max_arity args omitted ~ty_fun ty_fun0 ~(sargs : sargs) = match expand_head env ty_fun, expand_head env ty_fun0 with {desc=Tarrow (l, ty, ty_fun, com); level=lv} , {desc=Tarrow (_, ty0, ty_fun0, _)} @@ -3242,13 +3245,13 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex sargs, omitted , Some ( if not optional || is_optional l' then - (fun () -> type_argument env sarg0 ty ty0) + (fun () -> type_argument ?typeClashContext:(typeClashContextForFunctionArgument typeClashContext sarg0) env sarg0 ty ty0) else - (fun () -> option_some (type_argument env sarg0 + (fun () -> option_some (type_argument ?typeClashContext env sarg0 (extract_option_type env ty) (extract_option_type env ty0)))) in - type_args max_arity ((l,arg)::args) omitted ~ty_fun ty_fun0 ~sargs + type_args ?typeClashContext max_arity ((l,arg)::args) omitted ~ty_fun ty_fun0 ~sargs | _ -> type_unknown_args max_arity ~args omitted ty_fun0 sargs (* This is the hot path for non-labeled function*) in @@ -3284,7 +3287,7 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex | _ -> if uncurried then force_uncurried_type funct; let ty, max_arity = extract_uncurried_type funct.exp_type in - let targs, ret_t = type_args max_arity [] [] ~ty_fun:ty (instance env ty) ~sargs in + let targs, ret_t = type_args ?typeClashContext max_arity [] [] ~ty_fun:ty (instance env ty) ~sargs in let fully_applied, ret_t = update_uncurried_arity funct.exp_type ~nargs:(List.length !ignored + List.length sargs) ret_t in targs, ret_t, fully_applied @@ -3323,10 +3326,11 @@ and type_construct env loc lid sarg ty_expected attrs = exp_type = ty_res; exp_attributes = attrs; exp_env = env } in + let typeClashContext = typeClashContextMaybeOption ty_expected ty_res in if separate then begin end_def (); generalize_structure ty_res; - unify_exp env {texp with exp_type = instance_def ty_res} + unify_exp ?typeClashContext env {texp with exp_type = instance_def ty_res} (instance env ty_expected); end_def (); List.iter generalize_structure ty_args; @@ -3338,7 +3342,7 @@ and type_construct env loc lid sarg ty_expected attrs = | _ -> assert false in let texp = {texp with exp_type = ty_res} in - if not separate then unify_exp env texp (instance env ty_expected); + if not separate then unify_exp ?typeClashContext env texp (instance env ty_expected); let recarg = match constr.cstr_inlined with | None -> Rejected @@ -3372,12 +3376,13 @@ and type_statement env sexp = if is_Tvar ty && ty.level > tv.level then Location.prerr_warning loc Warnings.Nonreturning_statement; let expected_ty = instance_def Predef.type_unit in - unify_exp env exp expected_ty; + let typeClashContext = typeClashContextInStatement sexp in + unify_exp ?typeClashContext env exp expected_ty; exp (* Typing of match cases *) -and type_cases ?in_function env ty_arg ty_res partial_flag loc caselist : _ * Typedtree.partial = +and type_cases ?rootTypeClashContext ?in_function env ty_arg ty_res partial_flag loc caselist : _ * Typedtree.partial = (* ty_arg is _fully_ generalized *) let patterns = List.map (fun {pc_lhs=p} -> p) caselist in let contains_polyvars = List.exists contains_polymorphic_variant patterns in @@ -3484,10 +3489,10 @@ and type_cases ?in_function env ty_arg ty_res partial_flag loc caselist : _ * Ty | None -> None | Some scond -> Some - (type_expect ext_env (wrap_unpacks scond unpacks) + (type_expect ?typeClashContext:(if Option.is_some rootTypeClashContext then Some IfCondition else None) ext_env (wrap_unpacks scond unpacks) Predef.type_bool) in - let exp = type_expect ?in_function ext_env sexp ty_res' in + let exp = type_expect ?typeClashContext:rootTypeClashContext ?in_function ext_env sexp ty_res' in { c_lhs = pat; c_guard = guard; @@ -3818,33 +3823,33 @@ let report_error env ppf = function fprintf ppf "Variable %s must occur on both sides of this | pattern" (Ident.name id); spellcheck_idents ppf id valid_idents - | Expr_type_clash ( + | Expr_type_clash (( (_, {desc = Tarrow _}) :: (_, {desc = Tconstr (Pident {name = "function$"},_,_)}) :: _ - ) -> + ), _) -> fprintf ppf "This function is a curried function where an uncurried function is expected" - | Expr_type_clash ( + | Expr_type_clash (( (_, {desc = Tconstr (Pident {name = "function$"}, [{desc=Tvar _}; _],_)}) :: (_, {desc = Tarrow _}) :: _ - ) -> + ), _) -> fprintf ppf "This function is an uncurried function where a curried function is expected" - | Expr_type_clash ( + | Expr_type_clash (( (_, {desc = Tconstr (Pident {name = "function$"},[_; tA],_)}) :: (_, {desc = Tconstr (Pident {name = "function$"},[_; tB],_)}) :: _ - ) when Ast_uncurried.type_to_arity tA <> Ast_uncurried.type_to_arity tB -> + ), _) when Ast_uncurried.type_to_arity tA <> Ast_uncurried.type_to_arity tB -> let arityA = Ast_uncurried.type_to_arity tA |> string_of_int in let arityB = Ast_uncurried.type_to_arity tB |> string_of_int in reportArityMismatch ~arityA ~arityB ppf - | Expr_type_clash ( + | Expr_type_clash (( (_, {desc = Tconstr (Pdot (Pdot(Pident {name = "Js_OO"},"Meth",_),a,_),_,_)}) :: (_, {desc = Tconstr (Pdot (Pdot(Pident {name = "Js_OO"},"Meth",_),b,_),_,_)}) :: _ - ) when a <> b -> + ), _) when a <> b -> fprintf ppf "This method has %s but was expected %s" a b - | Expr_type_clash trace -> + | Expr_type_clash (trace, typeClashContext) -> (* modified *) fprintf ppf "@["; - print_expr_type_clash env trace ppf; + print_expr_type_clash ?typeClashContext env trace ppf; fprintf ppf "@]" | Apply_non_function typ -> (* modified *) diff --git a/jscomp/ml/typecore.mli b/jscomp/ml/typecore.mli index 19e99c40ff..650bae0d5f 100644 --- a/jscomp/ml/typecore.mli +++ b/jscomp/ml/typecore.mli @@ -68,7 +68,7 @@ type error = | Or_pattern_type_clash of Ident.t * (type_expr * type_expr) list | Multiply_bound_variable of string | Orpat_vars of Ident.t * Ident.t list - | Expr_type_clash of (type_expr * type_expr) list + | Expr_type_clash of (type_expr * type_expr) list * (Error_message_utils.typeClashContext option) | Apply_non_function of type_expr | Apply_wrong_label of arg_label * type_expr | Label_multiply_defined of string