From ac648b7dd08b5141fdba20bf970fc9df68cb0211 Mon Sep 17 00:00:00 2001
From: Pedro B S Lisboa <pedrobslisboa@gmail.com>
Date: Thu, 30 Jan 2025 05:57:56 -0300
Subject: [PATCH] fix: don't punne record fields and jsx prop with attr

Signed-off-by: Pedro B S Lisboa <pedrobslisboa@gmail.com>
---
 src/reason-parser/reason_pprint_ast.ml | 32 +++++++++++++++++---------
 test/jsx.t/input.re                    |  5 ++++
 test/jsx.t/run.t                       | 20 ++++++++++++++++
 test/sequences.t/input.re              | 17 ++++++++++++++
 test/sequences.t/run.t                 | 18 ++++++++++++++-
 test/typeDeclarations.t/input.re       |  8 +++++++
 test/typeDeclarations.t/run.t          |  8 +++++++
 7 files changed, 96 insertions(+), 12 deletions(-)

diff --git a/src/reason-parser/reason_pprint_ast.ml b/src/reason-parser/reason_pprint_ast.ml
index f5307671b..1c7d2ba13 100644
--- a/src/reason-parser/reason_pprint_ast.ml
+++ b/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
@@ -5233,13 +5237,14 @@ let createFormatter () =
                 processedAttrs
                 (Some [ self#dotdotdotChild expr ])
             | (Optional lbl, expression) :: tail ->
-              let { Reason_attributes.jsxAttrs; _ } =
+              let { Reason_attributes.jsxAttrs; stdAttrs; _ } =
                 Reason_attributes.partitionAttributes expression.pexp_attributes
               in
               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 stdAttrs ->
                   makeList ~break:Layout.Never [ atom "?"; atom lbl ]
                 | Pexp_construct _ when value_has_jsx ->
                   label
@@ -5254,13 +5259,15 @@ let createFormatter () =
               in
               processArguments tail (nextAttr :: processedAttrs) children
             | (Labelled lbl, expression) :: tail ->
-              let { Reason_attributes.jsxAttrs; _ } =
+              let { Reason_attributes.jsxAttrs; stdAttrs; _ } =
                 Reason_attributes.partitionAttributes expression.pexp_attributes
               in
               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 stdAttrs ->
+                  atom lbl
                 | _ when isJSXComponent expression ->
                   label
                     (atom (lbl ^ "="))
@@ -7095,13 +7102,16 @@ let createFormatter () =
               ; loc_ghost = false
               }
             in
+            let stdAttrs = Reason_attributes.extractStdAttrs e.pexp_attributes in 
             let theRow =
               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 && stdAttrs = []
+                ->
                 makeList (maybeQuoteFirstElem li [])
                 (* Force breaks for nested records or mel.obj sugar
                  * Example:
diff --git a/test/jsx.t/input.re b/test/jsx.t/input.re
index 3d9de4ce5..b77f48fe7 100644
--- a/test/jsx.t/input.re
+++ b/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/>}/>;
 
diff --git a/test/jsx.t/run.t b/test/jsx.t/run.t
index 01f2990e9..c848661c7 100644
--- a/test/jsx.t/run.t
+++ b/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={
diff --git a/test/sequences.t/input.re b/test/sequences.t/input.re
index 151ec035f..0a39fd149 100644
--- a/test/sequences.t/input.re
+++ b/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,
+};
\ No newline at end of file
diff --git a/test/sequences.t/run.t b/test/sequences.t/run.t
index a8e5cf34c..e98b2f30e 100644
--- a/test/sequences.t/run.t
+++ b/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
 
diff --git a/test/typeDeclarations.t/input.re b/test/typeDeclarations.t/input.re
index 4a8774c77..8013ca891 100644
--- a/test/typeDeclarations.t/input.re
+++ b/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;
diff --git a/test/typeDeclarations.t/run.t b/test/typeDeclarations.t/run.t
index d0799e40c..d3bc6b5a9 100644
--- a/test/typeDeclarations.t/run.t
+++ b/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 +=