Fix record field and jsx props punned when with attributes

.gitignore

@@ -12,3 +12,6 @@ _esybuild
 _esyinstall
 _release
 _export/
+
+# opam
+_opam/

src/reason-parser/reason_pprint_ast.ml

@@ -2113,8 +2113,10 @@ let createFormatter () =
         true
       | _ -> false
 
-    let isPunnedJsxArg lbl ident =
-      (not (isLongIdentWithDot ident.txt)) && Longident.last_exn ident.txt = lbl
+    let isPunnedJsxArg lbl ident attr =
+      (not (isLongIdentWithDot ident.txt))
+      && Longident.last_exn ident.txt = lbl
+      && attr = []
 
     let is_unit_pattern x =
       match x.ppat_desc with
@@ -4330,7 +4332,8 @@ let createFormatter () =
              PipeFirstTree.flastNode list) into a more convenient structure *
              that allows us to express the segments: "foo" "f(a, b)" "g(c, d)".
              * PipeFirstTree.t expresses those segments. *
-             [{exp = foo; args = []}; {exp = f; args = [a; b]}; {exp = g; args = [c; d]}]
+             [{exp = foo; args = []}; {exp = f; args = [a; b]}; {exp = g; args =
+              [c; d]}]
           *)
           let rec parse acc = function
             | PipeFirstTree.Exp e :: PipeFirstTree.Args args :: xs ->
@@ -4360,12 +4363,13 @@ let createFormatter () =
           *)
           let (flatNodes : PipeFirstTree.flatT) = flatten ~uncurried [] e in
           (* Turn * [Exp foo; Exp f; Args [a; b]; Exp g; Args [c; d]] * into *
-             [{exp = foo; args = []}; {exp = f; args = [a; b]}; {exp = g; args = [c; d]}]
+             [{exp = foo; args = []}; {exp = f; args = [a; b]}; {exp = g; args =
+              [c; d]}]
           *)
           let (pipetree : PipeFirstTree.t) = parse [] flatNodes in
           (* Turn *
-             [{exp = foo; args = []}; {exp = f; args = [a; b]}; {exp = g; args = [c; d]}]
-             * into * [foo; ->f(a, b); ->g(c, d)]
+             [{exp = foo; args = []}; {exp = f; args = [a; b]}; {exp = g; args =
+              [c; d]}] * into * [foo; ->f(a, b); ->g(c, d)]
           *)
           let pipeSegments =
             match pipetree with
@@ -5239,7 +5243,8 @@ let createFormatter () =
               let value_has_jsx = jsxAttrs != [] in
               let nextAttr =
                 match expression.pexp_desc with
-                | Pexp_ident ident when isPunnedJsxArg lbl ident ->
+                | Pexp_ident ident
+                  when isPunnedJsxArg lbl ident expression.pexp_attributes ->
                   makeList ~break:Layout.Never [ atom "?"; atom lbl ]
                 | Pexp_construct _ when value_has_jsx ->
                   label
@@ -5260,7 +5265,9 @@ let createFormatter () =
               let value_has_jsx = jsxAttrs != [] in
               let nextAttr =
                 match expression.pexp_desc with
-                | Pexp_ident ident when isPunnedJsxArg lbl ident -> atom lbl
+                | Pexp_ident ident
+                  when isPunnedJsxArg lbl ident expression.pexp_attributes ->
+                  atom lbl
                 | _ when isJSXComponent expression ->
                   label
                     (atom (lbl ^ "="))
@@ -5614,25 +5621,19 @@ let createFormatter () =
           let everythingButReturnVal =
             (* Because align_closing is set to false, you get: * * (Brackets[]
                inserted to show boundaries between open/close of pattern list) *
-               let[firstThing
-             *     secondThing
-             *     thirdThing]
-               * * It only wraps to indent four by coincidence: If the "opening"
-               token was * longer, you'd get: * *
-               letReallyLong[firstThing
-             *               secondThing
-             *               thirdThing]
-               * * For curried let bindings, we stick the arrow in the *last*
-               pattern: *
-               let[firstThing
-             *     secondThing
-             *     thirdThing =>]
-               * * But it could have just as easily been the "closing" token
-               corresponding to * "let". This works because we have
-               [align_closing = false]. The benefit of * shoving it in the last
-               pattern, is that we can turn [align_closing = true] * and still
-               have the arrow stuck to the last pattern (which is usually what
-               we * want) (See modeTwo below).
+               let[firstThing *     secondThing *     thirdThing] * * It only
+               wraps to indent four by coincidence: If the "opening" token was *
+               longer, you'd get: * *
+               letReallyLong[firstThing *               secondThing *           
+                             thirdThing] * * For curried let bindings, we stick
+               the arrow in the *last* pattern: *
+               let[firstThing *     secondThing *     thirdThing =>] * * But it
+               could have just as easily been the "closing" token corresponding
+               to * "let". This works because we have [align_closing = false].
+               The benefit of * shoving it in the last pattern, is that we can
+               turn [align_closing = true] * and still have the arrow stuck to
+               the last pattern (which is usually what we * want) (See modeTwo
+               below).
             *)
             match partitioning with
             | None when sweet ->
@@ -5824,11 +5825,10 @@ let createFormatter () =
            it becomes pretty printed as * let x:t =.... In the proposal, it is
            not impossible - it is only * impossible to preserve unnecessary
            parenthesis around the let binding. * * The one downside is that
-           integrating with existing code that uses
-           [let x =
-         * (blah:typ)] in standard OCaml will be parsed as a
-           Pexp_constraint. There * might be some lossiness (beyond parens) that
-           occurs in the original OCaml * parser.
+           integrating with existing code that uses [let x = * (blah:typ)] in
+           standard OCaml will be parsed as a Pexp_constraint. There * might be
+           some lossiness (beyond parens) that occurs in the original OCaml *
+           parser.
         *)
 
         method locallyAbstractPolymorphicFunctionBinding
@@ -7099,9 +7099,11 @@ let createFormatter () =
               match e.pexp_desc, shouldPun, allowPunning with
               (* record value punning. Turns {foo: foo, bar: 1} into {foo, bar: 1} *)
               (* also turns {Foo.bar: bar, baz: 1} into {Foo.bar, baz: 1} *)
+              (* don't turn {bar: [@foo] bar, baz: 1} into {bar, baz: 1} *)
               (* don't turn {bar: Foo.bar, baz: 1} into {bar, baz: 1}, naturally *)
               | Pexp_ident { txt = Lident value; _ }, true, true
-                when Longident.last_exn li.txt = value ->
+                when Longident.last_exn li.txt = value && e.pexp_attributes = []
+                ->
                 makeList (maybeQuoteFirstElem li [])
                 (* Force breaks for nested records or mel.obj sugar
                  * Example:

test/jsx.t/input.re

@@ -103,6 +103,9 @@ let icon = <Icon
 /* punning for explicitly passed optional */
 <Foo bar=?bar />;
 
+/* Don't pun for explicitly props with attributes */
+<Foo bar=?{[@browser_only]bar} />;
+
 /* don't pun explicitly passed optional with module identifier */
 <Foo bar=?Baz.bar />;
 
@@ -131,6 +134,8 @@ let y = [<Button onClick=handleStaleClick />, <Button onClick=handleStaleClick /
 <Description term=([@JSX] Text.createElement(~text="Age", ()))> child </Description>;
 
 <Description term={<Text superLongPunnedProp anotherSuperLongOneCrazyLongThingHere text="Age" />}> child </Description>;
+<Description term={<Text noPunnedProp={[@attribute] noPunnedProp} superLongPunnedProp anotherSuperLongOneCrazyLongThingHere text="Age" />}> child </Description>;
+<Description term={<Text noPunned={[@attribute] noPunnedProp} />}> child </Description>;
 
 <Foo bar={<Baz superLongPunnedProp anotherSuperLongOneCrazyLongThingHere/>}/>;
 

test/jsx.t/run.t

@@ -140,6 +140,9 @@ Format JSX
   /* punning for explicitly passed optional */
   <Foo ?bar />;
 
+  /* Don't pun for explicitly props with attributes */
+  <Foo bar=?{[@browser_only] bar} />;
+
   /* don't pun explicitly passed optional with module identifier */
   <Foo bar=?Baz.bar />;
 
@@ -200,6 +203,23 @@ Format JSX
     }>
     child
   </Description>;
+  <Description
+    term={
+      <Text
+        noPunnedProp={[@attribute] noPunnedProp}
+        superLongPunnedProp
+        anotherSuperLongOneCrazyLongThingHere
+        text="Age"
+      />
+    }>
+    child
+  </Description>;
+  <Description
+    term={
+      <Text noPunned={[@attribute] noPunnedProp} />
+    }>
+    child
+  </Description>;
 
   <Foo
     bar={

test/sequences.t/input.re

@@ -79,3 +79,20 @@ let thirdFieldPunned = {
   c
 };
 let singlePunAcceptedIfExtended = {...firstFieldPunned, a};
+
+/* non-punned */
+let firstFieldNonPun = {
+  a: [@with_attribute] a,
+  b,
+  c
+};
+let secondFieldNonPun = {
+  a,
+  b: [@with_attribute] b,
+  c
+};
+let thirdFieldNonPun = {
+  a,
+  b,
+  c: [@with_attribute] c,
+};

test/sequences.t/run.t

@@ -102,7 +102,23 @@ Print the formatted file
     ...firstFieldPunned,
     a,
   };
-
+
+  /* non-punned */
+  let firstFieldNonPun = {
+    a: [@with_attribute] a,
+    b,
+    c,
+  };
+  let secondFieldNonPun = {
+    a,
+    b: [@with_attribute] b,
+    c,
+  };
+  let thirdFieldNonPun = {
+    a,
+    b,
+    c: [@with_attribute] c,
+  };
 Type-check basics
   $ ocamlc -c -pp 'refmt --print binary' -intf-suffix .rei -impl formatted.re
 

test/typeDeclarations.t/input.re

@@ -57,10 +57,18 @@ type foo = option([@foo]string, [@bar](int => string => int));
 /* https://github.com/facebook/reason/issues/2073 */
 type a = array({. "someStringKeyThatCausesLineToBreak": string });
 
+/* Inline type record non punned field */
 type b = {
   punned: [@with_attribute] punned
 };
 
+/* Breakline record non punned field */
+type c = {
+  a: string,
+  b: string,
+  punned: [@with_attribute] punned
+};
+
 type%x foo = int;
 
 type%x foo += Int;

test/typeDeclarations.t/run.t

@@ -127,8 +127,16 @@ Format type declarations
       "someStringKeyThatCausesLineToBreak": string,
     });
 
+  /* Inline type record non punned field */
   type b = {punned: [@with_attribute] punned};
 
+  /* Breakline record non punned field */
+  type c = {
+    a: string,
+    b: string,
+    punned: [@with_attribute] punned,
+  };
+
   type%x foo = int;
 
   type%x foo +=