Skip to content

Commit 33fae02

Browse files
jtkieseljtkiesel
committed
fix: proper breaking of parenthesized expressions
1 parent aedcd79 commit 33fae02

4 files changed

Lines changed: 178 additions & 29 deletions

File tree

‎packages/prettier-plugin-java/src/printers/expressions.ts‎

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type {
22
FqnOrRefTypeCtx,
3+
PrimaryCstNode,
34
StringTemplateCstNode,
45
TextBlockTemplateCstNode
56
} from "java-parser";
@@ -12,6 +13,7 @@ import {
1213
each,
1314
findBaseIndent,
1415
flatMap,
16+
hasAssignmentOperators,
1517
hasLeadingComments,
1618
hasNonAssignmentOperators,
1719
indentInParentheses,
@@ -117,8 +119,11 @@ export default {
117119

118120
conditionalExpression(path, print, options) {
119121
const binaryExpression = call(path, print, "binaryExpression");
122+
const grandparentNodeName = (path.getNode(4) as JavaNonTerminal | null)
123+
?.name;
124+
const isInParentheses = grandparentNodeName === "parenthesisExpression";
120125
if (!path.node.children.QuestionMark) {
121-
return binaryExpression;
126+
return isInParentheses ? binaryExpression : group(binaryExpression);
122127
}
123128
const [consequent, alternate] = map(path, print, "expression");
124129
const suffix = [
@@ -127,16 +132,16 @@ export default {
127132
line,
128133
[": ", options.useTabs ? indent(alternate) : align(2, alternate)]
129134
];
130-
const isNestedTernary =
131-
(path.getNode(4) as JavaNonTerminal | null)?.name ===
132-
"conditionalExpression";
135+
const isNestedTernary = grandparentNodeName === "conditionalExpression";
133136
const alignedSuffix =
134137
!isNestedTernary || options.useTabs
135138
? suffix
136139
: align(Math.max(0, options.tabWidth - 2), suffix);
137-
return isNestedTernary
138-
? [binaryExpression, alignedSuffix]
139-
: group([binaryExpression, indent(alignedSuffix)]);
140+
if (isNestedTernary) {
141+
return [group(binaryExpression), alignedSuffix];
142+
}
143+
const parts = [group(binaryExpression), indent(alignedSuffix)];
144+
return isInParentheses ? parts : group(parts);
140145
},
141146

142147
binaryExpression(path, print, options) {
@@ -373,20 +378,34 @@ export default {
373378

374379
parenthesisExpression(path, print) {
375380
const expression = call(path, print, "expression");
376-
const ancestorName = (path.getNode(14) as JavaNonTerminal | null)?.name;
377-
const binaryExpression = path.getNode(8) as JavaNonTerminal | null;
381+
const ancestorNode1 = path.getNode(4) as PrimaryCstNode | null;
382+
const ancestorNode2 = path.getNode(14) as JavaNonTerminal | null;
378383
const { conditionalExpression, lambdaExpression } =
379384
path.node.children.expression[0].children;
380385
const hasLambda = lambdaExpression !== undefined;
381386
const hasTernary =
382387
conditionalExpression?.[0].children.QuestionMark !== undefined;
383-
return ancestorName &&
384-
["guard", "returnStatement"].includes(ancestorName) &&
385-
binaryExpression &&
386-
binaryExpression.name === "binaryExpression" &&
387-
Object.keys(binaryExpression.children).length === 1
388-
? indentInParentheses(expression)
389-
: ["(", hasLambda || hasTernary ? expression : indent(expression), ")"];
388+
const hasSuffix = ancestorNode1?.children.primarySuffix !== undefined;
389+
const isAssignment =
390+
(ancestorNode2?.name === "binaryExpression" &&
391+
hasAssignmentOperators(ancestorNode2)) ||
392+
ancestorNode2?.name === "variableInitializer";
393+
if (!hasLambda && hasSuffix && (!hasTernary || isAssignment)) {
394+
return indentInParentheses(hasTernary ? group(expression) : expression);
395+
} else if (
396+
ancestorNode2 &&
397+
["guard", "returnStatement"].includes(ancestorNode2.name)
398+
) {
399+
return indentInParentheses(group(expression));
400+
} else if (hasTernary && hasSuffix && !isAssignment) {
401+
return group(["(", expression, softline, ")"]);
402+
} else {
403+
return group([
404+
"(",
405+
hasLambda || hasTernary ? expression : indent(expression),
406+
")"
407+
]);
408+
}
390409
},
391410

392411
castExpression: printSingle,
@@ -699,8 +718,8 @@ function binary(
699718
operands.unshift(
700719
levelOperator !== undefined &&
701720
needsParentheses(nextOperator, levelOperator)
702-
? ["(", indent(content), ")"]
703-
: content
721+
? ["(", group(indent(content)), ")"]
722+
: group(content)
704723
);
705724
}
706725
}
@@ -711,17 +730,17 @@ function binary(
711730
!isAssignmentOperator(levelOperator) &&
712731
levelOperator !== "instanceof")
713732
) {
714-
return group(level);
733+
return level;
715734
}
716735
if (!isRoot || hasNonAssignmentOperators) {
717-
return group(indent(level));
736+
return indent(level);
718737
}
719738
const groupId = Symbol("assignment");
720-
return group([
739+
return [
721740
level[0],
722741
group(indent(level[1]), { id: groupId }),
723742
indentIfBreak(level[2], { groupId })
724-
]);
743+
];
725744
}
726745

727746
const precedencesByOperator = new Map(

‎packages/prettier-plugin-java/src/printers/helpers.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -376,15 +376,17 @@ export function isBinaryExpression(expression: ExpressionCstNode) {
376376
return hasNonAssignmentOperators(conditionalExpression.binaryExpression[0]);
377377
}
378378

379+
export function hasAssignmentOperators(
380+
binaryExpression: BinaryExpressionCstNode
381+
) {
382+
return binaryExpression.children.AssignmentOperator !== undefined;
383+
}
384+
379385
export function hasNonAssignmentOperators(
380386
binaryExpression: BinaryExpressionCstNode
381387
) {
382-
return Object.values(binaryExpression.children).some(
383-
child =>
384-
isTerminal(child[0]) &&
385-
!child[0].tokenType.CATEGORIES?.some(
386-
category => category.name === "AssignmentOperator"
387-
)
388+
return Object.keys(binaryExpression.children).some(name =>
389+
["BinaryOperator", "Instanceof", "shiftOperator"].includes(name)
388390
);
389391
}
390392

‎packages/prettier-plugin-java/test/unit-test/expressions/_input.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,5 +207,43 @@ public void nonStaticMultipleChainedMethodInvocations() {
207207
public void typeExpressionsInFqnParts() {
208208
var myVariable = ImmutableMap.<R, V>of<T>::a();
209209
}
210-
}
211210

211+
void parenthesesWithLeadingAndTrailingBreak() {
212+
(aaaaaaaaaa + bbbbbbbbbb + cccccccccc + dddddddddd + eeeeeeeeee).ffffffffff();
213+
(aaaaaaaaaa + bbbbbbbbbb + cccccccccc + dddddddddd + eeeeeeeeee)::ffffffffff;
214+
215+
aaaaaaaaaa = (bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
216+
aaaaaaaaaa = (bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
217+
aaaaaaaaaa = (bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
218+
219+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
220+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
221+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
222+
223+
switch (a) {
224+
case Bbbbbbbbbb bbbbbbbbbb when (cccccccccc && dddddddddd && eeeeeeeeee) -> ffffffffff;
225+
}
226+
227+
return (aaaaaaaaaa && bbbbbbbbbb && cccccccccc && dddddddddd && eeeeeeeeee && ffffffffff);
228+
}
229+
230+
void parenthesesWithTrailingBreak() {
231+
(aaaaaaaaaa && bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
232+
(aaaaaaaaaa && bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
233+
(aaaaaaaaaa && bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
234+
}
235+
236+
void parenthesesWithoutBreak() {
237+
(aaaaaaaaaa -> bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
238+
(aaaaaaaaaa -> bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
239+
(aaaaaaaaaa -> bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
240+
241+
aaaaaaaaaa = (bbbbbbbbbb -> cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
242+
aaaaaaaaaa = (bbbbbbbbbb -> cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
243+
aaaaaaaaaa = (bbbbbbbbbb -> cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
244+
245+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb -> cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
246+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb -> cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
247+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb -> cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
248+
}
249+
}

‎packages/prettier-plugin-java/test/unit-test/expressions/_output.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,4 +249,94 @@ public void nonStaticMultipleChainedMethodInvocations() {
249249
public void typeExpressionsInFqnParts() {
250250
var myVariable = ImmutableMap.<R, V>of<T>::a();
251251
}
252+
253+
void parenthesesWithLeadingAndTrailingBreak() {
254+
(
255+
aaaaaaaaaa +
256+
bbbbbbbbbb +
257+
cccccccccc +
258+
dddddddddd +
259+
eeeeeeeeee
260+
).ffffffffff();
261+
(
262+
aaaaaaaaaa +
263+
bbbbbbbbbb +
264+
cccccccccc +
265+
dddddddddd +
266+
eeeeeeeeee
267+
)::ffffffffff;
268+
269+
aaaaaaaaaa = (
270+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee
271+
).ffffffffff();
272+
aaaaaaaaaa = (
273+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee
274+
)::ffffffffff;
275+
aaaaaaaaaa = (
276+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee
277+
)[ffffffffff];
278+
279+
Aaaaaaaaaa aaaaaaaaaa = (
280+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee
281+
).ffffffffff();
282+
Aaaaaaaaaa aaaaaaaaaa = (
283+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee
284+
)::ffffffffff;
285+
Aaaaaaaaaa aaaaaaaaaa = (
286+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee
287+
)[ffffffffff];
288+
289+
switch (a) {
290+
case Bbbbbbbbbb bbbbbbbbbb when (
291+
cccccccccc && dddddddddd && eeeeeeeeee
292+
) -> ffffffffff;
293+
}
294+
295+
return (
296+
aaaaaaaaaa &&
297+
bbbbbbbbbb &&
298+
cccccccccc &&
299+
dddddddddd &&
300+
eeeeeeeeee &&
301+
ffffffffff
302+
);
303+
}
304+
305+
void parenthesesWithTrailingBreak() {
306+
(aaaaaaaaaa && bbbbbbbbbb && cccccccccc
307+
? dddddddddd
308+
: eeeeeeeeee
309+
).ffffffffff();
310+
(aaaaaaaaaa && bbbbbbbbbb && cccccccccc
311+
? dddddddddd
312+
: eeeeeeeeee
313+
)::ffffffffff;
314+
(aaaaaaaaaa && bbbbbbbbbb && cccccccccc
315+
? dddddddddd
316+
: eeeeeeeeee
317+
)[ffffffffff];
318+
}
319+
320+
void parenthesesWithoutBreak() {
321+
(aaaaaaaaaa ->
322+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
323+
(aaaaaaaaaa ->
324+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
325+
(aaaaaaaaaa ->
326+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
327+
328+
aaaaaaaaaa = (bbbbbbbbbb ->
329+
cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
330+
aaaaaaaaaa = (bbbbbbbbbb ->
331+
cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
332+
aaaaaaaaaa = (bbbbbbbbbb ->
333+
cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
334+
335+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb ->
336+
cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
337+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb ->
338+
cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
339+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb ->
340+
cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
341+
}
252342
}

0 commit comments

Comments
 (0)