Skip to content

Commit eb00295

Browse files
shulhicknitt
authored andcommitted
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
1 parent 77c2b64 commit eb00295

File tree

6 files changed

+117
-5
lines changed

6 files changed

+117
-5
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
1313
# 11.1.4
1414

15+
- Fix issue where long layout break added a trailing comma in partial application `...`. https://github.com/rescript-lang/rescript-compiler/pull/6949
16+
1517
# 11.1.3
1618

1719
#### :bug: Bug Fix

jscomp/syntax/src/res_parsetree_viewer.ml

+7
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,13 @@ let processUncurriedAppAttribute attrs =
7272
in
7373
process false [] attrs
7474

75+
let hasPartialAttribute attrs =
76+
List.exists
77+
(function
78+
| {Location.txt = "res.partial"}, _ -> true
79+
| _ -> false)
80+
attrs
81+
7582
let processPartialAppAttribute attrs =
7683
let rec process partialApp acc attrs =
7784
match attrs with

jscomp/syntax/src/res_parsetree_viewer.mli

+2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ type functionAttributesInfo = {
2929
attributes: Parsetree.attributes;
3030
}
3131

32+
val hasPartialAttribute : Parsetree.attributes -> bool
33+
3234
(* determines whether a function is async and/or uncurried based on the given attributes *)
3335
val processFunctionAttributes : Parsetree.attributes -> functionAttributesInfo
3436

jscomp/syntax/src/res_printer.ml

+21-5
Original file line numberDiff line numberDiff line change
@@ -4120,7 +4120,13 @@ and printPexpApply ~state expr cmtTbl =
41204120
let partial, attrs = ParsetreeViewer.processPartialAppAttribute attrs in
41214121
let args =
41224122
if partial then
4123-
let dummy = Ast_helper.Exp.constant (Ast_helper.Const.int 0) in
4123+
let loc =
4124+
{Asttypes.txt = "res.partial"; Asttypes.loc = expr.pexp_loc}
4125+
in
4126+
let attr = (loc, Parsetree.PTyp (Ast_helper.Typ.any ())) in
4127+
let dummy =
4128+
Ast_helper.Exp.constant ~attrs:[attr] (Ast_helper.Const.int 0)
4129+
in
41244130
args @ [(Asttypes.Labelled "...", dummy)]
41254131
else args
41264132
in
@@ -4700,6 +4706,18 @@ and printArguments ~state ~dotted ?(partial = false)
47004706
Doc.concat
47014707
[(if dotted then Doc.text "(. " else Doc.lparen); argDoc; Doc.rparen]
47024708
| args ->
4709+
(* Avoid printing trailing comma when there is ... in function application *)
4710+
let hasPartialAttr, printedArgs =
4711+
List.fold_right
4712+
(fun arg (flag, acc) ->
4713+
let _, expr = arg in
4714+
let hasPartialAttr =
4715+
ParsetreeViewer.hasPartialAttribute expr.Parsetree.pexp_attributes
4716+
in
4717+
let doc = printArgument ~state arg cmtTbl in
4718+
(flag || hasPartialAttr, doc :: acc))
4719+
args (false, [])
4720+
in
47034721
Doc.group
47044722
(Doc.concat
47054723
[
@@ -4708,11 +4726,9 @@ and printArguments ~state ~dotted ?(partial = false)
47084726
(Doc.concat
47094727
[
47104728
(if dotted then Doc.line else Doc.softLine);
4711-
Doc.join
4712-
~sep:(Doc.concat [Doc.comma; Doc.line])
4713-
(List.map (fun arg -> printArgument ~state arg cmtTbl) args);
4729+
Doc.join ~sep:(Doc.concat [Doc.comma; Doc.line]) printedArgs;
47144730
]);
4715-
(if partial then Doc.nil else Doc.trailingComma);
4731+
(if partial || hasPartialAttr then Doc.nil else Doc.trailingComma);
47164732
Doc.softLine;
47174733
Doc.rparen;
47184734
])

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

+34
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,37 @@ f(. {
7373

7474
resolve(.)
7575
resolve(. ())
76+
77+
let g = f(
78+
a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
79+
b,
80+
c
81+
)
82+
83+
let g = f(
84+
a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
85+
b,
86+
c,
87+
...
88+
)
89+
90+
let g = f(
91+
a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
92+
b,
93+
c => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(d, ...),
94+
...
95+
)
96+
97+
let g = f(
98+
a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a, ...),
99+
b,
100+
c => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(d),
101+
...
102+
)
103+
104+
105+
let g = f(
106+
a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
107+
b,
108+
c => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(d, ...)
109+
)

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

+51
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,54 @@ f(. {
9393

9494
resolve(.)
9595
resolve(. ())
96+
97+
let g = f(
98+
a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
99+
b,
100+
c,
101+
)
102+
103+
let g =
104+
f(
105+
a =>
106+
LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
107+
b,
108+
c,
109+
...
110+
)
111+
112+
let g =
113+
f(
114+
a =>
115+
LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
116+
b,
117+
c =>
118+
LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(
119+
d,
120+
...
121+
),
122+
...
123+
)
124+
125+
let g =
126+
f(
127+
a =>
128+
LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(
129+
a,
130+
...
131+
),
132+
b,
133+
c =>
134+
LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(d),
135+
...
136+
)
137+
138+
let g = f(
139+
a => LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(a),
140+
b,
141+
c =>
142+
LongModuleName.functionWithAlongNameThatWrapsTheEditorToTheNextLinexxxxxxxxxxxxxxxxxxxxxx(
143+
d,
144+
...
145+
),
146+
)

0 commit comments

Comments
 (0)