more work on error messages

zth · zth · commit 15b185579cb8 · 2025-08-27T09:27:32.000+02:00
diff --git a/compiler/ml/error_message_utils.ml b/compiler/ml/error_message_utils.ml
@@ -166,8 +166,7 @@ let error_expected_type_text ppf type_clash_context =
     fprintf ppf "But it's being compared to something of type:"
   | Some SwitchReturn -> fprintf ppf "But this switch is expected to return:"
   | Some LetUnwrapReturn ->
-    fprintf ppf
-      "But this @{<info>let?@} is used in a context expecting the type:"
+    fprintf ppf "But this @{<info>let?@} is used where this type is expected:"
   | Some TryReturn -> fprintf ppf "But this try/catch is expected to return:"
   | Some WhileCondition ->
     fprintf ppf "But a @{<info>while@} loop condition must always be of type:"
@@ -319,11 +318,65 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf
       "\n\n\
       \  All branches in a @{<info>switch@} must return the same type.@,\
        To fix this, change your branch to return the expected type."
-  | Some LetUnwrapReturn, _ ->
-    fprintf ppf
-      "\n\n\
-      \  @{<info>let?@} can only be used in a context that expects \
-       @{<info>option@} or @{<info>result@}."
+  | Some LetUnwrapReturn, bottom_aliases -> (
+    let kind =
+      match bottom_aliases with
+      | Some ({Types.desc = Tconstr (p, _, _)}, _)
+        when Path.same p Predef.path_option ->
+        `Option
+      | Some (_, {Types.desc = Tconstr (p, _, _)})
+        when Path.same p Predef.path_option ->
+        `Option
+      | Some ({Types.desc = Tconstr (p, _, _)}, _)
+        when Path.same p Predef.path_result ->
+        `Result
+      | Some (_, {Types.desc = Tconstr (p, _, _)})
+        when Path.same p Predef.path_result ->
+        `Result
+      | _ -> `Unknown
+    in
+    match kind with
+    | `Option ->
+      fprintf ppf
+        "\n\n\
+        \  This @{<info>let?@} unwraps an @{<info>option@}; use it where the \
+         enclosing function or let binding returns an @{<info>option@} so \
+         @{<info>None@} can propagate.\n\n\
+        \  Possible solutions:\n\
+        \  - Change the enclosing function or let binding to return \
+         @{<info>option<'t>@} and use @{<info>Some@} for success; \
+         @{<info>let?@} will propagate @{<info>None@}.\n\
+        \  - Replace @{<info>let?@} with a @{<info>switch@} and handle the \
+         @{<info>None@} case explicitly.\n\
+        \  - If you want a default value instead of early return, unwrap using \
+         @{<info>Option.getOr(default)@}."
+    | `Result ->
+      fprintf ppf
+        "\n\n\
+        \  This @{<info>let?@} unwraps a @{<info>result@}; use it where the \
+         enclosing function or let binding returns a @{<info>result@} so \
+         @{<info>Error@} can propagate.\n\n\
+        \  Possible solutions:\n\
+        \  - Change the enclosing function or let binding to return \
+         @{<info>result<'ok, 'error>@}; use @{<info>Ok@} for success, and \
+         @{<info>let?@} will propagate @{<info>Error@}.\n\
+        \  - Replace @{<info>let?@} with a @{<info>switch@} and handle the \
+         @{<info>Error@} case explicitly.\n\
+        \  - If you want a default value instead of early return, unwrap using \
+         @{<info>Result.getOr(default)@}."
+    | `Unknown ->
+      fprintf ppf
+        "\n\n\
+        \  @{<info>let?@} can only be used in a context that expects \
+         @{<info>option@} or @{<info>result@}.\n\n\
+        \  Possible solutions:\n\
+        \  - Change the enclosing function or let binding to return an \
+         @{<info>option<'t>@} or @{<info>result<'ok, 'error>@} and propagate \
+         with @{<info>Some/Ok@}.\n\
+        \  - Replace @{<info>let?@} with a @{<info>switch@} and handle the \
+         @{<info>None/Error@} case explicitly.\n\
+        \  - If you want a default value instead of early return, unwrap using \
+         @{<info>Option.getOr(default)@} or @{<info>Result.getOr(default)@}.")
   | Some TryReturn, _ ->
     fprintf ppf
       "\n\n\
diff --git a/tests/build_tests/super_errors/expected/let_unwrap_return_type_mismatch.res.expected b/tests/build_tests/super_errors/expected/let_unwrap_return_type_mismatch.res.expected
@@ -9,6 +9,11 @@
   8 [2m│[0m }
 
   This has type: [1;31moption<'a>[0m
-  But this [1;33mlet?[0m is used in a context expecting the type: [1;33mint[0m
+  But this [1;33mlet?[0m is used where this type is expected: [1;33mint[0m
 
-  [1;33mlet?[0m can only be used in a context that expects [1;33moption[0m or [1;33mresult[0m.
+  This [1;33mlet?[0m unwraps an [1;33moption[0m; use it where the enclosing function or let binding returns an [1;33moption[0m so [1;33mNone[0m can propagate.
+
+  Possible solutions:
+  - Change the enclosing function or let binding to return [1;33moption<'t>[0m and use [1;33mSome[0m for success; [1;33mlet?[0m will propagate [1;33mNone[0m.
+  - Replace [1;33mlet?[0m with a [1;33mswitch[0m and handle the [1;33mNone[0m case explicitly.
+  - If you want a default value instead of early return, unwrap using [1;33mOption.getOr(default)[0m.
diff --git a/tests/build_tests/super_errors/expected/let_unwrap_return_type_mismatch_block.res.expected b/tests/build_tests/super_errors/expected/let_unwrap_return_type_mismatch_block.res.expected
@@ -0,0 +1,19 @@
+
+  [1;31mWe've found a bug for you![0m
+  [36m/.../fixtures/let_unwrap_return_type_mismatch_block.res[0m:[2m10:12-18[0m
+
+   8 [2m┆[0m   1
+   9 [2m┆[0m } else {
+  [1;31m10[0m [2m┆[0m   let? [1;31mSome(x)[0m = None
+  11 [2m┆[0m   Some(x)
+  12 [2m┆[0m }
+
+  This has type: [1;31moption<'a>[0m
+  But this [1;33mlet?[0m is used where this type is expected: [1;33mint[0m
+
+  This [1;33mlet?[0m unwraps an [1;33moption[0m; use it where the enclosing function or let binding returns an [1;33moption[0m so [1;33mNone[0m can propagate.
+
+  Possible solutions:
+  - Change the enclosing function or let binding to return [1;33moption<'t>[0m and use [1;33mSome[0m for success; [1;33mlet?[0m will propagate [1;33mNone[0m.
+  - Replace [1;33mlet?[0m with a [1;33mswitch[0m and handle the [1;33mNone[0m case explicitly.
+  - If you want a default value instead of early return, unwrap using [1;33mOption.getOr(default)[0m.
diff --git a/tests/build_tests/super_errors/expected/let_unwrap_return_type_mismatch_result.res.expected b/tests/build_tests/super_errors/expected/let_unwrap_return_type_mismatch_result.res.expected
@@ -0,0 +1,19 @@
+
+  [1;31mWe've found a bug for you![0m
+  [36m/.../fixtures/let_unwrap_return_type_mismatch_result.res[0m:[2m6:8-12[0m
+
+  4 [2m│[0m 
+  5 [2m│[0m let fn = (): int => {
+  [1;31m6[0m [2m│[0m   let? [1;31mOk(v)[0m = Error("fail")
+  7 [2m│[0m   42
+  8 [2m│[0m }
+
+  This has type: [1;31mresult<'a, string>[0m
+  But this [1;33mlet?[0m is used where this type is expected: [1;33mint[0m
+
+  This [1;33mlet?[0m unwraps a [1;33mresult[0m; use it where the enclosing function or let binding returns a [1;33mresult[0m so [1;33mError[0m can propagate.
+
+  Possible solutions:
+  - Change the enclosing function or let binding to return [1;33mresult<'ok, 'error>[0m; use [1;33mOk[0m for success, and [1;33mlet?[0m will propagate [1;33mError[0m.
+  - Replace [1;33mlet?[0m with a [1;33mswitch[0m and handle the [1;33mError[0m case explicitly.
+  - If you want a default value instead of early return, unwrap using [1;33mResult.getOr(default)[0m.
diff --git a/tests/build_tests/super_errors/fixtures/let_unwrap_return_type_mismatch_block.res b/tests/build_tests/super_errors/fixtures/let_unwrap_return_type_mismatch_block.res
@@ -0,0 +1,15 @@
+@@config({flags: ["-enable-experimental", "LetUnwrap"]})
+
+let x = Some(1)
+
+let fn = (): int => {
+  let x = {
+    if 1 > 2 {
+      1
+    } else {
+      let? Some(x) = None
+      Some(x)
+    }
+  }
+  42
+}
diff --git a/tests/build_tests/super_errors/fixtures/let_unwrap_return_type_mismatch_result.res b/tests/build_tests/super_errors/fixtures/let_unwrap_return_type_mismatch_result.res
@@ -0,0 +1,8 @@
+@@config({flags: ["-enable-experimental", "LetUnwrap"]})
+
+let x = Ok(1)
+
+let fn = (): int => {
+  let? Ok(v) = Error("fail")
+  42
+}

-Original file line number
+Diff line change
 [2m│[0m }
   This has type: [1;31moption<'a>[0m
 -  But this [1;33mlet?[0m is used in a context expecting the type: [1;33mint[0m
 +  But this [1;33mlet?[0m is used where this type is expected: [1;33mint[0m
 -  [1;33mlet?[0m can only be used in a context that expects [1;33moption[0m or [1;33mresult[0m.
 +  This [1;33mlet?[0m unwraps an [1;33moption[0m; use it where the enclosing function or let binding returns an [1;33moption[0m so [1;33mNone[0m can propagate.
++
 +  Possible solutions:
 +  - Change the enclosing function or let binding to return [1;33moption<'t>[0m and use [1;33mSome[0m for success; [1;33mlet?[0m will propagate [1;33mNone[0m.
 +  - Replace [1;33mlet?[0m with a [1;33mswitch[0m and handle the [1;33mNone[0m case explicitly.
 +  - If you want a default value instead of early return, unwrap using [1;33mOption.getOr(default)[0m.
-Original file line number
+Diff line change
@@ @@ -0,0 +1,19 @@ @@
++
 +  [1;31mWe've found a bug for you![0m
 +  [36m/.../fixtures/let_unwrap_return_type_mismatch_result.res[0m:[2m6:8-12[0m
++
 +  4 [2m│[0m
 +  5 [2m│[0m let fn = (): int => {
 +  [1;31m6[0m [2m│[0m   let? [1;31mOk(v)[0m = Error("fail")
 +  7 [2m│[0m   42
 +  8 [2m│[0m }
++
 +  This has type: [1;31mresult<'a, string>[0m
 +  But this [1;33mlet?[0m is used where this type is expected: [1;33mint[0m
++
 +  This [1;33mlet?[0m unwraps a [1;33mresult[0m; use it where the enclosing function or let binding returns a [1;33mresult[0m so [1;33mError[0m can propagate.
++
 +  Possible solutions:
 +  - Change the enclosing function or let binding to return [1;33mresult<'ok, 'error>[0m; use [1;33mOk[0m for success, and [1;33mlet?[0m will propagate [1;33mError[0m.
 +  - Replace [1;33mlet?[0m with a [1;33mswitch[0m and handle the [1;33mError[0m case explicitly.
 +  - If you want a default value instead of early return, unwrap using [1;33mResult.getOr(default)[0m.