From b9cf72944b7f28e8a559da5537b3d0aee8100ef4 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 21 May 2025 17:18:29 +0200 Subject: [PATCH 1/4] suggest awaiting promise before using it when type mismatches --- compiler/ml/error_message_utils.ml | 32 ++++++++----------- compiler/ml/typecore.ml | 4 +-- .../expected/promise_needs_await.res.expected | 13 ++++++++ .../fixtures/promise_needs_await.res | 10 ++++++ 4 files changed, 38 insertions(+), 21 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/promise_needs_await.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/promise_needs_await.res diff --git a/compiler/ml/error_message_utils.ml b/compiler/ml/error_message_utils.ml index 819d2287d5..31d8cafc3b 100644 --- a/compiler/ml/error_message_utils.ml +++ b/compiler/ml/error_message_utils.ml @@ -62,9 +62,10 @@ let is_record_type ~extract_concrete_typedecl ~env ty = | _ -> false with _ -> false -let print_extra_type_clash_help ~extract_concrete_typedecl ~env ppf trace +let print_extra_type_clash_help ~extract_concrete_typedecl ~env ppf + (bottom_aliases : (Types.type_expr * Types.type_expr) option) type_clash_context = - match (type_clash_context, trace) with + match (type_clash_context, bottom_aliases) with | Some (MathOperator {for_float; operator; is_constant}), _ -> ( let operator_for_other_type = match operator with @@ -86,12 +87,8 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env ppf trace | _ -> "compute" in (* TODO check int vs float explicitly before showing this *) - (match (operator, trace) with - | ( "+", - [ - ({Types.desc = Tconstr (p1, _, _)}, _); - ({desc = Tconstr (p2, _, _)}, _); - ] ) + (match (operator, bottom_aliases) with + | "+", Some ({Types.desc = Tconstr (p1, _, _)}, {desc = Tconstr (p2, _, _)}) when Path.same Predef.path_string p1 || Path.same Predef.path_string p2 -> fprintf ppf "\n\n\ @@ -111,7 +108,7 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env ppf trace @{Belt.Float.toInt@} and @{Belt.Int.fromFloat@}." operator_text (if for_float then "float" else "int")); - match (is_constant, trace) with + match (is_constant, bottom_aliases) with | Some constant, _ -> if for_float then fprintf ppf @@ -125,11 +122,8 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env ppf trace \ - Make @{%s@} an @{int@} by removing the dot or \ explicitly converting to int" constant - | ( _, - [ - ({Types.desc = Tconstr (p1, _, _)}, _); - ({desc = Tconstr (p2, _, _)}, _); - ] ) -> ( + | _, Some ({Types.desc = Tconstr (p1, _, _)}, {desc = Tconstr (p2, _, _)}) + -> ( match (Path.name p1, Path.name p2) with | "float", "int" | "int", "float" -> fprintf ppf @@ -171,10 +165,7 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env ppf trace \ - 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)" - | ( _, - [ - ({Types.desc = Tconstr (_p1, _, _)}, _); ({desc = Tconstr (p2, _, _)}, _); - ] ) + | _, Some ({Types.desc = Tconstr (_p1, _, _)}, {desc = Tconstr (p2, _, _)}) when Path.same Predef.path_unit p2 -> fprintf ppf "\n\n\ @@ -182,7 +173,7 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env ppf trace \ - If you don't care about the result of this expression, you can \ assign it to @{_@} via @{let _ = ...@} or pipe it to \ @{ignore@} via @{expression->ignore@}\n\n" - | _, [({desc = Tobject _}, _); (({Types.desc = Tconstr _} as t1), _)] + | _, Some ({desc = Tobject _}, ({Types.desc = Tconstr _} as t1)) when is_record_type ~extract_concrete_typedecl ~env t1 -> fprintf ppf "\n\n\ @@ -191,6 +182,9 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env ppf trace \ - Did you mean to pass a record instead of an object? Objects are \ written with quoted keys, and records with unquoted keys. Remove the \ quotes from the object keys to pass it as a record instead of object. \n\n" + | _, Some ({Types.desc = Tconstr (p1, _, _)}, _) + when Path.same p1 Predef.path_promise -> + fprintf ppf "\n\n - Did you mean to await this promise before using it?\n" | _ -> () let type_clash_context_from_function sexp sfunct = diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index bb96310964..e762315655 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -784,8 +784,8 @@ let print_expr_type_clash ?type_clash_context env trace ppf = (function | ppf -> error_type_text ppf type_clash_context) (function ppf -> error_expected_type_text ppf type_clash_context); - print_extra_type_clash_help ~extract_concrete_typedecl ~env ppf trace - type_clash_context; + print_extra_type_clash_help ~extract_concrete_typedecl ~env ppf + bottom_aliases_result type_clash_context; show_extra_help ppf env trace let report_arity_mismatch ~arity_a ~arity_b ppf = diff --git a/tests/build_tests/super_errors/expected/promise_needs_await.res.expected b/tests/build_tests/super_errors/expected/promise_needs_await.res.expected new file mode 100644 index 0000000000..816aee3d01 --- /dev/null +++ b/tests/build_tests/super_errors/expected/promise_needs_await.res.expected @@ -0,0 +1,13 @@ + + We've found a bug for you! + /.../fixtures/promise_needs_await.res:9:3-5 + + 7 │ let x = () => { + 8 │ let res = Promise.resolve({one: "hi"}) + 9 │ res.one + 10 │ } + + This has type: Promise.t (defined as promise) + But it's expected to have type: record + + - Did you mean to await this promise before using it? \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/promise_needs_await.res b/tests/build_tests/super_errors/fixtures/promise_needs_await.res new file mode 100644 index 0000000000..617936637e --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/promise_needs_await.res @@ -0,0 +1,10 @@ +type record = { + one: string +} + +external getRecord: unit => promise = "getRecord" + +let x = () => { + let res = Promise.resolve({one: "hi"}) + res.one +} \ No newline at end of file From 6ed77788c2a2dceaa06881f4fcdcb7f66d998032 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 21 May 2025 17:48:07 +0200 Subject: [PATCH 2/4] format --- .../super_errors/fixtures/promise_needs_await.res | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/build_tests/super_errors/fixtures/promise_needs_await.res b/tests/build_tests/super_errors/fixtures/promise_needs_await.res index 617936637e..5e118a2843 100644 --- a/tests/build_tests/super_errors/fixtures/promise_needs_await.res +++ b/tests/build_tests/super_errors/fixtures/promise_needs_await.res @@ -1,10 +1,8 @@ -type record = { - one: string -} +type record = {one: string} external getRecord: unit => promise = "getRecord" let x = () => { let res = Promise.resolve({one: "hi"}) res.one -} \ No newline at end of file +} From efc69360d8263b98a37c4e18ea8c74a5fb150c75 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 21 May 2025 17:49:13 +0200 Subject: [PATCH 3/4] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed93f9f578..50e01c9a9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ - `rescript-tools doc` no longer includes shadowed bindings in its output. https://github.com/rescript-lang/rescript/pull/7497 +#### :nail_care: Polish + +- Suggest awaiting promise before using it when types mismatch. https://github.com/rescript-lang/rescript/pull/7498 + # 12.0.0-alpha.13 #### :boom: Breaking Change From dbe815302e9b208ac69a6f0b6b9ea7bff7fb90c3 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 21 May 2025 18:24:54 +0200 Subject: [PATCH 4/4] update output --- .../expected/promise_needs_await.res.expected | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/build_tests/super_errors/expected/promise_needs_await.res.expected b/tests/build_tests/super_errors/expected/promise_needs_await.res.expected index 816aee3d01..a9d5a856d2 100644 --- a/tests/build_tests/super_errors/expected/promise_needs_await.res.expected +++ b/tests/build_tests/super_errors/expected/promise_needs_await.res.expected @@ -1,11 +1,12 @@ We've found a bug for you! - /.../fixtures/promise_needs_await.res:9:3-5 + /.../fixtures/promise_needs_await.res:7:3-5 - 7 │ let x = () => { - 8 │ let res = Promise.resolve({one: "hi"}) - 9 │ res.one - 10 │ } + 5 │ let x = () => { + 6 │ let res = Promise.resolve({one: "hi"}) + 7 │ res.one + 8 │ } + 9 │ This has type: Promise.t (defined as promise) But it's expected to have type: record