Skip trailing comma in explicit partial application (#6949)

* Check for dotdotdot in args

* Check on attribute rather than label

* Add tests

* Fix naming convention

* Update CHANGELOG

Author: shulhi

CHANGELOG.md

@@ -23,6 +23,9 @@
   - Removed empty line at the end of `switch` statement
   - Removed empty `default` case from `switch` statement in the generated code
 
+#### :bug: Bug Fix
+- Fix issue where long layout break added a trailing comma in partial application `...`. https://github.com/rescript-lang/rescript-compiler/pull/6949
+
 # 12.0.0-alpha.1
 
 #### :rocket: New Feature

jscomp/syntax/src/res_parsetree_viewer.ml

@@ -57,6 +57,13 @@ let process_partial_app_attribute attrs =
   in
   process false [] attrs
 
+let has_partial_attribute attrs =
+  List.exists
+    (function
+      | {Location.txt = "res.partial"}, _ -> true
+      | _ -> false)
+    attrs
+
 type function_attributes_info = {async: bool; attributes: Parsetree.attributes}
 
 let process_function_attributes attrs =

jscomp/syntax/src/res_parsetree_viewer.mli

@@ -17,6 +17,8 @@ val functor_type :
 val process_partial_app_attribute :
   Parsetree.attributes -> bool * Parsetree.attributes
 
+val has_partial_attribute : Parsetree.attributes -> bool
+
 type function_attributes_info = {async: bool; attributes: Parsetree.attributes}
 
 (* determines whether a function is async and/or uncurried based on the given attributes *)

jscomp/syntax/src/res_printer.ml

@@ -4146,7 +4146,13 @@ and print_pexp_apply ~state expr cmt_tbl =
     let partial, attrs = ParsetreeViewer.process_partial_app_attribute attrs in
     let args =
       if partial then
-        let dummy = Ast_helper.Exp.constant (Ast_helper.Const.int 0) in
+        let loc =
+          {Asttypes.txt = "res.partial"; Asttypes.loc = expr.pexp_loc}
+        in
+        let attr = (loc, Parsetree.PTyp (Ast_helper.Typ.any ())) in
+        let dummy =
+          Ast_helper.Exp.constant ~attrs:[attr] (Ast_helper.Const.int 0)
+        in
         args @ [(Asttypes.Labelled "...", dummy)]
       else args
     in
@@ -4730,6 +4736,18 @@ and print_arguments ~state ?(partial = false)
     in
     Doc.concat [Doc.lparen; arg_doc; Doc.rparen]
   | args ->
+    (* Avoid printing trailing comma when there is ... in function application *)
+    let has_partial_attr, printed_args =
+      List.fold_right
+        (fun arg (flag, acc) ->
+          let _, expr = arg in
+          let has_partial_attr =
+            ParsetreeViewer.has_partial_attribute expr.Parsetree.pexp_attributes
+          in
+          let doc = print_argument ~state arg cmt_tbl in
+          (flag || has_partial_attr, doc :: acc))
+        args (false, [])
+    in
     Doc.group
       (Doc.concat
          [
@@ -4738,13 +4756,9 @@ and print_arguments ~state ?(partial = false)
              (Doc.concat
                 [
                   Doc.soft_line;
-                  Doc.join
-                    ~sep:(Doc.concat [Doc.comma; Doc.line])
-                    (List.map
-                       (fun arg -> print_argument ~state arg cmt_tbl)
-                       args);
+                  Doc.join ~sep:(Doc.concat [Doc.comma; Doc.line]) printed_args;
                 ]);
-           (if partial then Doc.nil else Doc.trailing_comma);
+           (if partial || has_partial_attr then Doc.nil else Doc.trailing_comma);
            Doc.soft_line;
            Doc.rparen;
          ])

jscomp/syntax/tests/printer/expr/apply.res

@@ -73,3 +73,37 @@ f(. {
 
 resolve(.)
 resolve(. ())
+
+let g =  f(
+  a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
+  b,
+  c
+)
+
+let g =  f(
+  a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
+  b,
+  c, 
+  ...
+)
+
+let g =  f(
+  a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
+  b,
+  c => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(d, ...),
+  ...
+)
+
+let g =  f(
+  a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a, ...),
+  b,
+  c => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(d),
+  ...
+)
+
+
+let g =  f(
+  a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
+  b,
+  c => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(d, ...)
+)

jscomp/syntax/tests/printer/expr/expected/apply.res.txt

@@ -93,3 +93,54 @@ f({
 
 resolve()
 resolve()
+
+let g = f(
+  a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
+  b,
+  c,
+)
+
+let g =
+  f(
+    a =>
+      LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
+    b,
+    c,
+    ...
+  )
+
+let g =
+  f(
+    a =>
+      LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
+    b,
+    c =>
+      LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(
+        d,
+        ...
+      ),
+    ...
+  )
+
+let g =
+  f(
+    a =>
+      LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(
+        a,
+        ...
+      ),
+    b,
+    c =>
+      LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(d),
+    ...
+  )
+
+let g = f(
+  a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
+  b,
+  c =>
+    LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(
+      d,
+      ...
+    ),
+)