Fix issue overlapping argument with default value and alias with default value (#734)

* Fix issue overlapping argument with default value

* add loc to pattern

* fix alias props with default value

* update changelog

* fix build error with Pexp_field

* fix tests

* fix handling default value of props

* fix tests

Author: mununki

CHANGELOG.md

@@ -60,6 +60,8 @@
 - Fix issue where uncurried functions were incorrectly converting the type of a prop given as a default value to curried https://github.com/rescript-lang/syntax/pull/731
 - Fix issue with printing async functions with locally abstract types https://github.com/rescript-lang/syntax/pull/732
 - Fix support for recursive components in JSX V4 https://github.com/rescript-lang/syntax/pull/733
+- Fix issue with overlapping labelled argument with default value https://github.com/rescript-lang/syntax/pull/734
+- Fix issue with using alias and default value together https://github.com/rescript-lang/syntax/pull/734
 
 #### :eyeglasses: Spec Compliance
 

cli/reactjs_jsx_v4.ml

@@ -729,10 +729,10 @@ let argToType ~newtypes ~(typeConstraints : core_type option) types
     :: types
   | _ -> types
 
-let argWithDefaultValue (name, default, _, _, _, _) =
-  match default with
-  | Some default when isOptional name -> Some (getLabel name, default)
-  | _ -> None
+let hasDefaultValue nameArgList =
+  nameArgList
+  |> List.exists (fun (name, default, _, _, _, _) ->
+         Option.is_some default && isOptional name)
 
 let argToConcreteType types (name, attrs, loc, type_) =
   match name with
@@ -1016,26 +1016,43 @@ let transformStructureItem ~config mapper item =
               (argToType ~newtypes ~typeConstraints)
               [] namedArgList
           in
-          let namedArgWithDefaultValueList =
-            List.filter_map argWithDefaultValue namedArgList
+          let vbMatch (name, default, _, alias, loc, _) =
+            let label = getLabel name in
+            match default with
+            | Some default ->
+              Vb.mk
+                (Pat.var (Location.mkloc alias loc))
+                (Exp.match_
+                   (Exp.field
+                      (Exp.ident {txt = Lident "props"; loc = Location.none})
+                      (Location.mknoloc @@ Lident label))
+                   [
+                     Exp.case
+                       (Pat.construct
+                          (Location.mknoloc @@ Lident "Some")
+                          (Some (Pat.var (Location.mknoloc label))))
+                       (Exp.ident (Location.mknoloc @@ Lident label));
+                     Exp.case
+                       (Pat.construct (Location.mknoloc @@ Lident "None") None)
+                       default;
+                   ])
+            | None ->
+              Vb.mk
+                (Pat.var (Location.mkloc alias loc))
+                (Exp.field
+                   (Exp.ident {txt = Lident "props"; loc = Location.none})
+                   (Location.mknoloc @@ Lident label))
           in
-          let vbMatch (label, default) =
-            Vb.mk
-              (Pat.var (Location.mknoloc label))
-              (Exp.match_
-                 (Exp.ident {txt = Lident label; loc = Location.none})
-                 [
-                   Exp.case
-                     (Pat.construct
-                        (Location.mknoloc @@ Lident "Some")
-                        (Some (Pat.var (Location.mknoloc label))))
-                     (Exp.ident (Location.mknoloc @@ Lident label));
-                   Exp.case
-                     (Pat.construct (Location.mknoloc @@ Lident "None") None)
-                     default;
-                 ])
+          let vbMatchExpr namedArgList expr =
+            let rec aux namedArgList =
+              match namedArgList with
+              | [] -> expr
+              | [namedArg] -> Exp.let_ Nonrecursive [vbMatch namedArg] expr
+              | namedArg :: rest ->
+                Exp.let_ Nonrecursive [vbMatch namedArg] (aux rest)
+            in
+            aux (List.rev namedArgList)
           in
-          let vbMatchList = List.map vbMatch namedArgWithDefaultValueList in
           (* type props = { ... } *)
           let propsRecordType =
             makePropsRecordType ~coreTypeOfAttr ~typVarsOfCoreType "props"
@@ -1154,20 +1171,27 @@ let transformStructureItem ~config mapper item =
           in
           (* add pattern matching for optional prop value *)
           let expression =
-            if List.length vbMatchList = 0 then expression
-            else Exp.let_ Nonrecursive vbMatchList expression
+            if hasDefaultValue namedArgList then
+              vbMatchExpr namedArgList expression
+            else expression
           in
           (* (ref) => expr *)
           let expression =
             List.fold_left
               (fun expr (_, pattern) -> Exp.fun_ Nolabel None pattern expr)
               expression patternsWithNolabel
           in
+          (* ({a, b, _}: props<'a, 'b>) *)
           let recordPattern =
             match patternsWithLabel with
             | [] -> Pat.any ()
             | _ -> Pat.record (List.rev patternsWithLabel) Open
           in
+          let recordPattern =
+            if hasDefaultValue namedArgList then
+              Pat.var {txt = "props"; loc = emptyLoc}
+            else recordPattern
+          in
           let expression =
             Exp.fun_ Nolabel None
               (Pat.constraint_ recordPattern

tests/ppx/react/aliasProps.res

@@ -9,3 +9,8 @@ module C1 = {
   @react.component
   let make = (~priority as p, ~text="Test") => React.string(p ++ text)
 }
+
+module C2 = {
+  @react.component
+  let make = (~foo as bar="") => React.string(bar)
+}

tests/ppx/react/defaultValueProp.res

@@ -0,0 +1,9 @@
+module C0 = {
+  @react.component
+  let make = (~a=2, ~b=a*2) => React.int(a + b)
+}
+
+module C1 = {
+  @react.component
+  let make = (~a=2, ~b) => React.int(a + b)
+}

tests/ppx/react/expected/aliasProps.res.txt

@@ -4,8 +4,9 @@ module C0 = {
   type props<'priority, 'text> = {priority: 'priority, text?: 'text}
 
   @react.component
-  let make = ({priority: _, ?text, _}: props<'priority, 'text>) => {
-    let text = switch text {
+  let make = (props: props<'priority, 'text>) => {
+    let _ = props.priority
+    let text = switch props.text {
     | Some(text) => text
     | None => "Test"
     }
@@ -23,8 +24,9 @@ module C1 = {
   type props<'priority, 'text> = {priority: 'priority, text?: 'text}
 
   @react.component
-  let make = ({priority: p, ?text, _}: props<'priority, 'text>) => {
-    let text = switch text {
+  let make = (props: props<'priority, 'text>) => {
+    let p = props.priority
+    let text = switch props.text {
     | Some(text) => text
     | None => "Test"
     }
@@ -37,3 +39,22 @@ module C1 = {
     \"AliasProps$C1"
   }
 }
+
+module C2 = {
+  type props<'foo> = {foo?: 'foo}
+
+  @react.component
+  let make = (props: props<'foo>) => {
+    let bar = switch props.foo {
+    | Some(foo) => foo
+    | None => ""
+    }
+
+    React.string(bar)
+  }
+  let make = {
+    let \"AliasProps$C2" = (props: props<_>) => make(props)
+
+    \"AliasProps$C2"
+  }
+}

tests/ppx/react/expected/defaultValueProp.res.txt

@@ -0,0 +1,40 @@
+module C0 = {
+  type props<'a, 'b> = {a?: 'a, b?: 'b}
+  @react.component
+  let make = (props: props<'a, 'b>) => {
+    let a = switch props.a {
+    | Some(a) => a
+    | None => 2
+    }
+    let b = switch props.b {
+    | Some(b) => b
+    | None => a * 2
+    }
+
+    React.int(a + b)
+  }
+  let make = {
+    let \"DefaultValueProp$C0" = (props: props<_>) => make(props)
+    \"DefaultValueProp$C0"
+  }
+}
+
+module C1 = {
+  type props<'a, 'b> = {a?: 'a, b: 'b}
+
+  @react.component
+  let make = (props: props<'a, 'b>) => {
+    let a = switch props.a {
+    | Some(a) => a
+    | None => 2
+    }
+    let b = props.b
+
+    React.int(a + b)
+  }
+  let make = {
+    let \"DefaultValueProp$C1" = (props: props<_>) => make(props)
+
+    \"DefaultValueProp$C1"
+  }
+}

tests/ppx/react/expected/uncurriedProps.res.txt

@@ -2,8 +2,8 @@
 type props<'a> = {a?: 'a}
 
 @react.component
-let make = ({?a, _}: props<(. unit) => unit>) => {
-  let a = switch a {
+let make = (props: props<(. unit) => unit>) => {
+  let a = switch props.a {
   | Some(a) => a
   | None => (. ()) => ()
   }
@@ -30,8 +30,8 @@ module Foo = {
   type props<'callback> = {callback?: 'callback}
 
   @react.component
-  let make = ({?callback, _}: props<(. string, bool, bool) => unit>) => {
-    let callback = switch callback {
+  let make = (props: props<(. string, bool, bool) => unit>) => {
+    let callback = switch props.callback {
     | Some(callback) => callback
     | None => (. _, _, _) => ()
     }