From 9fd8f52db8a7931d45e9d48e9767ab22e8b0fd16 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 20 Sep 2023 21:04:17 +0200 Subject: [PATCH 01/14] first batch of changes trying to improve the Somewhere wanted error messages --- .../expected/collections.res.expected | 2 +- .../expected/comparison_operator.res.expected | 11 + .../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 + .../expected/math_operator_float.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 +- .../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/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_float.res | 3 + .../fixtures/switch_different_types.res | 9 + .../super_errors/fixtures/switch_guard.res | 9 + jscomp/ml/typecore.ml | 276 ++++++++++++------ jscomp/ml/typecore.mli | 5 +- 38 files changed, 399 insertions(+), 113 deletions(-) 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_float.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/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_float.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 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..043ca08cd3 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/comparison_operator.res.expected @@ -0,0 +1,11 @@ + + 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 \ 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..09a60f4453 --- /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 + + In ReScript, `if` statements 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..875cac876a --- /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 + + Conditions for `if` statements must always evaluate to `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_float.res.expected b/jscomp/build_tests/super_errors/expected/math_operator_float.res.expected new file mode 100644 index 0000000000..3089a9aa19 --- /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 + + In ReScript, 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/primitives1.res.expected b/jscomp/build_tests/super_errors/expected/primitives1.res.expected index b262b9eae4..08237f89b6 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 + + In ReScript, 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/switch_different_types.res.expected b/jscomp/build_tests/super_errors/expected/switch_different_types.res.expected new file mode 100644 index 0000000000..86061069e1 --- /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..47aae008bd --- /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 + + Conditions for `if` statements must always evaluate to `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..967ab721bc 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 + + In ReScript, 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/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_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/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/typecore.ml b/jscomp/ml/typecore.ml index 2c1e471ce7..b31dcf08f9 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -23,6 +23,9 @@ open Typedtree open Btype open Ctype +type typeClashStatement = FunctionCall +type typeClashContext = FunctionReturn | MaybeUnwrapOption | IfCondition | IfReturn | Switch | StringConcat | ComparisonOperator | MathOperator of {forFloat: bool; operator: string; isConstant: string option} | FunctionArgument | Statement of typeClashStatement + type error = Polymorphic_label of Longident.t | Constructor_arity_mismatch of Longident.t * int * int @@ -31,7 +34,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 +338,15 @@ 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 ?ctx 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))) + (match ctx with | None -> () | Some ctx -> print_endline ctx); + raise(Error(loc, env, Expr_type_clash(trace, typeClashContext))) | Tags(l1,l2) -> raise(Typetexp.Error(loc, env, Typetexp.Variant_tags (l1, l2))) @@ -663,7 +667,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 +715,69 @@ let print_expr_type_clash env trace ppf = begin Printtyp.super_report_unification_error ppf env trace (function ppf -> - fprintf ppf "This has type:") + let text = (match typeClashContext with + | Some (Statement(FunctionCall)) -> "This function call returns:" + | Some (MathOperator{isConstant=Some _}) -> "This value has type:" + | _ -> "This has type:" + ) in + fprintf ppf "%s" text) (function ppf -> - fprintf ppf "Somewhere wanted:"); + 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 (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:" + ); + (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 In ReScript, 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 + | _, [{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 Conditions for `if` statements must always evaluate to `bool`. To fix this, change the highlighted code so it evaluates to a `bool`." + | Some IfReturn -> + fprintf ppf "\n\n In ReScript, `if` statements 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)`" + | _ -> () + ); show_extra_help ppf env trace; end @@ -1690,7 +1754,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 +1766,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 +1996,9 @@ let rec name_pattern default = function (* Typing of expressions *) -let unify_exp env exp expected_ty = +let unify_exp ?typeClashContext ?ctx 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 ?ctx loc env exp.exp_type expected_ty let is_ignore funct env = @@ -1984,23 +2048,27 @@ 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 ?ctx ?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 ?tctx:ctx ?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 ?tctx ?in_function ?(recarg=Rejected) env sexp ty_expected = let loc = sexp.pexp_loc in + let ctx = (match sexp.pexp_desc with | Pexp_constant _ -> "constant" | _ -> "-") in + let ctx = ctx ^ (match in_function with | None -> " " | Some (_loc, _t) -> " in fn ") in + let ctx = ctx ^ (Option.value tctx ~default:"") in + (* TODO: Figure out good way of discerning fn return value *) (* 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 ~ctx env (re exp) (instance env ty_expected); exp in let process_optional_label (id, ld, e) = @@ -2060,7 +2128,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = [{pvb_pat=spat; pvb_expr=sval; pvb_attributes=[]}], sbody) when contains_gadt env spat -> (* TODO: allow non-empty attributes? *) - type_expect ?in_function env + type_expect ~ctx:"pexp_let" ?in_function env {sexp with pexp_desc = Pexp_match (sval, [Ast_helper.Exp.case spat sbody])} ty_expected @@ -2074,7 +2142,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = let (pat_exp_list, new_env, unpacks) = type_let env rec_flag spat_sexp_list scp true in let body = - type_expect new_env (wrap_unpacks sbody unpacks) ty_expected in + type_expect ~ctx:"pexp_let 2" new_env (wrap_unpacks sbody unpacks) ty_expected in let () = if rec_flag = Recursive then Rec_check.check_recursive_bindings pat_exp_list @@ -2138,7 +2206,17 @@ 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 isConstant = (match sexp.pexp_desc with + | Pexp_constant (Pconst_integer(txt, _) | Pconst_float (txt, _)) -> Some txt + | _ -> None) in + let typeClashContext = (match sfunct.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) + ) 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 +2268,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 = []; @@ -2215,7 +2293,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = let to_unify = newgenty (Ttuple subtypes) in unify_exp_types loc env to_unify ty_expected; let expl = - List.map2 (fun body ty -> type_expect env body ty) sexpl subtypes + List.map2 (fun body ty -> type_expect ~ctx:"pexp_tuple" env body ty) sexpl subtypes in re { exp_desc = Texp_tuple expl; @@ -2247,7 +2325,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = begin match row_field_repr (List.assoc l row.row_fields), row_field_repr (List.assoc l row0.row_fields) with Rpresent (Some ty), Rpresent (Some ty0) -> - let arg = type_argument env sarg ty ty0 in + let arg = type_argument ~ctx:"pexp_variant#1" env sarg ty ty0 in re { exp_desc = Texp_variant(l, Some arg); exp_loc = loc; exp_extra = []; exp_type = ty_expected0; @@ -2288,7 +2366,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = opath lid_sexp_list) (fun x -> x) in - unify_exp_types loc env ty_record (instance env ty_expected); + unify_exp_types ~ctx:"record" loc env ty_record (instance env ty_expected); check_duplicates loc env lbl_exp_list; let label_descriptions, representation = match lbl_exp_list, repr_opt with | (_, { lbl_all = label_descriptions; lbl_repres = representation}, _) :: _, _ -> label_descriptions, representation @@ -2381,7 +2459,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = opath lid_sexp_list) (fun x -> x) in - unify_exp_types loc env ty_record (instance env ty_expected); + unify_exp_types ~ctx:"record2" loc env ty_record (instance env ty_expected); check_duplicates loc env lbl_exp_list; let opt_exp, label_definitions = let (_lid, lbl, _lbl_exp) = List.hd lbl_exp_list in @@ -2393,15 +2471,15 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = let ty_exp = instance env exp.exp_type in let unify_kept lbl = let _, ty_arg1, ty_res1 = instance_label false lbl in - unify_exp_types exp.exp_loc env ty_exp ty_res1; + unify_exp_types ~ctx:"record inner" exp.exp_loc env ty_exp ty_res1; match matching_label lbl with | lid, _lbl, lbl_exp -> (* do not connect result types for overridden labels *) Overridden (lid, lbl_exp) | exception Not_found -> begin let _, ty_arg2, ty_res2 = instance_label false lbl in - unify_exp_types loc env ty_arg1 ty_arg2; - unify_exp_types loc env (instance env ty_expected) ty_res2; + unify_exp_types ~ctx:"record inner 2" loc env ty_arg1 ty_arg2; + unify_exp_types ~ctx:"record inner 3" loc env (instance env ty_expected) ty_res2; Kept ty_arg1 end in @@ -2436,7 +2514,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = | Pexp_field(srecord, lid) -> let (record, label, _) = type_label_access env srecord lid in let (_, ty_arg, ty_res) = instance_label false label in - unify_exp env record ty_res; + unify_exp ~ctx:"field" env record ty_res; rue { exp_desc = Texp_field(record, lid, label); exp_loc = loc; exp_extra = []; @@ -2448,7 +2526,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = 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 - unify_exp env record ty_record; + unify_exp ~ctx:"set field" env record ty_record; if label.lbl_mut = Immutable then raise(Error(loc, env, Label_not_mutable lid.txt)); Builtin_attributes.check_deprecated_mutable lid.loc label.lbl_attributes @@ -2462,8 +2540,8 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = | Pexp_array(sargl) -> 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 + unify_exp_types ~ctx:"array" loc env to_unify ty_expected; + let argl = List.map (fun sarg -> type_expect ~ctx:"pexp_array" env sarg ty) sargl in re { exp_desc = Texp_array argl; exp_loc = loc; exp_extra = []; @@ -2471,10 +2549,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 ~ctx:"if" 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 ~ctx:"if 2" env sifso Predef.type_unit in rue { exp_desc = Texp_ifthenelse(cond, ifso, None); exp_loc = loc; exp_extra = []; @@ -2482,10 +2560,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 ~ctx:"if 3" env sifso ty_expected in + let ifnot = type_expect ~typeClashContext:IfReturn ~ctx:"if 4" env sifnot ty_expected in (* Keep sharing *) - unify_exp env ifnot ifso.exp_type; + unify_exp ~typeClashContext:IfReturn ~ctx:"if" env ifnot ifso.exp_type; re { exp_desc = Texp_ifthenelse(cond, ifso, Some ifnot); exp_loc = loc; exp_extra = []; @@ -2495,7 +2573,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = end | Pexp_sequence(sexp1, sexp2) -> let exp1 = type_statement env sexp1 in - let exp2 = type_expect env sexp2 ty_expected in + let exp2 = type_expect ~ctx:"pexp_sequence" env sexp2 ty_expected in re { exp_desc = Texp_sequence(exp1, exp2); exp_loc = loc; exp_extra = []; @@ -2503,7 +2581,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = exp_attributes = sexp.pexp_attributes; exp_env = env } | Pexp_while(scond, sbody) -> - let cond = type_expect env scond Predef.type_bool in + let cond = type_expect ~ctx:"pexp_while" env scond Predef.type_bool in let body = type_statement env sbody in rue { exp_desc = Texp_while(cond, body); @@ -2512,8 +2590,8 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = exp_attributes = sexp.pexp_attributes; exp_env = env } | Pexp_for(param, slow, shigh, dir, sbody) -> - let low = type_expect env slow Predef.type_int in - let high = type_expect env shigh Predef.type_int in + let low = type_expect ~ctx:"pexp_for" env slow Predef.type_int in + let high = type_expect ~ctx:"pexp_for 2" env shigh Predef.type_int in let id, new_env = match param.ppat_desc with | Ppat_any -> Ident.create "_for", env @@ -2541,9 +2619,9 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = if separate then begin end_def (); generalize_structure ty; - (type_argument env sarg ty (instance env ty), instance env ty) + (type_argument ~ctx:"constraint#1" env sarg ty (instance env ty), instance env ty) end else - (type_argument env sarg ty ty, ty) + (type_argument ~ctx:"constraint#2" env sarg ty ty, ty) in rue { exp_desc = arg.exp_desc; @@ -2573,7 +2651,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 @@ -2623,10 +2701,10 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = end_def (); generalize_structure ty; generalize_structure ty'; - (type_argument env sarg ty (instance env ty), + (type_argument ~ctx:"coerce#1" env sarg ty (instance env ty), instance env ty', Some cty, cty') end else - (type_argument env sarg ty ty, ty', Some cty, cty') + (type_argument ~ctx:"coerce#2" env sarg ty ty, ty', Some cty, cty') in rue { exp_desc = arg.exp_desc; @@ -2699,7 +2777,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = let (id, new_env) = Env.enter_module name.txt modl.mod_type env in Ctype.init_def(Ident.current_time()); Typetexp.widen context; - let body = type_expect new_env sbody ty_expected in + let body = type_expect ~ctx:"pexp_letmodule" new_env sbody ty_expected in (* go back to original level *) end_def (); (* Unification of body.exp_type with the fresh variable ty @@ -2750,7 +2828,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = | Pexp_lazy e -> let ty = newgenvar () in let to_unify = Predef.type_lazy_t ty in - unify_exp_types loc env to_unify ty_expected; + unify_exp_types ~ctx:"lazy" loc env to_unify ty_expected; let arg = type_expect env e ty in re { exp_desc = Texp_lazy arg; @@ -2769,24 +2847,24 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = repr cty.ctyp_type, Some cty in if sty <> None then - unify_exp_types loc env (instance env ty) (instance env ty_expected); + unify_exp_types ~ctx:"poly" loc env (instance env ty) (instance env ty_expected); let exp = match (expand_head env ty).desc with Tpoly (ty', []) -> - let exp = type_expect env sbody ty' in + let exp = type_expect ~ctx:"poly2" env sbody ty' in { exp with exp_type = instance env ty } | Tpoly (ty', tl) -> (* One more level to generalize locally *) begin_def (); let vars, ty'' = instance_poly true tl ty' in - let exp = type_expect env sbody ty'' in + let exp = type_expect ~ctx:"poly3" env sbody ty'' in end_def (); check_univars env false "method" exp ty_expected vars; { exp with exp_type = instance env ty } | Tvar _ -> let exp = type_exp env sbody in let exp = {exp with exp_type = newty (Tpoly (exp.exp_type, []))} in - unify_exp env exp ty; + unify_exp ~ctx:"tvar" env exp ty; exp | _ -> assert false in @@ -2860,7 +2938,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = exp_env = env } | Pexp_open (ovf, lid, e) -> let (path, newenv) = !type_open ovf env sexp.pexp_loc lid in - let exp = type_expect newenv e ty_expected in + let exp = type_expect ~ctx:"pexp_open" newenv e ty_expected in { exp with exp_extra = (Texp_open (ovf, path, lid, newenv), loc, sexp.pexp_attributes) :: @@ -2995,7 +3073,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 ~ctx:"typelabelexp#1" env sarg ty_arg (instance env ty_arg) in end_def (); try check_univars env (vars <> []) "field value" arg label.lbl_arg vars; @@ -3007,7 +3085,7 @@ and type_label_exp create env loc ty_expected let arg = type_exp env sarg in end_def (); generalize_expansive env arg.exp_type; - unify_exp env arg ty_arg; + unify_exp ~ctx:"type label exp" env arg ty_arg; check_univars env false "field value" arg label.lbl_arg vars; arg with Error (_, _, Less_general _) as e -> raise e @@ -3015,7 +3093,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 ?ctx ?recarg env sarg ty_expected' ty_expected = (* ty_expected' may be generic *) let no_labels ty = let ls, tvar = list_labels env ty in @@ -3049,10 +3127,10 @@ and type_argument ?recarg env sarg ty_expected' ty_expected = let texp = {texp with exp_type = instance env texp.exp_type} and ty_fun = instance env ty_fun' in if not (simple_res || no_labels ty_res) then begin - unify_exp env texp ty_expected; + unify_exp ~ctx:"type argument" env texp ty_expected; texp end else begin - unify_exp env {texp with exp_type = ty_fun} ty_expected; + unify_exp ~ctx:"type argument 2" env {texp with exp_type = ty_fun} ty_expected; if args = [] then texp else (* eta-expand to avoid side effects *) let var_pair name ty = @@ -3095,8 +3173,9 @@ 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 ctx = Option.value ctx ~default:"" in + let texp = type_expect ?typeClashContext ~ctx:("targ1 " ^ ctx) ?recarg env sarg ty_expected' in + unify_exp ?typeClashContext ~ctx:"type arg 2" 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 +3183,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 @@ -3130,7 +3209,7 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex begin match (expand_head env funct.exp_type).desc with | Tvar _ | Tarrow _ -> - unify_exp env funct uncurried_typ + unify_exp ~ctx:"type application tvar" env funct uncurried_typ | _ -> raise(Error(funct.exp_loc, env, Apply_non_function (expand_head env funct.exp_type))) end @@ -3212,14 +3291,14 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex in let optional = is_optional l1 in let arg1 () = - let arg1 = type_expect env sarg1 ty1 in + let arg1 = type_expect ~ctx:"arg1" env sarg1 ty1 in if optional then - unify_exp env arg1 (type_option(newvar())); + unify_exp ~ctx:"opt" env arg1 (type_option(newvar())); arg1 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, _)} @@ -3241,14 +3320,21 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex (Warnings.Nonoptional_label (Printtyp.string_of_label l)); sargs, omitted , Some ( - if not optional || is_optional l' then - (fun () -> type_argument env sarg0 ty ty0) - else - (fun () -> option_some (type_argument env sarg0 + if not optional || is_optional l' then( + let typeClashContext = (match typeClashContext with + | Some MathOperator {forFloat; operator} -> Some + (MathOperator {forFloat; operator; isConstant = + match sarg0.pexp_desc with + | Pexp_constant (Pconst_integer (txt, _) | Pconst_float (txt, _)) -> Some txt + | _ -> None}) + | typeClashContext -> typeClashContext) in + (fun () -> type_argument ?typeClashContext ~ctx:"opt#1" env sarg0 ty ty0) + )else + (fun () -> option_some (type_argument ?typeClashContext env ~ctx:"opt#2" 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 @@ -3272,7 +3358,7 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex let ty_arg, ty_res = filter_arrow env (instance env funct.exp_type) Nolabel in - let exp = type_expect env sarg ty_arg in + let exp = type_expect ~ctx:"sargs" env sarg ty_arg in begin match (expand_head env exp.exp_type).desc with | Tarrow _ -> Location.prerr_warning exp.exp_loc Warnings.Partial_application @@ -3284,7 +3370,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 +3409,14 @@ and type_construct env loc lid sarg ty_expected attrs = exp_type = ty_res; exp_attributes = attrs; exp_env = env } in + let typeClashContext = (match ty_expected, ty_res with + | {desc=Tconstr (expectedPath, _, _)}, {desc=Tconstr (typePath, _, _)} + when Path.same Predef.path_option typePath && Path.same expectedPath Predef.path_option = false -> Some MaybeUnwrapOption + | _ -> None) 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 ~ctx:"type construct" env {texp with exp_type = instance_def ty_res} (instance env ty_expected); end_def (); List.iter generalize_structure ty_args; @@ -3338,7 +3428,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 ~ctx:"type construct 2" env texp (instance env ty_expected); let recarg = match constr.cstr_inlined with | None -> Rejected @@ -3353,7 +3443,7 @@ and type_construct env loc lid sarg ty_expected attrs = end in let args = - List.map2 (fun e (t,t0) -> type_argument ~recarg env e t t0) sargs + List.map2 (fun e (t,t0) -> type_argument ~ctx:"typeconstruct#1" ~recarg env e t t0) sargs (List.combine ty_args ty_args0) in if constr.cstr_private = Private then raise(Error(loc, env, Private_type ty_res)); @@ -3372,12 +3462,16 @@ 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 = (match sexp.pexp_desc with + | Pexp_apply _ -> Some(Statement(FunctionCall)) + | _ -> None + ) in + unify_exp ?typeClashContext ~ctx:"type statement" 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 +3578,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:(match rootTypeClashContext with | Some _ -> Some IfCondition | None -> None) ~ctx:"guard" 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 ~ctx:"guard exp" ?in_function ext_env sexp ty_res' in { c_lhs = pat; c_guard = guard; @@ -3498,7 +3592,7 @@ and type_cases ?in_function env ty_arg ty_res partial_flag loc caselist : _ * Ty in if has_gadts then begin let ty_res' = instance env ty_res in - List.iter (fun c -> unify_exp env c.c_rhs ty_res') cases + List.iter (fun c -> unify_exp ~ctx:"gadts" env c.c_rhs ty_res') cases end; let do_init = has_gadts || needs_exhaust_check in let lev, env = @@ -3529,7 +3623,7 @@ and type_cases ?in_function env ty_arg ty_res partial_flag loc caselist : _ * Ty if do_init then begin end_def (); (* Ensure that existential types do not escape *) - unify_exp_types loc env (instance env ty_res) (newvar ()) ; + unify_exp_types ~ctx:"type cases" loc env (instance env ty_res) (newvar ()) ; end; cases, partial @@ -3664,14 +3758,14 @@ and type_let ?(check = fun s -> Warnings.Unused_var s) let vars, ty' = instance_poly ~keep_names:true true tl ty in let exp = Builtin_attributes.warning_scope pvb_attributes - (fun () -> type_expect exp_env sexp ty') + (fun () -> type_expect ~ctx:"polyy" exp_env sexp ty') in end_def (); check_univars env true "definition" exp pat.pat_type vars; {exp with exp_type = instance env exp.exp_type} | _ -> Builtin_attributes.warning_scope pvb_attributes (fun () -> - type_expect exp_env sexp pat.pat_type)) + type_expect ~ctx:"poly rest" exp_env sexp pat.pat_type)) spat_sexp_list pat_slot_list in current_slot := None; if is_recursive && not !rec_needed @@ -3818,33 +3912,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..64e7c6744b 100644 --- a/jscomp/ml/typecore.mli +++ b/jscomp/ml/typecore.mli @@ -60,6 +60,9 @@ val name_pattern : string -> Typedtree.case list -> Ident.t val self_coercion : (Path.t * Location.t list ref) list ref +type typeClashStatement = FunctionCall +type typeClashContext = FunctionReturn |MaybeUnwrapOption | IfCondition | IfReturn | Switch | StringConcat | ComparisonOperator | MathOperator of {forFloat: bool; operator: string; isConstant: string option} | FunctionArgument | Statement of typeClashStatement + type error = Polymorphic_label of Longident.t | Constructor_arity_mismatch of Longident.t * int * int @@ -68,7 +71,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 From b7efb125c551650a5bff409c39c26a7f1a801cca Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 21 Sep 2023 08:48:40 +0200 Subject: [PATCH 02/14] tweaks --- .../expected/comparison_operator.res.expected | 4 +++- .../expected/if_branch_mismatch.res.expected | 2 +- .../expected/if_condition_mismatch.res.expected | 2 +- .../expected/math_operator_float.res.expected | 2 +- .../super_errors/expected/primitives1.res.expected | 2 +- .../expected/switch_different_types.res.expected | 2 +- .../super_errors/expected/switch_guard.res.expected | 2 +- .../super_errors/expected/type1.res.expected | 2 +- jscomp/ml/typecore.ml | 10 ++++++---- 9 files changed, 16 insertions(+), 12 deletions(-) diff --git a/jscomp/build_tests/super_errors/expected/comparison_operator.res.expected b/jscomp/build_tests/super_errors/expected/comparison_operator.res.expected index 043ca08cd3..b2988d3431 100644 --- a/jscomp/build_tests/super_errors/expected/comparison_operator.res.expected +++ b/jscomp/build_tests/super_errors/expected/comparison_operator.res.expected @@ -8,4 +8,6 @@ 4 │ This has type: option - But it's being compared to something of type: int \ No newline at end of file + 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/if_branch_mismatch.res.expected b/jscomp/build_tests/super_errors/expected/if_branch_mismatch.res.expected index 09a60f4453..714a6f074f 100644 --- a/jscomp/build_tests/super_errors/expected/if_branch_mismatch.res.expected +++ b/jscomp/build_tests/super_errors/expected/if_branch_mismatch.res.expected @@ -11,6 +11,6 @@ This has type: int But this if statement is expected to return: string - In ReScript, `if` statements must return the same type in all branches (`if`, `else if`, `else`). + 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 index 875cac876a..e1d1330566 100644 --- a/jscomp/build_tests/super_errors/expected/if_condition_mismatch.res.expected +++ b/jscomp/build_tests/super_errors/expected/if_condition_mismatch.res.expected @@ -9,4 +9,4 @@ This has type: string But if conditions must always be of type: bool - Conditions for `if` statements must always evaluate to `bool`. To fix this, change the highlighted code so it evaluates to a `bool`. \ No newline at end of file + Conditions for if statements must always evaluate to 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_float.res.expected b/jscomp/build_tests/super_errors/expected/math_operator_float.res.expected index 3089a9aa19..cd0fffa1c1 100644 --- a/jscomp/build_tests/super_errors/expected/math_operator_float.res.expected +++ b/jscomp/build_tests/super_errors/expected/math_operator_float.res.expected @@ -10,7 +10,7 @@ This has type: int But it's being used with the +. operator, which works on: float - In ReScript, floats and ints have their own mathematical operators. This means you cannot add a float and an int without converting between the two. + 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. diff --git a/jscomp/build_tests/super_errors/expected/primitives1.res.expected b/jscomp/build_tests/super_errors/expected/primitives1.res.expected index 08237f89b6..b1a303eccb 100644 --- a/jscomp/build_tests/super_errors/expected/primitives1.res.expected +++ b/jscomp/build_tests/super_errors/expected/primitives1.res.expected @@ -9,7 +9,7 @@ This value has type: float But it's being used with the + operator, which works on: int - In ReScript, floats and ints have their own mathematical operators. This means you cannot add a float and an int without converting between the two. + 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. 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 index 86061069e1..14e96e8964 100644 --- a/jscomp/build_tests/super_errors/expected/switch_different_types.res.expected +++ b/jscomp/build_tests/super_errors/expected/switch_different_types.res.expected @@ -11,4 +11,4 @@ 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 + 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 index 47aae008bd..568f445872 100644 --- a/jscomp/build_tests/super_errors/expected/switch_guard.res.expected +++ b/jscomp/build_tests/super_errors/expected/switch_guard.res.expected @@ -11,4 +11,4 @@ This has type: string But if conditions must always be of type: bool - Conditions for `if` statements must always evaluate to `bool`. To fix this, change the highlighted code so it evaluates to a `bool`. \ No newline at end of file + Conditions for if statements must always evaluate to 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/type1.res.expected b/jscomp/build_tests/super_errors/expected/type1.res.expected index 967ab721bc..036daa2550 100644 --- a/jscomp/build_tests/super_errors/expected/type1.res.expected +++ b/jscomp/build_tests/super_errors/expected/type1.res.expected @@ -8,7 +8,7 @@ This value has type: float But it's being used with the + operator, which works on: int - In ReScript, floats and ints have their own mathematical operators. This means you cannot add a float and an int without converting between the two. + 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. diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index b31dcf08f9..609e8cc516 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -751,7 +751,7 @@ let print_expr_type_clash ?typeClashContext env trace ppf = begin | '*' -> "multiply" | _ -> "compute") in (* TODO check int vs float explicitly before showing this *) - (fprintf ppf "\n\n In ReScript, 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@}." + (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 @@ -769,13 +769,15 @@ let print_expr_type_clash ?typeClashContext env trace ppf = begin | _ -> ()) | _ -> ()) | 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." + 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 Conditions for `if` statements must always evaluate to `bool`. To fix this, change the highlighted code so it evaluates to a `bool`." + fprintf ppf "\n\n Conditions for @{if@} statements must always evaluate to @{bool@}. To fix this, change the highlighted code so it evaluates to a @{bool@}." | Some IfReturn -> - fprintf ppf "\n\n In ReScript, `if` statements must return the same type in all branches (`if`, `else if`, `else`)." + 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." | _ -> () ); show_extra_help ppf env trace; From 79c827f1ae0df26f39152c5ad0428d6f7216e030 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 21 Sep 2023 09:04:54 +0200 Subject: [PATCH 03/14] add more fixtures --- .../math_operator_constant.res.expected | 20 +++++++++++++++++++ .../expected/math_operator_int.res.expected | 20 +++++++++++++++++++ .../fixtures/math_operator_constant.res | 3 +++ .../fixtures/math_operator_int.res | 3 +++ 4 files changed, 46 insertions(+) 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_int.res.expected create mode 100644 jscomp/build_tests/super_errors/fixtures/math_operator_constant.res create mode 100644 jscomp/build_tests/super_errors/fixtures/math_operator_int.res 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_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/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_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. From b2ffe9308caf99cb24b8ac12f55ad798511a08a5 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 21 Sep 2023 11:17:00 +0200 Subject: [PATCH 04/14] remove redundant text --- .../super_errors/expected/if_condition_mismatch.res.expected | 2 +- .../build_tests/super_errors/expected/switch_guard.res.expected | 2 +- jscomp/ml/typecore.ml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 index e1d1330566..f81ffca952 100644 --- a/jscomp/build_tests/super_errors/expected/if_condition_mismatch.res.expected +++ b/jscomp/build_tests/super_errors/expected/if_condition_mismatch.res.expected @@ -9,4 +9,4 @@ This has type: string But if conditions must always be of type: bool - Conditions for if statements must always evaluate to bool. To fix this, change the highlighted code so it evaluates to a bool. \ No newline at end of file + 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/switch_guard.res.expected b/jscomp/build_tests/super_errors/expected/switch_guard.res.expected index 568f445872..f04ff7dd76 100644 --- a/jscomp/build_tests/super_errors/expected/switch_guard.res.expected +++ b/jscomp/build_tests/super_errors/expected/switch_guard.res.expected @@ -11,4 +11,4 @@ This has type: string But if conditions must always be of type: bool - Conditions for if statements must always evaluate to bool. To fix this, change the highlighted code so it evaluates to a bool. \ No newline at end of file + To fix this, change the highlighted code so it evaluates to a bool. \ No newline at end of file diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 609e8cc516..f1ae797b97 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -771,7 +771,7 @@ let print_expr_type_clash ?typeClashContext env trace ppf = begin | 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 Conditions for @{if@} statements must always evaluate to @{bool@}. To fix this, change the highlighted code so it evaluates to a @{bool@}." + 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 -> From 3db81796f81885c70e1d975c89db3a959317b5ef Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 21 Sep 2023 21:01:47 +0200 Subject: [PATCH 05/14] add specialized errors for array items and setting record fields --- .../array_item_type_mismatch.res.expected | 13 +++++++++++++ .../set_record_field_type_match.res.expected | 13 +++++++++++++ .../fixtures/array_item_type_mismatch.res | 1 + .../fixtures/set_record_field_type_match.res | 11 +++++++++++ jscomp/ml/typecore.ml | 15 ++++++++++----- jscomp/ml/typecore.mli | 2 +- 6 files changed, 49 insertions(+), 6 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/set_record_field_type_match.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/set_record_field_type_match.res 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..d6f2a267e5 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/array_item_type_mismatch.res.expected @@ -0,0 +1,13 @@ + + 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. + + 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..d66f721a64 --- /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 │ + + This 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/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/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/ml/typecore.ml b/jscomp/ml/typecore.ml index f1ae797b97..f9b1649e53 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -24,7 +24,7 @@ open Btype open Ctype type typeClashStatement = FunctionCall -type typeClashContext = FunctionReturn | MaybeUnwrapOption | IfCondition | IfReturn | Switch | StringConcat | ComparisonOperator | MathOperator of {forFloat: bool; operator: string; isConstant: string option} | FunctionArgument | Statement of typeClashStatement +type typeClashContext = SetRecordField | ArrayValue | FunctionReturn | MaybeUnwrapOption | IfCondition | IfReturn | Switch | StringConcat | ComparisonOperator | MathOperator of {forFloat: bool; operator: string; isConstant: string option} | FunctionArgument | Statement of typeClashStatement type error = Polymorphic_label of Longident.t @@ -718,6 +718,7 @@ let print_expr_type_clash ?typeClashContext env trace ppf = begin 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:" | _ -> "This has type:" ) in fprintf ppf "%s" text) @@ -728,6 +729,8 @@ let print_expr_type_clash ?typeClashContext env trace ppf = begin | 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:" @@ -778,6 +781,8 @@ let print_expr_type_clash ?typeClashContext env trace ppf = begin 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." | _ -> () ); show_extra_help ppf env trace; @@ -2527,7 +2532,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex 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 ~ctx:"set field" env record ty_record; if label.lbl_mut = Immutable then raise(Error(loc, env, Label_not_mutable lid.txt)); @@ -2543,7 +2548,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex let ty = newgenvar() in let to_unify = Predef.type_array ty in unify_exp_types ~ctx:"array" loc env to_unify ty_expected; - let argl = List.map (fun sarg -> type_expect ~ctx:"pexp_array" env sarg ty) sargl in + let argl = List.map (fun sarg -> type_expect ~typeClashContext:ArrayValue ~ctx:"pexp_array" env sarg ty) sargl in re { exp_desc = Texp_array argl; exp_loc = loc; exp_extra = []; @@ -3043,7 +3048,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 (); @@ -3075,7 +3080,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 ~ctx:"typelabelexp#1" env sarg ty_arg (instance env ty_arg) in + let arg = type_argument ?typeClashContext ~ctx:"typelabelexp#1" env sarg ty_arg (instance env ty_arg) in end_def (); try check_univars env (vars <> []) "field value" arg label.lbl_arg vars; diff --git a/jscomp/ml/typecore.mli b/jscomp/ml/typecore.mli index 64e7c6744b..925d76d668 100644 --- a/jscomp/ml/typecore.mli +++ b/jscomp/ml/typecore.mli @@ -61,7 +61,7 @@ val name_pattern : string -> Typedtree.case list -> Ident.t val self_coercion : (Path.t * Location.t list ref) list ref type typeClashStatement = FunctionCall -type typeClashContext = FunctionReturn |MaybeUnwrapOption | IfCondition | IfReturn | Switch | StringConcat | ComparisonOperator | MathOperator of {forFloat: bool; operator: string; isConstant: string option} | FunctionArgument | Statement of typeClashStatement +type typeClashContext = SetRecordField | ArrayValue | FunctionReturn | MaybeUnwrapOption | IfCondition | IfReturn | Switch | StringConcat | ComparisonOperator | MathOperator of {forFloat: bool; operator: string; isConstant: string option} | FunctionArgument | Statement of typeClashStatement type error = Polymorphic_label of Longident.t From a412502aa9eb4e1e560b39384e28419baf8a778f Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 22 Sep 2023 11:35:27 +0200 Subject: [PATCH 06/14] tweak set record field message --- .../expected/set_record_field_type_match.res.expected | 2 +- jscomp/ml/typecore.ml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) 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 index d66f721a64..d45880d7d2 100644 --- 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 @@ -7,7 +7,7 @@ 11 │ user.name = 12 12 │ - This has type: int + 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/ml/typecore.ml b/jscomp/ml/typecore.ml index f9b1649e53..360c3c0824 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -719,6 +719,7 @@ let print_expr_type_clash ?typeClashContext env trace ppf = begin | 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) From 873e2d34d4293eb017aa8f6c1bad6cbea1b612c3 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 24 Sep 2023 19:09:23 +0200 Subject: [PATCH 07/14] hint about tuple in array message --- .../expected/array_item_type_mismatch.res.expected | 4 ++++ jscomp/ml/typecore.ml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) 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 index d6f2a267e5..e42a839d8d 100644 --- 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 @@ -9,5 +9,9 @@ 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. You can convert string to int with Belt.Int.fromString. \ No newline at end of file diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 360c3c0824..c0bc400046 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -783,7 +783,7 @@ let print_expr_type_clash ?typeClashContext env trace ppf = begin | 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." + 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." | _ -> () ); show_extra_help ppf env trace; From c28e0115029acd957b35fe72ec62090143198b82 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 24 Sep 2023 19:54:07 +0200 Subject: [PATCH 08/14] move message handling to own file --- jscomp/ml/error_message_utils.ml | 149 ++++++++++++++++++++++ jscomp/ml/typecore.ml | 209 ++++++++++--------------------- jscomp/ml/typecore.mli | 5 +- 3 files changed, 217 insertions(+), 146 deletions(-) create mode 100644 jscomp/ml/error_message_utils.ml diff --git a/jscomp/ml/error_message_utils.ml b/jscomp/ml/error_message_utils.ml new file mode 100644 index 0000000000..b5f758d184 --- /dev/null +++ b/jscomp/ml/error_message_utils.ml @@ -0,0 +1,149 @@ +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." + | _ -> () diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index c0bc400046..539b70dcc1 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -22,9 +22,7 @@ open Types open Typedtree open Btype open Ctype - -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 +open Error_message_utils type error = Polymorphic_label of Longident.t @@ -338,14 +336,13 @@ 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 ?typeClashContext ?ctx 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 -> - (match ctx with | None -> () | Some ctx -> print_endline ctx); raise(Error(loc, env, Expr_type_clash(trace, typeClashContext))) | Tags(l1,l2) -> raise(Typetexp.Error(loc, env, Typetexp.Variant_tags (l1, l2))) @@ -715,77 +712,10 @@ let print_expr_type_clash ?typeClashContext env trace ppf = begin Printtyp.super_report_unification_error ppf env trace (function ppf -> - 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) + errorTypeText ppf typeClashContext) (function ppf -> - 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:" - ); - (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 - | _, [{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." - | _ -> () - ); + errorExpectedTypeText ppf typeClashContext); + printExtraTypeClashHelp ppf trace typeClashContext; show_extra_help ppf env trace; end @@ -2004,9 +1934,9 @@ let rec name_pattern default = function (* Typing of expressions *) -let unify_exp ?typeClashContext ?ctx env exp expected_ty = +let unify_exp ?typeClashContext env exp expected_ty = let loc = proper_exp_loc exp in - unify_exp_types ?typeClashContext ?ctx loc env exp.exp_type expected_ty + unify_exp_types ?typeClashContext loc env exp.exp_type expected_ty let is_ignore funct env = @@ -2056,27 +1986,23 @@ let rec type_exp ?recarg env sexp = In the principal case, [type_expected'] may be at generic_level. *) -and type_expect ?typeClashContext ?ctx ?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_ ?typeClashContext ?tctx:ctx ?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_ ?typeClashContext ?tctx ?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 - let ctx = (match sexp.pexp_desc with | Pexp_constant _ -> "constant" | _ -> "-") in - let ctx = ctx ^ (match in_function with | None -> " " | Some (_loc, _t) -> " in fn ") in - let ctx = ctx ^ (Option.value tctx ~default:"") in - (* TODO: Figure out good way of discerning fn return value *) (* Record the expression type before unifying it with the expected type *) let rue exp = - unify_exp ?typeClashContext ~ctx 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) = @@ -2136,7 +2062,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex [{pvb_pat=spat; pvb_expr=sval; pvb_attributes=[]}], sbody) when contains_gadt env spat -> (* TODO: allow non-empty attributes? *) - type_expect ~ctx:"pexp_let" ?in_function env + type_expect ?in_function env {sexp with pexp_desc = Pexp_match (sval, [Ast_helper.Exp.case spat sbody])} ty_expected @@ -2150,7 +2076,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex let (pat_exp_list, new_env, unpacks) = type_let env rec_flag spat_sexp_list scp true in let body = - type_expect ~ctx:"pexp_let 2" new_env (wrap_unpacks sbody unpacks) ty_expected in + type_expect new_env (wrap_unpacks sbody unpacks) ty_expected in let () = if rec_flag = Recursive then Rec_check.check_recursive_bindings pat_exp_list @@ -2301,7 +2227,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex let to_unify = newgenty (Ttuple subtypes) in unify_exp_types loc env to_unify ty_expected; let expl = - List.map2 (fun body ty -> type_expect ~ctx:"pexp_tuple" env body ty) sexpl subtypes + List.map2 (fun body ty -> type_expect env body ty) sexpl subtypes in re { exp_desc = Texp_tuple expl; @@ -2333,7 +2259,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex begin match row_field_repr (List.assoc l row.row_fields), row_field_repr (List.assoc l row0.row_fields) with Rpresent (Some ty), Rpresent (Some ty0) -> - let arg = type_argument ~ctx:"pexp_variant#1" env sarg ty ty0 in + let arg = type_argument env sarg ty ty0 in re { exp_desc = Texp_variant(l, Some arg); exp_loc = loc; exp_extra = []; exp_type = ty_expected0; @@ -2374,7 +2300,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex opath lid_sexp_list) (fun x -> x) in - unify_exp_types ~ctx:"record" loc env ty_record (instance env ty_expected); + unify_exp_types loc env ty_record (instance env ty_expected); check_duplicates loc env lbl_exp_list; let label_descriptions, representation = match lbl_exp_list, repr_opt with | (_, { lbl_all = label_descriptions; lbl_repres = representation}, _) :: _, _ -> label_descriptions, representation @@ -2467,7 +2393,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex opath lid_sexp_list) (fun x -> x) in - unify_exp_types ~ctx:"record2" loc env ty_record (instance env ty_expected); + unify_exp_types loc env ty_record (instance env ty_expected); check_duplicates loc env lbl_exp_list; let opt_exp, label_definitions = let (_lid, lbl, _lbl_exp) = List.hd lbl_exp_list in @@ -2479,15 +2405,15 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex let ty_exp = instance env exp.exp_type in let unify_kept lbl = let _, ty_arg1, ty_res1 = instance_label false lbl in - unify_exp_types ~ctx:"record inner" exp.exp_loc env ty_exp ty_res1; + unify_exp_types exp.exp_loc env ty_exp ty_res1; match matching_label lbl with | lid, _lbl, lbl_exp -> (* do not connect result types for overridden labels *) Overridden (lid, lbl_exp) | exception Not_found -> begin let _, ty_arg2, ty_res2 = instance_label false lbl in - unify_exp_types ~ctx:"record inner 2" loc env ty_arg1 ty_arg2; - unify_exp_types ~ctx:"record inner 3" loc env (instance env ty_expected) ty_res2; + unify_exp_types loc env ty_arg1 ty_arg2; + unify_exp_types loc env (instance env ty_expected) ty_res2; Kept ty_arg1 end in @@ -2522,7 +2448,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex | Pexp_field(srecord, lid) -> let (record, label, _) = type_label_access env srecord lid in let (_, ty_arg, ty_res) = instance_label false label in - unify_exp ~ctx:"field" env record ty_res; + unify_exp env record ty_res; rue { exp_desc = Texp_field(record, lid, label); exp_loc = loc; exp_extra = []; @@ -2534,7 +2460,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex let ty_record = if opath = None then newvar () else record.exp_type in let (label_loc, label, newval) = type_label_exp ~typeClashContext:SetRecordField false env loc ty_record (lid, label, snewval) in - unify_exp ~ctx:"set field" env record ty_record; + unify_exp env record ty_record; if label.lbl_mut = Immutable then raise(Error(loc, env, Label_not_mutable lid.txt)); Builtin_attributes.check_deprecated_mutable lid.loc label.lbl_attributes @@ -2548,8 +2474,8 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex | Pexp_array(sargl) -> let ty = newgenvar() in let to_unify = Predef.type_array ty in - unify_exp_types ~ctx:"array" loc env to_unify ty_expected; - let argl = List.map (fun sarg -> type_expect ~typeClashContext:ArrayValue ~ctx:"pexp_array" env sarg ty) sargl in + unify_exp_types loc env to_unify ty_expected; + 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 = []; @@ -2557,10 +2483,10 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex exp_attributes = sexp.pexp_attributes; exp_env = env } | Pexp_ifthenelse(scond, sifso, sifnot) -> - let cond = type_expect ~typeClashContext:IfCondition ~ctx:"if" 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 ~typeClashContext:IfReturn ~ctx:"if 2" 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 = []; @@ -2568,10 +2494,10 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex exp_attributes = sexp.pexp_attributes; exp_env = env } | Some sifnot -> - let ifso = type_expect ~typeClashContext:IfReturn ~ctx:"if 3" env sifso ty_expected in - let ifnot = type_expect ~typeClashContext:IfReturn ~ctx:"if 4" 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 ~typeClashContext:IfReturn ~ctx:"if" 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 = []; @@ -2581,7 +2507,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex end | Pexp_sequence(sexp1, sexp2) -> let exp1 = type_statement env sexp1 in - let exp2 = type_expect ~ctx:"pexp_sequence" env sexp2 ty_expected in + let exp2 = type_expect env sexp2 ty_expected in re { exp_desc = Texp_sequence(exp1, exp2); exp_loc = loc; exp_extra = []; @@ -2589,7 +2515,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex exp_attributes = sexp.pexp_attributes; exp_env = env } | Pexp_while(scond, sbody) -> - let cond = type_expect ~ctx:"pexp_while" env scond Predef.type_bool in + let cond = type_expect env scond Predef.type_bool in let body = type_statement env sbody in rue { exp_desc = Texp_while(cond, body); @@ -2598,8 +2524,8 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex exp_attributes = sexp.pexp_attributes; exp_env = env } | Pexp_for(param, slow, shigh, dir, sbody) -> - let low = type_expect ~ctx:"pexp_for" env slow Predef.type_int in - let high = type_expect ~ctx:"pexp_for 2" env shigh Predef.type_int in + let low = type_expect env slow Predef.type_int in + let high = type_expect env shigh Predef.type_int in let id, new_env = match param.ppat_desc with | Ppat_any -> Ident.create "_for", env @@ -2627,9 +2553,9 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex if separate then begin end_def (); generalize_structure ty; - (type_argument ~ctx:"constraint#1" env sarg ty (instance env ty), instance env ty) + (type_argument env sarg ty (instance env ty), instance env ty) end else - (type_argument ~ctx:"constraint#2" env sarg ty ty, ty) + (type_argument env sarg ty ty, ty) in rue { exp_desc = arg.exp_desc; @@ -2709,10 +2635,10 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex end_def (); generalize_structure ty; generalize_structure ty'; - (type_argument ~ctx:"coerce#1" env sarg ty (instance env ty), + (type_argument env sarg ty (instance env ty), instance env ty', Some cty, cty') end else - (type_argument ~ctx:"coerce#2" env sarg ty ty, ty', Some cty, cty') + (type_argument env sarg ty ty, ty', Some cty, cty') in rue { exp_desc = arg.exp_desc; @@ -2785,7 +2711,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex let (id, new_env) = Env.enter_module name.txt modl.mod_type env in Ctype.init_def(Ident.current_time()); Typetexp.widen context; - let body = type_expect ~ctx:"pexp_letmodule" new_env sbody ty_expected in + let body = type_expect new_env sbody ty_expected in (* go back to original level *) end_def (); (* Unification of body.exp_type with the fresh variable ty @@ -2836,7 +2762,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex | Pexp_lazy e -> let ty = newgenvar () in let to_unify = Predef.type_lazy_t ty in - unify_exp_types ~ctx:"lazy" loc env to_unify ty_expected; + unify_exp_types loc env to_unify ty_expected; let arg = type_expect env e ty in re { exp_desc = Texp_lazy arg; @@ -2855,24 +2781,24 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex repr cty.ctyp_type, Some cty in if sty <> None then - unify_exp_types ~ctx:"poly" loc env (instance env ty) (instance env ty_expected); + unify_exp_types loc env (instance env ty) (instance env ty_expected); let exp = match (expand_head env ty).desc with Tpoly (ty', []) -> - let exp = type_expect ~ctx:"poly2" env sbody ty' in + let exp = type_expect env sbody ty' in { exp with exp_type = instance env ty } | Tpoly (ty', tl) -> (* One more level to generalize locally *) begin_def (); let vars, ty'' = instance_poly true tl ty' in - let exp = type_expect ~ctx:"poly3" env sbody ty'' in + let exp = type_expect env sbody ty'' in end_def (); check_univars env false "method" exp ty_expected vars; { exp with exp_type = instance env ty } | Tvar _ -> let exp = type_exp env sbody in let exp = {exp with exp_type = newty (Tpoly (exp.exp_type, []))} in - unify_exp ~ctx:"tvar" env exp ty; + unify_exp env exp ty; exp | _ -> assert false in @@ -2946,7 +2872,7 @@ and type_expect_ ?typeClashContext ?tctx ?in_function ?(recarg=Rejected) env sex exp_env = env } | Pexp_open (ovf, lid, e) -> let (path, newenv) = !type_open ovf env sexp.pexp_loc lid in - let exp = type_expect ~ctx:"pexp_open" newenv e ty_expected in + let exp = type_expect newenv e ty_expected in { exp with exp_extra = (Texp_open (ovf, path, lid, newenv), loc, sexp.pexp_attributes) :: @@ -3081,7 +3007,7 @@ and type_label_exp ?typeClashContext 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 ?typeClashContext ~ctx:"typelabelexp#1" 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; @@ -3093,7 +3019,7 @@ and type_label_exp ?typeClashContext create env loc ty_expected let arg = type_exp env sarg in end_def (); generalize_expansive env arg.exp_type; - unify_exp ~ctx:"type label exp" env arg ty_arg; + unify_exp env arg ty_arg; check_univars env false "field value" arg label.lbl_arg vars; arg with Error (_, _, Less_general _) as e -> raise e @@ -3101,7 +3027,7 @@ and type_label_exp ?typeClashContext create env loc ty_expected in (lid, label, {arg with exp_type = instance env arg.exp_type}) -and type_argument ?typeClashContext ?ctx ?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 @@ -3135,10 +3061,10 @@ and type_argument ?typeClashContext ?ctx ?recarg env sarg ty_expected' ty_expect let texp = {texp with exp_type = instance env texp.exp_type} and ty_fun = instance env ty_fun' in if not (simple_res || no_labels ty_res) then begin - unify_exp ~ctx:"type argument" env texp ty_expected; + unify_exp env texp ty_expected; texp end else begin - unify_exp ~ctx:"type argument 2" env {texp with exp_type = ty_fun} ty_expected; + unify_exp env {texp with exp_type = ty_fun} ty_expected; if args = [] then texp else (* eta-expand to avoid side effects *) let var_pair name ty = @@ -3181,9 +3107,8 @@ and type_argument ?typeClashContext ?ctx ?recarg env sarg ty_expected' ty_expect func let_var) } end | _ -> - let ctx = Option.value ctx ~default:"" in - let texp = type_expect ?typeClashContext ~ctx:("targ1 " ^ ctx) ?recarg env sarg ty_expected' in - unify_exp ?typeClashContext ~ctx:"type arg 2" 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 *) @@ -3217,7 +3142,7 @@ and type_application ?typeClashContext uncurried env funct (sargs : sargs) : tar begin match (expand_head env funct.exp_type).desc with | Tvar _ | Tarrow _ -> - unify_exp ~ctx:"type application tvar" env funct uncurried_typ + unify_exp env funct uncurried_typ | _ -> raise(Error(funct.exp_loc, env, Apply_non_function (expand_head env funct.exp_type))) end @@ -3299,9 +3224,9 @@ and type_application ?typeClashContext uncurried env funct (sargs : sargs) : tar in let optional = is_optional l1 in let arg1 () = - let arg1 = type_expect ~ctx:"arg1" env sarg1 ty1 in + let arg1 = type_expect env sarg1 ty1 in if optional then - unify_exp ~ctx:"opt" env arg1 (type_option(newvar())); + unify_exp env arg1 (type_option(newvar())); arg1 in type_unknown_args max_arity ~args:((l1, Some arg1) :: args) omitted ty2 sargl @@ -3336,9 +3261,9 @@ and type_application ?typeClashContext uncurried env funct (sargs : sargs) : tar | Pexp_constant (Pconst_integer (txt, _) | Pconst_float (txt, _)) -> Some txt | _ -> None}) | typeClashContext -> typeClashContext) in - (fun () -> type_argument ?typeClashContext ~ctx:"opt#1" env sarg0 ty ty0) + (fun () -> type_argument ?typeClashContext env sarg0 ty ty0) )else - (fun () -> option_some (type_argument ?typeClashContext env ~ctx:"opt#2" sarg0 + (fun () -> option_some (type_argument ?typeClashContext env sarg0 (extract_option_type env ty) (extract_option_type env ty0)))) in @@ -3366,7 +3291,7 @@ and type_application ?typeClashContext uncurried env funct (sargs : sargs) : tar let ty_arg, ty_res = filter_arrow env (instance env funct.exp_type) Nolabel in - let exp = type_expect ~ctx:"sargs" env sarg ty_arg in + let exp = type_expect env sarg ty_arg in begin match (expand_head env exp.exp_type).desc with | Tarrow _ -> Location.prerr_warning exp.exp_loc Warnings.Partial_application @@ -3424,7 +3349,7 @@ and type_construct env loc lid sarg ty_expected attrs = if separate then begin end_def (); generalize_structure ty_res; - unify_exp ?typeClashContext ~ctx:"type construct" 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; @@ -3436,7 +3361,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 ?typeClashContext ~ctx:"type construct 2" 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 @@ -3451,7 +3376,7 @@ and type_construct env loc lid sarg ty_expected attrs = end in let args = - List.map2 (fun e (t,t0) -> type_argument ~ctx:"typeconstruct#1" ~recarg env e t t0) sargs + List.map2 (fun e (t,t0) -> type_argument ~recarg env e t t0) sargs (List.combine ty_args ty_args0) in if constr.cstr_private = Private then raise(Error(loc, env, Private_type ty_res)); @@ -3474,7 +3399,7 @@ and type_statement env sexp = | Pexp_apply _ -> Some(Statement(FunctionCall)) | _ -> None ) in - unify_exp ?typeClashContext ~ctx:"type statement" env exp expected_ty; + unify_exp ?typeClashContext env exp expected_ty; exp (* Typing of match cases *) @@ -3586,10 +3511,10 @@ and type_cases ?rootTypeClashContext ?in_function env ty_arg ty_res partial_flag | None -> None | Some scond -> Some - (type_expect ?typeClashContext:(match rootTypeClashContext with | Some _ -> Some IfCondition | None -> None) ~ctx:"guard" ext_env (wrap_unpacks scond unpacks) + (type_expect ?typeClashContext:(match rootTypeClashContext with | Some _ -> Some IfCondition | None -> None) ext_env (wrap_unpacks scond unpacks) Predef.type_bool) in - let exp = type_expect ?typeClashContext:rootTypeClashContext ~ctx:"guard exp" ?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; @@ -3600,7 +3525,7 @@ and type_cases ?rootTypeClashContext ?in_function env ty_arg ty_res partial_flag in if has_gadts then begin let ty_res' = instance env ty_res in - List.iter (fun c -> unify_exp ~ctx:"gadts" env c.c_rhs ty_res') cases + List.iter (fun c -> unify_exp env c.c_rhs ty_res') cases end; let do_init = has_gadts || needs_exhaust_check in let lev, env = @@ -3631,7 +3556,7 @@ and type_cases ?rootTypeClashContext ?in_function env ty_arg ty_res partial_flag if do_init then begin end_def (); (* Ensure that existential types do not escape *) - unify_exp_types ~ctx:"type cases" loc env (instance env ty_res) (newvar ()) ; + unify_exp_types loc env (instance env ty_res) (newvar ()) ; end; cases, partial @@ -3766,14 +3691,14 @@ and type_let ?(check = fun s -> Warnings.Unused_var s) let vars, ty' = instance_poly ~keep_names:true true tl ty in let exp = Builtin_attributes.warning_scope pvb_attributes - (fun () -> type_expect ~ctx:"polyy" exp_env sexp ty') + (fun () -> type_expect exp_env sexp ty') in end_def (); check_univars env true "definition" exp pat.pat_type vars; {exp with exp_type = instance env exp.exp_type} | _ -> Builtin_attributes.warning_scope pvb_attributes (fun () -> - type_expect ~ctx:"poly rest" exp_env sexp pat.pat_type)) + type_expect exp_env sexp pat.pat_type)) spat_sexp_list pat_slot_list in current_slot := None; if is_recursive && not !rec_needed diff --git a/jscomp/ml/typecore.mli b/jscomp/ml/typecore.mli index 925d76d668..650bae0d5f 100644 --- a/jscomp/ml/typecore.mli +++ b/jscomp/ml/typecore.mli @@ -60,9 +60,6 @@ val name_pattern : string -> Typedtree.case list -> Ident.t val self_coercion : (Path.t * Location.t list ref) list ref -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 - type error = Polymorphic_label of Longident.t | Constructor_arity_mismatch of Longident.t * int * int @@ -71,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 * (typeClashContext option) + | 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 From 89ba484f8ae536cf5971fa050f0727ce13ed4a72 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 24 Sep 2023 19:56:37 +0200 Subject: [PATCH 09/14] cleanup --- jscomp/ml/typecore.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 539b70dcc1..e6adcbf8a3 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -712,9 +712,9 @@ let print_expr_type_clash ?typeClashContext env trace ppf = begin Printtyp.super_report_unification_error ppf env trace (function ppf -> - errorTypeText ppf typeClashContext) + errorTypeText ppf typeClashContext) (function ppf -> - errorExpectedTypeText ppf typeClashContext); + errorExpectedTypeText ppf typeClashContext); printExtraTypeClashHelp ppf trace typeClashContext; show_extra_help ppf env trace; end From 0a351364b22ee417a8b0c5b6f51e159c335a9f64 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 24 Sep 2023 20:02:46 +0200 Subject: [PATCH 10/14] cleanup --- jscomp/ml/error_message_utils.ml | 19 +++++++++++++++++++ jscomp/ml/typecore.ml | 11 +---------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/jscomp/ml/error_message_utils.ml b/jscomp/ml/error_message_utils.ml index b5f758d184..c82cf2c1f0 100644 --- a/jscomp/ml/error_message_utils.ml +++ b/jscomp/ml/error_message_utils.ml @@ -147,3 +147,22 @@ let printExtraTypeClashHelp ppf trace typeClashContext = \ - Use a tuple, if your array is of fixed length. Tuples can mix types \ freely, and compiles to a JavaScript array." | _ -> () + +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 + \ No newline at end of file diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index e6adcbf8a3..5a624ed2db 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -2140,16 +2140,7 @@ and type_expect_ ?typeClashContext ?in_function ?(recarg=Rejected) env sexp ty_e 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 isConstant = (match sexp.pexp_desc with - | Pexp_constant (Pconst_integer(txt, _) | Pconst_float (txt, _)) -> Some txt - | _ -> None) in - let typeClashContext = (match sfunct.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) - ) 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; From a878602c7652a7d73e2ca7518b796a13c663d6ee Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 24 Sep 2023 20:20:07 +0200 Subject: [PATCH 11/14] cleanup --- jscomp/ml/error_message_utils.ml | 31 +++++++++++++++++++++++++++++++ jscomp/ml/typecore.ml | 20 ++++---------------- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/jscomp/ml/error_message_utils.ml b/jscomp/ml/error_message_utils.ml index c82cf2c1f0..81251b0e59 100644 --- a/jscomp/ml/error_message_utils.ml +++ b/jscomp/ml/error_message_utils.ml @@ -165,4 +165,35 @@ let typeClashContextFromFunction sexp sfunct = | 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 \ No newline at end of file diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 5a624ed2db..576139ee58 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -3245,13 +3245,7 @@ and type_application ?typeClashContext uncurried env funct (sargs : sargs) : tar sargs, omitted , Some ( if not optional || is_optional l' then( - let typeClashContext = (match typeClashContext with - | Some MathOperator {forFloat; operator} -> Some - (MathOperator {forFloat; operator; isConstant = - match sarg0.pexp_desc with - | Pexp_constant (Pconst_integer (txt, _) | Pconst_float (txt, _)) -> Some txt - | _ -> None}) - | typeClashContext -> typeClashContext) in + let typeClashContext = typeClashContextForFunctionArgument typeClashContext sarg0 in (fun () -> type_argument ?typeClashContext env sarg0 ty ty0) )else (fun () -> option_some (type_argument ?typeClashContext env sarg0 @@ -3333,10 +3327,7 @@ and type_construct env loc lid sarg ty_expected attrs = exp_type = ty_res; exp_attributes = attrs; exp_env = env } in - let typeClashContext = (match ty_expected, ty_res with - | {desc=Tconstr (expectedPath, _, _)}, {desc=Tconstr (typePath, _, _)} - when Path.same Predef.path_option typePath && Path.same expectedPath Predef.path_option = false -> Some MaybeUnwrapOption - | _ -> None) in + let typeClashContext = typeClashContextMaybeOption ty_expected ty_res in if separate then begin end_def (); generalize_structure ty_res; @@ -3386,10 +3377,7 @@ 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 - let typeClashContext = (match sexp.pexp_desc with - | Pexp_apply _ -> Some(Statement(FunctionCall)) - | _ -> None - ) in + let typeClashContext = typeClashContextInStatement sexp in unify_exp ?typeClashContext env exp expected_ty; exp @@ -3502,7 +3490,7 @@ and type_cases ?rootTypeClashContext ?in_function env ty_arg ty_res partial_flag | None -> None | Some scond -> Some - (type_expect ?typeClashContext:(match rootTypeClashContext with | Some _ -> Some IfCondition | None -> None) 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 ?typeClashContext:rootTypeClashContext ?in_function ext_env sexp ty_res' in From 99bbf9fa4bac5894318cc86a616a79700a2c6eba Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 24 Sep 2023 20:22:02 +0200 Subject: [PATCH 12/14] cleanup --- jscomp/ml/typecore.ml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 576139ee58..111ac9fe9e 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -3244,10 +3244,9 @@ and type_application ?typeClashContext uncurried env funct (sargs : sargs) : tar (Warnings.Nonoptional_label (Printtyp.string_of_label l)); sargs, omitted , Some ( - if not optional || is_optional l' then( - let typeClashContext = typeClashContextForFunctionArgument typeClashContext sarg0 in - (fun () -> type_argument ?typeClashContext env sarg0 ty ty0) - )else + if not optional || is_optional l' then + (fun () -> type_argument ?typeClashContext:(typeClashContextForFunctionArgument typeClashContext sarg0) env sarg0 ty ty0) + else (fun () -> option_some (type_argument ?typeClashContext env sarg0 (extract_option_type env ty) (extract_option_type env ty0)))) From f85fdc4986e790dc09a93d3bf5cf12316fcad896 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 24 Sep 2023 20:24:55 +0200 Subject: [PATCH 13/14] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) 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 From cd464f2156847c777780debcc517921464811c35 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 24 Sep 2023 20:39:12 +0200 Subject: [PATCH 14/14] add example of tuple to error message --- .../array_item_type_mismatch.res.expected | 2 +- jscomp/ml/error_message_utils.ml | 28 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) 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 index e42a839d8d..54c99ad256 100644 --- 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 @@ -12,6 +12,6 @@ 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. + - 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/ml/error_message_utils.ml b/jscomp/ml/error_message_utils.ml index 81251b0e59..f7ab333bb8 100644 --- a/jscomp/ml/error_message_utils.ml +++ b/jscomp/ml/error_message_utils.ml @@ -145,7 +145,8 @@ let printExtraTypeClashHelp ppf trace typeClashContext = \ 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." + freely, and compiles to a JavaScript array. Example of a tuple: `let \ + myTuple = (10, \"hello\", 15.5, true)" | _ -> () let typeClashContextFromFunction sexp sfunct = @@ -165,22 +166,22 @@ let typeClashContextFromFunction sexp sfunct = | 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); - }) + { + 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 = @@ -188,7 +189,7 @@ let typeClashContextMaybeOption ty_expected ty_res = | ( {Types.desc = Tconstr (expectedPath, _, _)}, {Types.desc = Tconstr (typePath, _, _)} ) when Path.same Predef.path_option typePath - && Path.same expectedPath Predef.path_option = false -> + && Path.same expectedPath Predef.path_option = false -> Some MaybeUnwrapOption | _ -> None @@ -196,4 +197,3 @@ let typeClashContextInStatement sexp = match sexp.Parsetree.pexp_desc with | Pexp_apply _ -> Some (Statement FunctionCall) | _ -> None - \ No newline at end of file