Skip to content

Commit b98cf67

Browse files
jtkieselclementdessoude
jtkiesel
authored and
clementdessoude
committed
feat: improve binary expression formatting
1 parent 9f2687d commit b98cf67

File tree

7 files changed

+376
-244
lines changed

7 files changed

+376
-244
lines changed

‎packages/prettier-plugin-java/src/printers/comments/handle-comments.ts

Lines changed: 80 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,98 @@
1-
import { hasLeadingComments } from "./comments-utils";
2-
import { BinaryExpressionCtx, IToken } from "java-parser";
1+
import { hasLeadingComments, hasTrailingComments } from "./comments-utils";
2+
import {
3+
BinaryExpressionCtx,
4+
IToken,
5+
UnaryExpressionCstNode
6+
} from "java-parser";
37

48
export function handleCommentsBinaryExpression(ctx: BinaryExpressionCtx) {
9+
moveOperatorLeadingCommentsToNextExpression(ctx);
10+
moveExpressionTrailingCommentsToNextOperator(ctx);
11+
}
12+
13+
function moveOperatorLeadingCommentsToNextExpression(ctx: BinaryExpressionCtx) {
514
let unaryExpressionIndex = 1;
6-
if (ctx.BinaryOperator !== undefined) {
7-
ctx.BinaryOperator.forEach(binaryOperator => {
8-
if (hasLeadingComments(binaryOperator)) {
15+
ctx.BinaryOperator?.forEach(binaryOperator => {
16+
if (hasLeadingComments(binaryOperator)) {
17+
while (
18+
ctx.unaryExpression[unaryExpressionIndex].location.startOffset <
19+
binaryOperator.endOffset
20+
) {
21+
unaryExpressionIndex++;
22+
}
23+
24+
// Adapt the position of the operator and its leading comments
25+
const shiftUp =
26+
binaryOperator.leadingComments[0].startLine -
27+
1 -
28+
binaryOperator.startLine;
29+
30+
if (
31+
binaryOperator.startLine !==
32+
ctx.unaryExpression[unaryExpressionIndex].location.startLine
33+
) {
34+
binaryOperator.leadingComments.forEach(comment => {
35+
comment.startLine += 1;
36+
comment.endLine += 1;
37+
});
38+
}
39+
binaryOperator.startLine += shiftUp;
40+
binaryOperator.endLine += shiftUp;
41+
42+
// Move binaryOperator's leading comments to the following
43+
// unaryExpression
44+
ctx.unaryExpression[unaryExpressionIndex].leadingComments =
45+
ctx.unaryExpression[unaryExpressionIndex].leadingComments || [];
46+
ctx.unaryExpression[unaryExpressionIndex].leadingComments!.unshift(
47+
...binaryOperator.leadingComments
48+
);
49+
delete (binaryOperator as IToken).leadingComments;
50+
}
51+
});
52+
}
53+
54+
function moveExpressionTrailingCommentsToNextOperator(
55+
ctx: BinaryExpressionCtx
56+
) {
57+
const binaryOperators = ctx.BinaryOperator;
58+
let binaryOperatorIndex = 1;
59+
if (binaryOperators?.length) {
60+
ctx.unaryExpression.forEach(unaryExpression => {
61+
if (hasTrailingComments(unaryExpression)) {
962
while (
10-
ctx.unaryExpression[unaryExpressionIndex].location.startOffset <
11-
binaryOperator.endOffset
63+
binaryOperatorIndex < binaryOperators.length &&
64+
unaryExpression.location.endOffset &&
65+
binaryOperators[binaryOperatorIndex].startOffset <
66+
unaryExpression.location.endOffset
1267
) {
13-
unaryExpressionIndex++;
68+
binaryOperatorIndex++;
1469
}
70+
const binaryOperator = binaryOperators[binaryOperatorIndex];
1571

16-
// Adapt the position of the operator and its leading comments
72+
// Adapt the position of the expression and its trailing comments
1773
const shiftUp =
18-
binaryOperator.leadingComments[0].startLine -
74+
unaryExpression.trailingComments[0].startLine -
1975
1 -
20-
binaryOperator.startLine;
76+
unaryExpression.location.startLine;
2177

22-
if (
23-
binaryOperator.startLine !==
24-
ctx.unaryExpression[unaryExpressionIndex].location.startLine
25-
) {
26-
binaryOperator.leadingComments.forEach(comment => {
78+
if (unaryExpression.location.startLine !== binaryOperator.startLine) {
79+
unaryExpression.trailingComments.forEach(comment => {
2780
comment.startLine += 1;
2881
comment.endLine += 1;
2982
});
3083
}
31-
binaryOperator.startLine += shiftUp;
32-
binaryOperator.endLine += shiftUp;
33-
34-
// Assign the leading comments & trailing comments of the binaryOperator
35-
// to the following unaryExpression as leading comments
36-
ctx.unaryExpression[unaryExpressionIndex].leadingComments =
37-
ctx.unaryExpression[unaryExpressionIndex].leadingComments || [];
38-
ctx.unaryExpression[unaryExpressionIndex].leadingComments!.unshift(
39-
...binaryOperator.leadingComments
84+
unaryExpression.location.startLine += shiftUp;
85+
if (unaryExpression.location.endLine !== undefined) {
86+
unaryExpression.location.endLine += shiftUp;
87+
}
88+
89+
// Move unaryExpression's trailing comments to the following
90+
// binaryOperator
91+
binaryOperator.trailingComments = binaryOperator.trailingComments ?? [];
92+
binaryOperator.trailingComments.unshift(
93+
...unaryExpression.trailingComments
4094
);
41-
delete (binaryOperator as IToken).leadingComments;
95+
delete (unaryExpression as UnaryExpressionCstNode).trailingComments;
4296
}
4397
});
4498
}

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

Lines changed: 52 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import {
4141
ReferenceTypeCastExpressionCtx,
4242
RegularLambdaParameterCtx,
4343
TernaryExpressionCtx,
44-
TypeArgumentListCtx,
4544
TypeArgumentsOrDiamondCtx,
4645
TypePatternCtx,
4746
UnaryExpressionCtx,
@@ -50,7 +49,6 @@ import {
5049
} from "java-parser/api";
5150

5251
import forEach from "lodash/forEach";
53-
import { Doc } from "prettier";
5452
import { builders } from "prettier/doc";
5553
import { BaseCstPrettierPrinter } from "../base-cst-printer";
5654
import { isAnnotationCstNode } from "../types/utils";
@@ -63,20 +61,18 @@ import { printTokenWithComments } from "./comments/format-comments";
6361
import { handleCommentsBinaryExpression } from "./comments/handle-comments";
6462
import { concat, dedent, group, indent } from "./prettier-builder";
6563
import {
64+
binary,
6665
findDeepElementInPartsArray,
6766
isExplicitLambdaParameter,
68-
isShiftOperator,
6967
isUniqueMethodInvocation,
70-
matchCategory,
7168
putIntoBraces,
7269
rejectAndConcat,
7370
rejectAndJoin,
7471
rejectAndJoinSeps,
75-
separateTokensIntoGroups,
7672
sortAnnotationIdentifier,
77-
sortNodes
73+
sortNodes,
74+
sortTokens
7875
} from "./printer-utils";
79-
import join = builders.join;
8076

8177
const { ifBreak, line, softline, indentIfBreak } = builders;
8278

@@ -244,104 +240,54 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter {
244240
return binaryExpression;
245241
}
246242

247-
binaryExpression(ctx: BinaryExpressionCtx, params: any) {
243+
binaryExpression(
244+
ctx: BinaryExpressionCtx,
245+
params?: { addParenthesisToWrapStatement?: boolean }
246+
) {
248247
handleCommentsBinaryExpression(ctx);
249248

250-
const instanceofReferences = this.mapVisit(
251-
sortNodes([ctx.pattern, ctx.referenceType])
252-
);
253-
const expression = this.mapVisit(ctx.expression);
254-
const unaryExpression = this.mapVisit(ctx.unaryExpression);
255-
256-
const { groupsOfOperator, sortedBinaryOperators } =
257-
separateTokensIntoGroups(ctx);
258-
const segmentsSplitByBinaryOperator: any[] = [];
259-
let currentSegment = [];
249+
const sortedNodes = sortNodes([
250+
ctx.pattern,
251+
ctx.referenceType,
252+
ctx.expression,
253+
ctx.unaryExpression
254+
]);
260255

261-
if (groupsOfOperator.length === 1 && groupsOfOperator[0].length === 0) {
262-
return unaryExpression.shift();
263-
}
256+
const nodes = this.mapVisit(
257+
sortedNodes,
258+
sortedNodes.length === 1 ? params : undefined
259+
);
260+
const tokens = sortTokens([
261+
ctx.Instanceof,
262+
ctx.AssignmentOperator,
263+
ctx.Less,
264+
ctx.Greater,
265+
ctx.BinaryOperator
266+
]);
267+
const hasTokens = tokens.length > 0;
264268

265-
groupsOfOperator.forEach(subgroup => {
266-
currentSegment = [unaryExpression.shift()];
267-
for (let i = 0; i < subgroup.length; i++) {
268-
const token = subgroup[i];
269-
const shiftOperator = isShiftOperator(subgroup, i);
270-
if (token.tokenType.name === "Instanceof") {
271-
currentSegment.push(
272-
rejectAndJoin(" ", [
273-
ctx.Instanceof![0],
274-
instanceofReferences.shift()
275-
])
276-
);
277-
} else if (matchCategory(token, "'AssignmentOperator'")) {
278-
currentSegment.push(
279-
indent(rejectAndJoin(line, [token, expression.shift()]))
280-
);
281-
} else if (
282-
shiftOperator === "leftShift" ||
283-
shiftOperator === "rightShift"
284-
) {
285-
currentSegment.push(
286-
rejectAndJoin(" ", [
287-
rejectAndConcat([token, subgroup[i + 1]]),
288-
unaryExpression.shift()
289-
])
290-
);
291-
i++;
292-
} else if (shiftOperator === "doubleRightShift") {
293-
currentSegment.push(
294-
rejectAndJoin(" ", [
295-
rejectAndConcat([token, subgroup[i + 1], subgroup[i + 2]]),
296-
unaryExpression.shift()
297-
])
298-
);
299-
i += 2;
300-
} else if (matchCategory(token, "'BinaryOperator'")) {
301-
currentSegment.push(
302-
rejectAndJoin(line, [token, unaryExpression.shift()])
303-
);
304-
}
305-
}
306-
segmentsSplitByBinaryOperator.push(
307-
group(rejectAndJoin(" ", currentSegment))
308-
);
309-
});
269+
const content = binary(nodes, tokens, true);
310270

311-
if (params !== undefined && params.addParenthesisToWrapStatement) {
312-
return group(
313-
concat([
314-
ifBreak("(", ""),
315-
indent(
316-
concat([
317-
softline,
318-
group(
319-
rejectAndJoinSeps(
320-
sortedBinaryOperators.map(elt => concat([" ", elt, line])),
321-
segmentsSplitByBinaryOperator
322-
)
323-
)
324-
])
325-
),
326-
softline,
327-
ifBreak(")")
328-
])
329-
);
330-
}
331-
332-
return group(
333-
rejectAndJoinSeps(
334-
sortedBinaryOperators.map(elt => concat([" ", elt, line])),
335-
segmentsSplitByBinaryOperator
336-
)
337-
);
271+
return hasTokens && params?.addParenthesisToWrapStatement
272+
? group(
273+
concat([
274+
ifBreak("("),
275+
indent(concat([softline, content])),
276+
softline,
277+
ifBreak(")")
278+
])
279+
)
280+
: content;
338281
}
339282

340-
unaryExpression(ctx: UnaryExpressionCtx) {
283+
unaryExpression(
284+
ctx: UnaryExpressionCtx,
285+
params?: { addParenthesisToWrapStatement?: boolean }
286+
) {
341287
const unaryPrefixOperator = ctx.UnaryPrefixOperator
342288
? ctx.UnaryPrefixOperator
343289
: [];
344-
const primary = this.visit(ctx.primary);
290+
const primary = this.visit(ctx.primary, params);
345291
const unarySuffixOperator = ctx.UnarySuffixOperator
346292
? ctx.UnarySuffixOperator
347293
: [];
@@ -369,10 +315,14 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter {
369315
]);
370316
}
371317

372-
primary(ctx: PrimaryCtx) {
318+
primary(
319+
ctx: PrimaryCtx,
320+
params?: { addParenthesisToWrapStatement?: boolean }
321+
) {
373322
const countMethodInvocation = isUniqueMethodInvocation(ctx.primarySuffix);
374323

375324
const primaryPrefix = this.visit(ctx.primaryPrefix, {
325+
...params,
376326
shouldBreakBeforeFirstMethodInvocation: countMethodInvocation > 1
377327
});
378328

@@ -561,9 +511,13 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter {
561511
return rejectAndConcat([keyWord, typeArguments]);
562512
}
563513

564-
parenthesisExpression(ctx: ParenthesisExpressionCtx) {
514+
parenthesisExpression(
515+
ctx: ParenthesisExpressionCtx,
516+
params?: { addParenthesisToWrapStatement?: boolean }
517+
) {
565518
const expression = this.visit(ctx.expression);
566-
return putIntoBraces(expression, softline, ctx.LBrace[0], ctx.RBrace[0]);
519+
const separator = params?.addParenthesisToWrapStatement ? softline : "";
520+
return putIntoBraces(expression, separator, ctx.LBrace[0], ctx.RBrace[0]);
567521
}
568522

569523
castExpression(ctx: CastExpressionCtx) {

0 commit comments

Comments
 (0)