Skip to content

Commit 35e72f0

Browse files
jtkieseljtkiesel
committed
fix: proper breaking of parenthesized expressions
1 parent 4ab4d33 commit 35e72f0

4 files changed

Lines changed: 184 additions & 29 deletions

File tree

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

Lines changed: 47 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import type {
2+
BinaryExpressionCstNode,
23
FqnOrRefTypeCtx,
4+
PrimaryCstNode,
35
StringTemplateCstNode,
46
TextBlockTemplateCstNode
57
} from "java-parser";
@@ -12,6 +14,7 @@ import {
1214
each,
1315
findBaseIndent,
1416
flatMap,
17+
hasAssignmentOperators,
1518
hasLeadingComments,
1619
hasNonAssignmentOperators,
1720
indentInParentheses,
@@ -117,8 +120,11 @@ export default {
117120

118121
conditionalExpression(path, print, options) {
119122
const binaryExpression = call(path, print, "binaryExpression");
123+
const grandparentNodeName = (path.getNode(4) as JavaNonTerminal | null)
124+
?.name;
125+
const isInParentheses = grandparentNodeName === "parenthesisExpression";
120126
if (!path.node.children.QuestionMark) {
121-
return binaryExpression;
127+
return isInParentheses ? binaryExpression : group(binaryExpression);
122128
}
123129
const [consequent, alternate] = map(path, print, "expression");
124130
const suffix = [
@@ -127,16 +133,16 @@ export default {
127133
line,
128134
[": ", options.useTabs ? indent(alternate) : align(2, alternate)]
129135
];
130-
const isNestedTernary =
131-
(path.getNode(4) as JavaNonTerminal | null)?.name ===
132-
"conditionalExpression";
136+
const isNestedTernary = grandparentNodeName === "conditionalExpression";
133137
const alignedSuffix =
134138
!isNestedTernary || options.useTabs
135139
? suffix
136140
: align(Math.max(0, options.tabWidth - 2), suffix);
137-
return isNestedTernary
138-
? [binaryExpression, alignedSuffix]
139-
: group([binaryExpression, indent(alignedSuffix)]);
141+
if (isNestedTernary) {
142+
return [group(binaryExpression), alignedSuffix];
143+
}
144+
const parts = [group(binaryExpression), indent(alignedSuffix)];
145+
return isInParentheses ? parts : group(parts);
140146
},
141147

142148
binaryExpression(path, print, options) {
@@ -373,20 +379,39 @@ export default {
373379

374380
parenthesisExpression(path, print) {
375381
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;
382+
const primaryAncestor = path.getNode(4) as PrimaryCstNode | null;
383+
const binaryExpressionAncestor = path.getNode(
384+
8
385+
) as BinaryExpressionCstNode | null;
386+
const outerAncestor = path.getNode(14) as JavaNonTerminal | null;
378387
const { conditionalExpression, lambdaExpression } =
379388
path.node.children.expression[0].children;
380389
const hasLambda = lambdaExpression !== undefined;
381390
const hasTernary =
382391
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), ")"];
392+
const hasSuffix = primaryAncestor?.children.primarySuffix !== undefined;
393+
const isAssignment =
394+
(outerAncestor?.name === "binaryExpression" &&
395+
hasAssignmentOperators(outerAncestor)) ||
396+
outerAncestor?.name === "variableInitializer";
397+
if (!hasLambda && hasSuffix && (!hasTernary || isAssignment)) {
398+
return indentInParentheses(hasTernary ? group(expression) : expression);
399+
} else if (
400+
binaryExpressionAncestor &&
401+
Object.keys(binaryExpressionAncestor.children).length === 1 &&
402+
outerAncestor &&
403+
["guard", "returnStatement"].includes(outerAncestor.name)
404+
) {
405+
return indentInParentheses(group(expression));
406+
} else if (hasTernary && hasSuffix && !isAssignment) {
407+
return group(["(", expression, softline, ")"]);
408+
} else {
409+
return group([
410+
"(",
411+
hasLambda || hasTernary ? expression : indent(expression),
412+
")"
413+
]);
414+
}
390415
},
391416

392417
castExpression: printSingle,
@@ -699,8 +724,8 @@ function binary(
699724
operands.unshift(
700725
levelOperator !== undefined &&
701726
needsParentheses(nextOperator, levelOperator)
702-
? ["(", indent(content), ")"]
703-
: content
727+
? ["(", group(indent(content)), ")"]
728+
: group(content)
704729
);
705730
}
706731
}
@@ -711,17 +736,17 @@ function binary(
711736
!isAssignmentOperator(levelOperator) &&
712737
levelOperator !== "instanceof")
713738
) {
714-
return group(level);
739+
return level;
715740
}
716741
if (!isRoot || hasNonAssignmentOperators) {
717-
return group(indent(level));
742+
return indent(level);
718743
}
719744
const groupId = Symbol("assignment");
720-
return group([
745+
return [
721746
level[0],
722747
group(indent(level[1]), { id: groupId }),
723748
indentIfBreak(level[2], { groupId })
724-
]);
749+
];
725750
}
726751

727752
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)