Skip to content

Commit 033eda2

Browse files
jtkieseljtkiesel
committed
fix: proper breaking of parenthesized expressions
1 parent aedcd79 commit 033eda2

4 files changed

Lines changed: 183 additions & 29 deletions

File tree

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

Lines changed: 44 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,37 @@ 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 (
394+
(!hasLambda && hasSuffix && (!hasTernary || isAssignment)) ||
395+
(ancestorNode2 &&
396+
["guard", "returnStatement"].includes(ancestorNode2.name))
397+
) {
398+
return indentInParentheses(
399+
hasTernary || ancestorNode2?.name === "returnStatement"
400+
? group(expression)
401+
: expression
402+
);
403+
} else if (hasTernary && hasSuffix && !isAssignment) {
404+
return group(["(", expression, softline, ")"]);
405+
} else {
406+
return group([
407+
"(",
408+
hasLambda || hasTernary ? expression : indent(expression),
409+
")"
410+
]);
411+
}
390412
},
391413

392414
castExpression: printSingle,
@@ -699,8 +721,8 @@ function binary(
699721
operands.unshift(
700722
levelOperator !== undefined &&
701723
needsParentheses(nextOperator, levelOperator)
702-
? ["(", indent(content), ")"]
703-
: content
724+
? ["(", group(indent(content)), ")"]
725+
: group(content)
704726
);
705727
}
706728
}
@@ -711,17 +733,17 @@ function binary(
711733
!isAssignmentOperator(levelOperator) &&
712734
levelOperator !== "instanceof")
713735
) {
714-
return group(level);
736+
return level;
715737
}
716738
if (!isRoot || hasNonAssignmentOperators) {
717-
return group(indent(level));
739+
return indent(level);
718740
}
719741
const groupId = Symbol("assignment");
720-
return group([
742+
return [
721743
level[0],
722744
group(indent(level[1]), { id: groupId }),
723745
indentIfBreak(level[2], { groupId })
724-
]);
746+
];
725747
}
726748

727749
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: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,4 +249,96 @@ 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 &&
292+
dddddddddd &&
293+
eeeeeeeeee
294+
) -> ffffffffff;
295+
}
296+
297+
return (
298+
aaaaaaaaaa &&
299+
bbbbbbbbbb &&
300+
cccccccccc &&
301+
dddddddddd &&
302+
eeeeeeeeee &&
303+
ffffffffff
304+
);
305+
}
306+
307+
void parenthesesWithTrailingBreak() {
308+
(aaaaaaaaaa && bbbbbbbbbb && cccccccccc
309+
? dddddddddd
310+
: eeeeeeeeee
311+
).ffffffffff();
312+
(aaaaaaaaaa && bbbbbbbbbb && cccccccccc
313+
? dddddddddd
314+
: eeeeeeeeee
315+
)::ffffffffff;
316+
(aaaaaaaaaa && bbbbbbbbbb && cccccccccc
317+
? dddddddddd
318+
: eeeeeeeeee
319+
)[ffffffffff];
320+
}
321+
322+
void parenthesesWithoutBreak() {
323+
(aaaaaaaaaa ->
324+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
325+
(aaaaaaaaaa ->
326+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
327+
(aaaaaaaaaa ->
328+
bbbbbbbbbb && cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
329+
330+
aaaaaaaaaa = (bbbbbbbbbb ->
331+
cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
332+
aaaaaaaaaa = (bbbbbbbbbb ->
333+
cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
334+
aaaaaaaaaa = (bbbbbbbbbb ->
335+
cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
336+
337+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb ->
338+
cccccccccc ? dddddddddd : eeeeeeeeee).ffffffffff();
339+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb ->
340+
cccccccccc ? dddddddddd : eeeeeeeeee)::ffffffffff;
341+
Aaaaaaaaaa aaaaaaaaaa = (bbbbbbbbbb ->
342+
cccccccccc ? dddddddddd : eeeeeeeeee)[ffffffffff];
343+
}
252344
}

0 commit comments

Comments
 (0)