Skip to content

Commit 9e931a9

Browse files
jtkieseljtkiesel
committed
fix: ternary comment placement and indentation
1 parent 9ef6af0 commit 9e931a9

13 files changed

Lines changed: 251 additions & 138 deletions

File tree

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

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ export function handleLineComment(
9494
) {
9595
return [
9696
handleBinaryExpressionComments,
97+
handleConditionalExpressionComments,
9798
handleFqnOrRefTypeComments,
9899
handleIfStatementComments,
99100
handleJumpStatementComments,
@@ -116,11 +117,7 @@ function handleBinaryExpressionComments(
116117
options: JavaParserOptions
117118
) {
118119
const { enclosingNode, precedingNode, followingNode } = commentNode;
119-
if (
120-
enclosingNode &&
121-
isNonTerminal(enclosingNode) &&
122-
enclosingNode.name === "binaryExpression"
123-
) {
120+
if (enclosingNode?.name === "binaryExpression") {
124121
if (isBinaryOperator(followingNode)) {
125122
if (options.experimentalOperatorPosition === "start") {
126123
util.addLeadingComment(followingNode, commentNode);
@@ -139,26 +136,37 @@ function handleBinaryExpressionComments(
139136
return false;
140137
}
141138

142-
function handleFqnOrRefTypeComments(commentNode: JavaComment) {
143-
const { enclosingNode, followingNode } = commentNode;
139+
function handleConditionalExpressionComments(commentNode: JavaComment) {
140+
const { startLine, endLine, enclosingNode, precedingNode, followingNode } =
141+
commentNode;
144142
if (
145-
enclosingNode &&
146-
isNonTerminal(enclosingNode) &&
147-
enclosingNode.name === "fqnOrRefType" &&
148-
followingNode
143+
enclosingNode?.name === "conditionalExpression" &&
144+
precedingNode &&
145+
followingNode &&
146+
isNonTerminal(precedingNode) &&
147+
isNonTerminal(followingNode) &&
148+
precedingNode.location.endLine < startLine &&
149+
endLine < followingNode.location.startLine
149150
) {
150151
util.addLeadingComment(followingNode, commentNode);
151152
return true;
152153
}
153154
return false;
154155
}
155156

157+
function handleFqnOrRefTypeComments(commentNode: JavaComment) {
158+
const { enclosingNode, followingNode } = commentNode;
159+
if (enclosingNode?.name === "fqnOrRefType" && followingNode) {
160+
util.addLeadingComment(followingNode, commentNode);
161+
return true;
162+
}
163+
return false;
164+
}
165+
156166
function handleIfStatementComments(commentNode: JavaComment) {
157167
const { enclosingNode, precedingNode } = commentNode;
158168
if (
159-
enclosingNode &&
160-
isNonTerminal(enclosingNode) &&
161-
enclosingNode.name === "ifStatement" &&
169+
enclosingNode?.name === "ifStatement" &&
162170
precedingNode &&
163171
isNonTerminal(precedingNode) &&
164172
precedingNode.name === "statement"
@@ -175,7 +183,6 @@ function handleJumpStatementComments(commentNode: JavaComment) {
175183
enclosingNode &&
176184
!precedingNode &&
177185
!followingNode &&
178-
isNonTerminal(enclosingNode) &&
179186
["breakStatement", "continueStatement", "returnStatement"].includes(
180187
enclosingNode.name
181188
)
@@ -189,10 +196,8 @@ function handleJumpStatementComments(commentNode: JavaComment) {
189196
function handleLabeledStatementComments(commentNode: JavaComment) {
190197
const { enclosingNode, precedingNode } = commentNode;
191198
if (
192-
enclosingNode &&
199+
enclosingNode?.name === "labeledStatement" &&
193200
precedingNode &&
194-
isNonTerminal(enclosingNode) &&
195-
enclosingNode.name === "labeledStatement" &&
196201
isTerminal(precedingNode) &&
197202
precedingNode.tokenType.name === "Identifier"
198203
) {
@@ -205,9 +210,7 @@ function handleLabeledStatementComments(commentNode: JavaComment) {
205210
function handleMethodDeclaratorComments(commentNode: JavaComment) {
206211
const { enclosingNode } = commentNode;
207212
if (
208-
enclosingNode &&
209-
isNonTerminal(enclosingNode) &&
210-
enclosingNode.name === "methodDeclarator" &&
213+
enclosingNode?.name === "methodDeclarator" &&
211214
!enclosingNode.children.receiverParameter &&
212215
!enclosingNode.children.formalParameterList &&
213216
enclosingNode.children.LBrace[0].startOffset < commentNode.startOffset &&
@@ -224,7 +227,6 @@ function handleNameComments(commentNode: JavaComment) {
224227
if (
225228
enclosingNode &&
226229
precedingNode &&
227-
isNonTerminal(enclosingNode) &&
228230
isTerminal(precedingNode) &&
229231
precedingNode.tokenType.name === "Identifier" &&
230232
[
@@ -262,8 +264,8 @@ export type JavaComment = IToken & {
262264
trailing: boolean;
263265
printed: boolean;
264266
enclosingNode?: JavaNonTerminal;
265-
precedingNode?: JavaNonTerminal;
266-
followingNode?: JavaNonTerminal;
267+
precedingNode?: JavaNode;
268+
followingNode?: JavaNode;
267269
};
268270

269271
type FormatterOffOnRange = {

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

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
} from "./helpers.js";
3131

3232
const {
33+
align,
3334
breakParent,
3435
conditionalGroup,
3536
group,
@@ -114,23 +115,28 @@ export default {
114115
conciseLambdaParameter: printSingle,
115116
lambdaBody: printSingle,
116117

117-
conditionalExpression(path, print) {
118+
conditionalExpression(path, print, options) {
118119
const binaryExpression = call(path, print, "binaryExpression");
119120
if (!path.node.children.QuestionMark) {
120121
return binaryExpression;
121122
}
122-
const expressions = map(path, print, "expression");
123-
const contents = indent(
124-
join(line, [
125-
binaryExpression,
126-
["? ", expressions[0]],
127-
[": ", expressions[1]]
128-
])
129-
);
123+
const [consequent, alternate] = map(path, print, "expression");
124+
const parts = [binaryExpression];
125+
const part = [
126+
line,
127+
["? ", options.useTabs ? indent(consequent) : align(2, consequent)],
128+
line,
129+
[": ", options.useTabs ? indent(alternate) : align(2, alternate)]
130+
];
130131
const isNestedTernary =
131132
(path.getNode(4) as JavaNonTerminal | null)?.name ===
132133
"conditionalExpression";
133-
return isNestedTernary ? contents : group(contents);
134+
parts.push(
135+
!isNestedTernary || options.useTabs
136+
? part
137+
: align(Math.max(0, options.tabWidth - 2), part)
138+
);
139+
return isNestedTernary ? parts : group(indent(parts));
134140
},
135141

136142
binaryExpression(path, print, options) {

‎packages/prettier-plugin-java/test/unit-test/binary_expressions/operator-position-end/_input.java‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,25 +17,6 @@ public String binaryOperationThatShouldNotBreak() {
1717
return "This operation should" + "not break";
1818
}
1919

20-
public int ternaryOperationThatShouldBreak() {
21-
int shortInteger = thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne ? thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne : thisIsAShortInteger;
22-
return thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne ? thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne : thisIsAShortInteger;
23-
}
24-
25-
public int ternaryOperationThatShouldBreak2() {
26-
int shortInteger = thisIsAVeryLongInteger ? thisIsAnotherVeryLongOne : thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne;
27-
return thisIsAVeryLongInteger ? thisIsAnotherVeryLongOne : thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne;
28-
}
29-
30-
public int ternaryOperationThatShouldNotBreak() {
31-
int a = b ? b : c;
32-
return b ? b : c;
33-
}
34-
35-
void nestedTernary() {
36-
aaaaaaaaaa ? bbbbbbbbbb : cccccccccc ? dddddddddd : eeeeeeeeee ? ffffffffff : gggggggggg;
37-
}
38-
3920
public boolean binaryOperationWithComments() {
4021
boolean a = one || two >> 1 // one
4122
// two

‎packages/prettier-plugin-java/test/unit-test/binary_expressions/operator-position-end/_output.java‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -26,39 +26,6 @@ public String binaryOperationThatShouldNotBreak() {
2626
return "This operation should" + "not break";
2727
}
2828

29-
public int ternaryOperationThatShouldBreak() {
30-
int shortInteger = thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne
31-
? thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne
32-
: thisIsAShortInteger;
33-
return thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne
34-
? thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne
35-
: thisIsAShortInteger;
36-
}
37-
38-
public int ternaryOperationThatShouldBreak2() {
39-
int shortInteger = thisIsAVeryLongInteger
40-
? thisIsAnotherVeryLongOne
41-
: thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne;
42-
return thisIsAVeryLongInteger
43-
? thisIsAnotherVeryLongOne
44-
: thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne;
45-
}
46-
47-
public int ternaryOperationThatShouldNotBreak() {
48-
int a = b ? b : c;
49-
return b ? b : c;
50-
}
51-
52-
void nestedTernary() {
53-
aaaaaaaaaa
54-
? bbbbbbbbbb
55-
: cccccccccc
56-
? dddddddddd
57-
: eeeeeeeeee
58-
? ffffffffff
59-
: gggggggggg;
60-
}
61-
6229
public boolean binaryOperationWithComments() {
6330
boolean a =
6431
one ||

‎packages/prettier-plugin-java/test/unit-test/binary_expressions/operator-position-start/_input.java‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,25 +17,6 @@ public String binaryOperationThatShouldNotBreak() {
1717
return "This operation should" + "not break";
1818
}
1919

20-
public int ternaryOperationThatShouldBreak() {
21-
int shortInteger = thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne ? thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne : thisIsAShortInteger;
22-
return thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne ? thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne : thisIsAShortInteger;
23-
}
24-
25-
public int ternaryOperationThatShouldBreak2() {
26-
int shortInteger = thisIsAVeryLongInteger ? thisIsAnotherVeryLongOne : thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne;
27-
return thisIsAVeryLongInteger ? thisIsAnotherVeryLongOne : thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne;
28-
}
29-
30-
public int ternaryOperationThatShouldNotBreak() {
31-
int a = b ? b : c;
32-
return b ? b : c;
33-
}
34-
35-
void nestedTernary() {
36-
aaaaaaaaaa ? bbbbbbbbbb : cccccccccc ? dddddddddd : eeeeeeeeee ? ffffffffff : gggggggggg;
37-
}
38-
3920
public boolean binaryOperationWithComments() {
4021
boolean a = one || two >> 1 // one
4122
// two

‎packages/prettier-plugin-java/test/unit-test/binary_expressions/operator-position-start/_output.java‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -26,39 +26,6 @@ public String binaryOperationThatShouldNotBreak() {
2626
return "This operation should" + "not break";
2727
}
2828

29-
public int ternaryOperationThatShouldBreak() {
30-
int shortInteger = thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne
31-
? thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne
32-
: thisIsAShortInteger;
33-
return thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne
34-
? thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne
35-
: thisIsAShortInteger;
36-
}
37-
38-
public int ternaryOperationThatShouldBreak2() {
39-
int shortInteger = thisIsAVeryLongInteger
40-
? thisIsAnotherVeryLongOne
41-
: thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne;
42-
return thisIsAVeryLongInteger
43-
? thisIsAnotherVeryLongOne
44-
: thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne;
45-
}
46-
47-
public int ternaryOperationThatShouldNotBreak() {
48-
int a = b ? b : c;
49-
return b ? b : c;
50-
}
51-
52-
void nestedTernary() {
53-
aaaaaaaaaa
54-
? bbbbbbbbbb
55-
: cccccccccc
56-
? dddddddddd
57-
: eeeeeeeeee
58-
? ffffffffff
59-
: gggggggggg;
60-
}
61-
6229
public boolean binaryOperationWithComments() {
6330
boolean a =
6431
one

‎packages/prettier-plugin-java/test/unit-test/conditional-expression/conditional-expression.spec.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import path from "path";
2+
import url from "url";
3+
import { testSample } from "../../test-utils.js";
4+
5+
const __dirname = path.dirname(url.fileURLToPath(import.meta.url));
6+
7+
describe("prettier-java", () => {
8+
testSample(path.resolve(__dirname, "spaces"));
9+
testSample(path.resolve(__dirname, "tabs"));
10+
});

‎packages/prettier-plugin-java/test/unit-test/conditional-expression/spaces/.prettierrc.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"tabWidth": 4
3+
}

‎packages/prettier-plugin-java/test/unit-test/conditional-expression/spaces/_input.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
class ConditionalExpression {
2+
3+
int ternaryOperationThatShouldBreak() {
4+
int shortInteger = thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne ? thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne : thisIsAShortInteger;
5+
return thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne ? thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne : thisIsAShortInteger;
6+
}
7+
8+
int ternaryOperationThatShouldBreak2() {
9+
int shortInteger = thisIsAVeryLongInteger ? thisIsAnotherVeryLongOne : thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne;
10+
return thisIsAVeryLongInteger ? thisIsAnotherVeryLongOne : thisIsAnotherVeryLongIntegerThatIsEvenLongerThanFirstOne;
11+
}
12+
13+
int ternaryOperationThatShouldNotBreak() {
14+
int a = b ? b : c;
15+
return b ? b : c;
16+
}
17+
18+
void nestedTernary() {
19+
aaaaaaaaaa ? bbbbbbbbbb : cccccccccc ? dddddddddd : eeeeeeeeee ? ffffffffff : gggggggggg;
20+
}
21+
22+
void ternaryWithComments() {
23+
a
24+
? // b
25+
b
26+
: // c
27+
c;
28+
a
29+
// b
30+
? b
31+
// c
32+
: c;
33+
a ? // b
34+
b
35+
: // c
36+
c;
37+
a
38+
? b // b
39+
: c; // c
40+
}
41+
}

0 commit comments

Comments
 (0)