From b8428cfa49d2d10d57b32726d326d96ed0e22f1b Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Tue, 13 Aug 2024 16:01:00 +0800 Subject: [PATCH 1/5] Check for dotdotdot in args --- jscomp/syntax/src/res_printer.ml | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/jscomp/syntax/src/res_printer.ml b/jscomp/syntax/src/res_printer.ml index fbb397f83e..957e6d7f54 100644 --- a/jscomp/syntax/src/res_printer.ml +++ b/jscomp/syntax/src/res_printer.ml @@ -4730,6 +4730,20 @@ 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 hasDotDotDot, printed_args = + List.fold_right + (fun arg (flag, acc) -> + let arg_lbl, _ = arg in + let hasDotDotDot = + match arg_lbl with + | Asttypes.Labelled "..." -> true + | _ -> false + in + let doc = print_argument ~state arg cmt_tbl in + (flag || hasDotDotDot, doc :: acc)) + args (false, []) + in Doc.group (Doc.concat [ @@ -4738,13 +4752,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 || hasDotDotDot then Doc.nil else Doc.trailing_comma); Doc.soft_line; Doc.rparen; ]) From 85019dc0ee480401599c5c980a49cf0cf89023a0 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Tue, 13 Aug 2024 21:58:43 +0800 Subject: [PATCH 2/5] Check on attribute rather than label --- jscomp/syntax/src/res_parsetree_viewer.ml | 7 +++++++ jscomp/syntax/src/res_parsetree_viewer.mli | 2 ++ jscomp/syntax/src/res_printer.ml | 14 +++++++++----- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/jscomp/syntax/src/res_parsetree_viewer.ml b/jscomp/syntax/src/res_parsetree_viewer.ml index 4b26dc55ec..75d0228442 100644 --- a/jscomp/syntax/src/res_parsetree_viewer.ml +++ b/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 = diff --git a/jscomp/syntax/src/res_parsetree_viewer.mli b/jscomp/syntax/src/res_parsetree_viewer.mli index 6661dfc923..0cd2053694 100644 --- a/jscomp/syntax/src/res_parsetree_viewer.mli +++ b/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 *) diff --git a/jscomp/syntax/src/res_printer.ml b/jscomp/syntax/src/res_printer.ml index 957e6d7f54..4cbdf58fac 100644 --- a/jscomp/syntax/src/res_printer.ml +++ b/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 @@ -4734,11 +4740,9 @@ and print_arguments ~state ?(partial = false) let hasDotDotDot, printed_args = List.fold_right (fun arg (flag, acc) -> - let arg_lbl, _ = arg in + let _, expr = arg in let hasDotDotDot = - match arg_lbl with - | Asttypes.Labelled "..." -> true - | _ -> false + ParsetreeViewer.has_partial_attribute expr.Parsetree.pexp_attributes in let doc = print_argument ~state arg cmt_tbl in (flag || hasDotDotDot, doc :: acc)) From d26906898fcd872a66f4d52222a90e481da889f8 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Tue, 13 Aug 2024 22:06:19 +0800 Subject: [PATCH 3/5] Add tests --- jscomp/syntax/tests/printer/expr/apply.res | 34 +++++++++++++ .../tests/printer/expr/expected/apply.res.txt | 51 +++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/jscomp/syntax/tests/printer/expr/apply.res b/jscomp/syntax/tests/printer/expr/apply.res index 9d44d720d3..3c9d1aed48 100644 --- a/jscomp/syntax/tests/printer/expr/apply.res +++ b/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, ...) +) diff --git a/jscomp/syntax/tests/printer/expr/expected/apply.res.txt b/jscomp/syntax/tests/printer/expr/expected/apply.res.txt index c04b627ad2..f37f83f248 100644 --- a/jscomp/syntax/tests/printer/expr/expected/apply.res.txt +++ b/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, + ... + ), +) From 9b8695ac89350b2824301c199c6accae350d35c1 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Tue, 13 Aug 2024 22:13:02 +0800 Subject: [PATCH 4/5] Fix naming convention --- jscomp/syntax/src/res_printer.ml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/jscomp/syntax/src/res_printer.ml b/jscomp/syntax/src/res_printer.ml index 4cbdf58fac..ae8055d7b5 100644 --- a/jscomp/syntax/src/res_printer.ml +++ b/jscomp/syntax/src/res_printer.ml @@ -4737,15 +4737,15 @@ and print_arguments ~state ?(partial = false) Doc.concat [Doc.lparen; arg_doc; Doc.rparen] | args -> (* Avoid printing trailing comma when there is ... in function application *) - let hasDotDotDot, printed_args = + let has_partial_attr, printed_args = List.fold_right (fun arg (flag, acc) -> let _, expr = arg in - let hasDotDotDot = + let has_partial_attr = ParsetreeViewer.has_partial_attribute expr.Parsetree.pexp_attributes in let doc = print_argument ~state arg cmt_tbl in - (flag || hasDotDotDot, doc :: acc)) + (flag || has_partial_attr, doc :: acc)) args (false, []) in Doc.group @@ -4758,7 +4758,7 @@ and print_arguments ~state ?(partial = false) Doc.soft_line; Doc.join ~sep:(Doc.concat [Doc.comma; Doc.line]) printed_args; ]); - (if partial || hasDotDotDot then Doc.nil else Doc.trailing_comma); + (if partial || has_partial_attr then Doc.nil else Doc.trailing_comma); Doc.soft_line; Doc.rparen; ]) From a05b98c49ff98be849136c39301cfa4b1cfae36f Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Thu, 15 Aug 2024 06:54:12 +0800 Subject: [PATCH 5/5] Update CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ee29f0853..9c4e84f4d6 100644 --- a/CHANGELOG.md +++ b/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