Fix optional default-arg typing and improve generated match errors

CHANGELOG.md

@@ -34,6 +34,7 @@
 - Rewatch: preserve warnings after atomic-save full rebuilds. https://github.com/rescript-lang/rescript/pull/8358
 - Preserve JSX prop locations across the AST0 translation layer, fixing `0:0` editor diagnostics in PPX-related flows. https://github.com/rescript-lang/rescript/pull/8350
 - Fix type lowering for `dict{}` and `async`, so you don't need to annotate one extra time when the type is known. https://github.com/rescript-lang/rescript/pull/8359
+- Fix optional default-arg typing in plain functions and JSX components, and improve generated match errors. https://github.com/rescript-lang/rescript/pull/8386
 - Rewatch: don't suppress progress messages under `-v`/`-vv`. https://github.com/rescript-lang/rescript/pull/8371
 - Rewatch: print 'Finished compilation' in watch plain output mode. https://github.com/rescript-lang/rescript/pull/8372
 

compiler/ml/parsetree_utils.ml

@@ -0,0 +1,58 @@
+open Parsetree
+open Asttypes
+open Longident
+
+(* Optional arguments with defaults are lowered in two steps:
+
+     let f = (~test: option<int>=42) => body
+
+   first binds the raw optional argument, then rebinds the resolved value:
+
+     let *opt* = test in
+     let test = switch *opt* {
+     | Some(x) => x
+     | None => 42
+     }
+
+   The second binding is no longer option<int>; it is plain int. This helper
+   rewrites the original pattern so that:
+
+     (~test: option<int>=42)
+
+   becomes a resolved-value pattern equivalent to:
+
+     (~test: int=42)
+
+   for the synthesized rebinding step only. *)
+let rec adapt_pattern_for_resolved_default pattern =
+  match pattern.ppat_desc with
+  | Ppat_constraint
+      ( inner_pattern,
+        ({ptyp_desc = Ptyp_constr ({txt = Lident "option"}, [inner_type])} as
+         inner_constraint) ) ->
+    (* After resolving Some/None, the value has type t, not option<t>. *)
+    {
+      pattern with
+      ppat_desc =
+        Ppat_constraint
+          ( adapt_pattern_for_resolved_default inner_pattern,
+            {inner_constraint with ptyp_desc = inner_type.ptyp_desc} );
+    }
+  | Ppat_constraint
+      (inner_pattern, ({ptyp_desc = Ptyp_package _} as inner_constraint)) ->
+    (* Preserve first-class module constraints. We still recurse so an outer
+       option<...> annotation can be removed if present. *)
+    {
+      pattern with
+      ppat_desc =
+        Ppat_constraint
+          (adapt_pattern_for_resolved_default inner_pattern, inner_constraint);
+    }
+  | Ppat_constraint (inner_pattern, inner_constraint) ->
+    {
+      pattern with
+      ppat_desc =
+        Ppat_constraint
+          (adapt_pattern_for_resolved_default inner_pattern, inner_constraint);
+    }
+  | _ -> pattern

compiler/ml/parsetree_utils.mli

@@ -0,0 +1 @@
+val adapt_pattern_for_resolved_default : Parsetree.pattern -> Parsetree.pattern

compiler/ml/typecore.ml

@@ -2452,14 +2452,18 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected)
     in
     let smatch =
       Exp.match_ ~loc:sloc
+        ~attrs:[(mknoloc "#optional_arg_default", PStr [])]
         (Exp.ident ~loc (mknoloc (Longident.Lident "*opt*")))
         scases
     in
     let pat = Pat.var ~loc:sloc (mknoloc "*opt*") in
+    let resolved_spat =
+      Parsetree_utils.adapt_pattern_for_resolved_default spat
+    in
     let body =
       Exp.let_ ~loc Nonrecursive
         ~attrs:[(mknoloc "#default", PStr [])]
-        [Vb.mk spat smatch]
+        [Vb.mk resolved_spat smatch]
         sbody
     in
     type_function ?in_function ~arity ~async loc sexp.pexp_attributes env
@@ -2540,13 +2544,15 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected)
     (* Note: val_caselist = [] and exn_caselist = [], i.e. a fully
        empty pattern matching can be generated by Camlp4 with its
        revised syntax.  Let's accept it for backward compatibility. *)
+    let has_attr name =
+      Ext_list.exists sexp.pexp_attributes (fun ({txt}, _) -> txt = name)
+    in
+    (* Optional-default lowering synthesizes a match expression, but from the
+       user's point of view this is still part of a function definition. Use
+       function-style diagnostics instead of reporting a phantom switch. *)
     let call_context =
-      if
-        Ext_list.exists sexp.pexp_attributes (fun ({txt}, _) ->
-            match txt with
-            | "let.unwrap" -> true
-            | _ -> false)
-      then `LetUnwrap
+      if has_attr "let.unwrap" then `LetUnwrap
+      else if has_attr "#optional_arg_default" then `Function
       else `Switch
     in
     let val_cases, partial =

compiler/syntax/src/jsx_v4.ml

@@ -510,6 +510,9 @@ let vb_match ~expr (name, default, pattern, _alias, loc, _) =
   match default with
   | Some default ->
     let resolved_name = "__" ^ label ^ "_value" in
+    let resolved_pattern =
+      Parsetree_utils.adapt_pattern_for_resolved_default pattern
+    in
     let value_binding =
       Vb.mk
         (Pat.var (Location.mkloc resolved_name loc))
@@ -528,7 +531,10 @@ let vb_match ~expr (name, default, pattern, _alias, loc, _) =
     in
     Exp.let_ Nonrecursive [value_binding]
       (Exp.let_ Nonrecursive
-         [Vb.mk pattern (Exp.ident (Location.mknoloc @@ Lident resolved_name))]
+         [
+           Vb.mk resolved_pattern
+             (Exp.ident (Location.mknoloc @@ Lident resolved_name));
+         ]
          expr)
   | None -> expr
 

tests/build_tests/super_errors/expected/optional_default_arg_type_mismatch.res.expected

@@ -0,0 +1,13 @@
+
+  We've found a bug for you!
+  /.../fixtures/optional_default_arg_type_mismatch.res:2:19-22
+
+  1 │ let f = (~test: option<int>=42) => {
+  2 │   let _: string = test
+  3 │ }
+  4 │ 
+
+  This has type: int
+  But it's expected to have type: string
+
+  You can convert int to string with Int.toString.

tests/build_tests/super_errors/fixtures/optional_default_arg_type_mismatch.res

@@ -0,0 +1,3 @@
+let f = (~test: option<int>=42) => {
+  let _: string = test
+}

tests/tests/src/issue_8385_optional_args_typecheck.mjs

@@ -0,0 +1,15 @@
+// Generated by ReScript, PLEASE EDIT WITH CARE
+
+
+function Issue_8385_optional_args_typecheck$A(props) {
+  return null;
+}
+
+let A = {
+  make: Issue_8385_optional_args_typecheck$A
+};
+
+export {
+  A,
+}
+/* No side effect */

tests/tests/src/issue_8385_optional_args_typecheck.res

@@ -0,0 +1,14 @@
+@@config({flags: ["-bs-jsx", "4"]})
+
+module type Test = {
+  @react.component
+  let make: (~test: int=?) => React.element
+}
+
+module A: Test = {
+  @react.component
+  let make = (~test: option<int>=42) => {
+    let _: int = test
+    React.null
+  }
+}

tests/tests/src/optional_default_arg_annotation_test.mjs

@@ -0,0 +1,11 @@
+// Generated by ReScript, PLEASE EDIT WITH CARE
+
+
+function f(testOpt) {
+
+}
+
+export {
+  f,
+}
+/* No side effect */

tests/tests/src/optional_default_arg_annotation_test.res

@@ -0,0 +1,3 @@
+let f = (~test: option<int>=42) => {
+  let _: int = test
+}