From 02b4414a77e19069609346232d11f4ca977852f1 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Thu, 23 Apr 2026 15:00:29 +0200 Subject: [PATCH 1/4] Improve error for generated optional default match Signed-off-by: Christoph Knittel --- compiler/ml/typecore.ml | 15 +++++++++------ ...ptional_default_arg_type_mismatch.res.expected | 10 ++++++++++ .../optional_default_arg_type_mismatch.res | 4 ++++ 3 files changed, 23 insertions(+), 6 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/optional_default_arg_type_mismatch.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/optional_default_arg_type_mismatch.res diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index 9f06915d2e..765749d30f 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -2452,6 +2452,7 @@ 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 @@ -2540,13 +2541,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 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 + (* 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. *) + else if has_attr "#optional_arg_default" then `Function else `Switch in let val_cases, partial = diff --git a/tests/build_tests/super_errors/expected/optional_default_arg_type_mismatch.res.expected b/tests/build_tests/super_errors/expected/optional_default_arg_type_mismatch.res.expected new file mode 100644 index 0000000000..fabc1bf22f --- /dev/null +++ b/tests/build_tests/super_errors/expected/optional_default_arg_type_mismatch.res.expected @@ -0,0 +1,10 @@ + + We've found a bug for you! + /.../fixtures/optional_default_arg_type_mismatch.res:1:29-30 + + 1 │ let f = (~test: option=42) => { + 2 │ let _: int = test + 3 │ () + + This has type: int + But it's expected to have type: option diff --git a/tests/build_tests/super_errors/fixtures/optional_default_arg_type_mismatch.res b/tests/build_tests/super_errors/fixtures/optional_default_arg_type_mismatch.res new file mode 100644 index 0000000000..37c876c7bc --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/optional_default_arg_type_mismatch.res @@ -0,0 +1,4 @@ +let f = (~test: option=42) => { + let _: int = test + () +} From 448192c5c81472c7f9d8d75da17f735bae518164 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Thu, 23 Apr 2026 15:16:27 +0200 Subject: [PATCH 2/4] Fix optional arg default annotation typing --- compiler/ml/typecore.ml | 64 +++++++++++++++++-- ...nal_default_arg_type_mismatch.res.expected | 13 ++-- .../optional_default_arg_type_mismatch.res | 3 +- .../optional_default_arg_annotation_test.mjs | 11 ++++ .../optional_default_arg_annotation_test.res | 3 + 5 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 tests/tests/src/optional_default_arg_annotation_test.mjs create mode 100644 tests/tests/src/optional_default_arg_annotation_test.res diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index 765749d30f..dd58ed8e4e 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -2313,6 +2313,61 @@ and type_expect ~context ?deprecated_context ?in_function ?recarg env sexp (Cmt_format.Partial_expression exp :: previous_saved_types); exp +(* Optional arguments with defaults are lowered in two steps: + + let f = (~test: option=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; it is plain int. This helper + rewrites the original pattern so that: + + (~test: option=42) + + becomes a resolved-value pattern equivalent to: + + (~test: int=42) + + for the synthesized rebinding step only. *) +and 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. *) + { + 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 + and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected) env sexp ty_expected = let loc = sexp.pexp_loc in @@ -2457,10 +2512,11 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected) scases in let pat = Pat.var ~loc:sloc (mknoloc "*opt*") in + let resolved_spat = 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 @@ -2544,11 +2600,11 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected) 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 has_attr "let.unwrap" then `LetUnwrap - (* 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. *) else if has_attr "#optional_arg_default" then `Function else `Switch in diff --git a/tests/build_tests/super_errors/expected/optional_default_arg_type_mismatch.res.expected b/tests/build_tests/super_errors/expected/optional_default_arg_type_mismatch.res.expected index fabc1bf22f..8f711eb758 100644 --- a/tests/build_tests/super_errors/expected/optional_default_arg_type_mismatch.res.expected +++ b/tests/build_tests/super_errors/expected/optional_default_arg_type_mismatch.res.expected @@ -1,10 +1,13 @@ We've found a bug for you! - /.../fixtures/optional_default_arg_type_mismatch.res:1:29-30 + /.../fixtures/optional_default_arg_type_mismatch.res:2:19-22 - 1 │ let f = (~test: option=42) => { - 2 │ let _: int = test - 3 │ () + 1 │ let f = (~test: option=42) => { + 2 │ let _: string = test + 3 │ } + 4 │ This has type: int - But it's expected to have type: option + But it's expected to have type: string + + You can convert int to string with Int.toString. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/optional_default_arg_type_mismatch.res b/tests/build_tests/super_errors/fixtures/optional_default_arg_type_mismatch.res index 37c876c7bc..6b6ae5ad3d 100644 --- a/tests/build_tests/super_errors/fixtures/optional_default_arg_type_mismatch.res +++ b/tests/build_tests/super_errors/fixtures/optional_default_arg_type_mismatch.res @@ -1,4 +1,3 @@ let f = (~test: option=42) => { - let _: int = test - () + let _: string = test } diff --git a/tests/tests/src/optional_default_arg_annotation_test.mjs b/tests/tests/src/optional_default_arg_annotation_test.mjs new file mode 100644 index 0000000000..f4d6eebf1d --- /dev/null +++ b/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 */ diff --git a/tests/tests/src/optional_default_arg_annotation_test.res b/tests/tests/src/optional_default_arg_annotation_test.res new file mode 100644 index 0000000000..51ff6f5d3b --- /dev/null +++ b/tests/tests/src/optional_default_arg_annotation_test.res @@ -0,0 +1,3 @@ +let f = (~test: option=42) => { + let _: int = test +} From 39757fdb2ce3d672fc3a5b4ad10c2076b467fd1f Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Thu, 23 Apr 2026 16:26:58 +0200 Subject: [PATCH 3/4] Fix JSX optional arg default typing --- compiler/ml/parsetree_utils.ml | 58 ++++++++++++++++++ compiler/ml/parsetree_utils.mli | 1 + compiler/ml/typecore.ml | 59 +------------------ compiler/syntax/src/jsx_v4.ml | 8 ++- .../issue_8385_optional_args_typecheck.mjs | 15 +++++ .../issue_8385_optional_args_typecheck.res | 14 +++++ 6 files changed, 98 insertions(+), 57 deletions(-) create mode 100644 compiler/ml/parsetree_utils.ml create mode 100644 compiler/ml/parsetree_utils.mli create mode 100644 tests/tests/src/issue_8385_optional_args_typecheck.mjs create mode 100644 tests/tests/src/issue_8385_optional_args_typecheck.res diff --git a/compiler/ml/parsetree_utils.ml b/compiler/ml/parsetree_utils.ml new file mode 100644 index 0000000000..1e7cdda484 --- /dev/null +++ b/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=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; it is plain int. This helper + rewrites the original pattern so that: + + (~test: option=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. *) + { + 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 diff --git a/compiler/ml/parsetree_utils.mli b/compiler/ml/parsetree_utils.mli new file mode 100644 index 0000000000..1a6c2dbdb2 --- /dev/null +++ b/compiler/ml/parsetree_utils.mli @@ -0,0 +1 @@ +val adapt_pattern_for_resolved_default : Parsetree.pattern -> Parsetree.pattern diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index dd58ed8e4e..a638744720 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -2313,61 +2313,6 @@ and type_expect ~context ?deprecated_context ?in_function ?recarg env sexp (Cmt_format.Partial_expression exp :: previous_saved_types); exp -(* Optional arguments with defaults are lowered in two steps: - - let f = (~test: option=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; it is plain int. This helper - rewrites the original pattern so that: - - (~test: option=42) - - becomes a resolved-value pattern equivalent to: - - (~test: int=42) - - for the synthesized rebinding step only. *) -and 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. *) - { - 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 - and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected) env sexp ty_expected = let loc = sexp.pexp_loc in @@ -2512,7 +2457,9 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected) scases in let pat = Pat.var ~loc:sloc (mknoloc "*opt*") in - let resolved_spat = adapt_pattern_for_resolved_default spat in + let resolved_spat = + Parsetree_utils.adapt_pattern_for_resolved_default spat + in let body = Exp.let_ ~loc Nonrecursive ~attrs:[(mknoloc "#default", PStr [])] diff --git a/compiler/syntax/src/jsx_v4.ml b/compiler/syntax/src/jsx_v4.ml index 83bbb0dcdc..d89eafc24b 100644 --- a/compiler/syntax/src/jsx_v4.ml +++ b/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 diff --git a/tests/tests/src/issue_8385_optional_args_typecheck.mjs b/tests/tests/src/issue_8385_optional_args_typecheck.mjs new file mode 100644 index 0000000000..0012dbc22c --- /dev/null +++ b/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 */ diff --git a/tests/tests/src/issue_8385_optional_args_typecheck.res b/tests/tests/src/issue_8385_optional_args_typecheck.res new file mode 100644 index 0000000000..21ec684265 --- /dev/null +++ b/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=42) => { + let _: int = test + React.null + } +} From 979541f884e2785b929cadbc53ee81baa8d332bb Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Thu, 23 Apr 2026 15:44:27 +0200 Subject: [PATCH 4/4] CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccb15f427a..16b4d8dd0c 100644 --- a/CHANGELOG.md +++ b/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