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 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 9f06915d2e..a638744720 100644 --- a/compiler/ml/typecore.ml +++ b/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 = 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/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..8f711eb758 --- /dev/null +++ b/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=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. \ 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 new file mode 100644 index 0000000000..6b6ae5ad3d --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/optional_default_arg_type_mismatch.res @@ -0,0 +1,3 @@ +let f = (~test: option=42) => { + let _: string = test +} 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 + } +} 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 +}