From 4a367545e1273d07271058447d45990fdf490375 Mon Sep 17 00:00:00 2001 From: Jordan Kiesel Date: Sat, 5 Aug 2023 20:29:29 -0600 Subject: [PATCH] fix: stable format for if expression with trailing comment closes #592 --- .../src/printers/blocks-and-statements.ts | 16 +++++-- .../src/printers/comments/handle-comments.ts | 46 ++++++++++++++++++- .../complex/_output.java | 5 +- .../if-statement/_input.java | 20 ++++++++ .../if-statement/_output.java | 22 +++++++++ .../unit-test/empty_statement/_output.java | 18 +++++--- .../test/unit-test/sealed/_output.java | 11 +++-- 7 files changed, 119 insertions(+), 19 deletions(-) diff --git a/packages/prettier-plugin-java/src/printers/blocks-and-statements.ts b/packages/prettier-plugin-java/src/printers/blocks-and-statements.ts index 993ad624..203b1b5f 100644 --- a/packages/prettier-plugin-java/src/printers/blocks-and-statements.ts +++ b/packages/prettier-plugin-java/src/printers/blocks-and-statements.ts @@ -3,6 +3,7 @@ import { builders } from "prettier/doc"; import { concat, dedent, group, indent, join } from "./prettier-builder"; import { printTokenWithComments } from "./comments/format-comments"; +import { handleCommentsIfStatement } from "./comments/handle-comments"; import { hasLeadingLineComments, hasTrailingLineComments @@ -192,12 +193,19 @@ export class BlocksAndStatementPrettierVisitor extends BaseCstPrettierPrinter { } ifStatement(ctx: IfStatementCtx) { + handleCommentsIfStatement(ctx); + const expression = this.visit(ctx.expression); const ifStatement = this.visit(ctx.statement[0], { allowEmptyStatement: true }); - const ifSeparator = isStatementEmptyStatement(ifStatement) ? "" : " "; + const ifStatementCtx = + ctx.statement[0].children.statementWithoutTrailingSubstatement?.[0] + .children; + const emptyIfStatement = ifStatementCtx?.emptyStatement !== undefined; + const hasIfBlock = ifStatementCtx?.block !== undefined; + const ifSeparator = emptyIfStatement ? "" : hasIfBlock ? " " : indent(line); let elsePart: Doc = ""; if (ctx.Else !== undefined) { @@ -208,7 +216,9 @@ export class BlocksAndStatementPrettierVisitor extends BaseCstPrettierPrinter { const elseOnSameLine = hasTrailingLineComments(ctx.statement[0]) || - hasLeadingLineComments(ctx.Else[0]) + hasLeadingLineComments(ctx.Else[0]) || + emptyIfStatement || + !hasIfBlock ? hardline : " "; @@ -226,7 +236,7 @@ export class BlocksAndStatementPrettierVisitor extends BaseCstPrettierPrinter { ifSeparator ]) ]), - ifStatement, + hasIfBlock ? ifStatement : indent(ifStatement), elsePart ]); } diff --git a/packages/prettier-plugin-java/src/printers/comments/handle-comments.ts b/packages/prettier-plugin-java/src/printers/comments/handle-comments.ts index 59ae4616..818fd7ac 100644 --- a/packages/prettier-plugin-java/src/printers/comments/handle-comments.ts +++ b/packages/prettier-plugin-java/src/printers/comments/handle-comments.ts @@ -1,5 +1,5 @@ -import { hasLeadingComments } from "./comments-utils"; -import { BinaryExpressionCtx, IToken } from "java-parser"; +import { hasLeadingComments, hasTrailingComments } from "./comments-utils"; +import { BinaryExpressionCtx, IToken, IfStatementCtx } from "java-parser"; export function handleCommentsBinaryExpression(ctx: BinaryExpressionCtx) { let unaryExpressionIndex = 1; @@ -43,3 +43,45 @@ export function handleCommentsBinaryExpression(ctx: BinaryExpressionCtx) { }); } } + +export function handleCommentsIfStatement(ctx: IfStatementCtx) { + const rBrace = ctx.RBrace[0]; + if (!hasTrailingComments(rBrace)) { + return; + } + const statement = + ctx.statement[0].children.statementWithoutTrailingSubstatement?.[0] + .children; + if (!statement) { + return; + } + const inner = [ + statement.assertStatement, + statement.breakStatement, + statement.continueStatement, + statement.doStatement, + statement.emptyStatement, + statement.expressionStatement, + statement.expressionStatement, + statement.returnStatement, + statement.switchStatement, + statement.synchronizedStatement, + statement.throwStatement, + statement.tryStatement, + statement.yieldStatement + ].find(s => s)?.[0]; + const block = statement.block?.[0].children.blockStatements?.[0]; + const rCurly = statement.block?.[0].children.RCurly[0]; + const target = inner ?? block ?? rCurly; + if (!target) { + return; + } + if (block) { + block.location.startLine--; + } + if (!target.leadingComments) { + target.leadingComments = []; + } + target.leadingComments.unshift(...rBrace.trailingComments); + delete ctx.RBrace[0].trailingComments; +} diff --git a/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/complex/_output.java b/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/complex/_output.java index 8d0a1a1e..af405df9 100644 --- a/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/complex/_output.java +++ b/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/complex/_output.java @@ -23,9 +23,8 @@ private void myFunction( /* axis y */int arg2, /* axis z */int arg3 ) { - if (arg1 == 0 && arg2 == 0 && arg == 3) throw new RuntimeException( - "X Y Z cannot be all 0" - ); + if (arg1 == 0 && arg2 == 0 && arg == 3) + throw new RuntimeException("X Y Z cannot be all 0"); int /*variable name is of value var */var = arg1 + arg2 + arg3; if /*true*/(var == 0) { diff --git a/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/if-statement/_input.java b/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/if-statement/_input.java index d53f9178..1138b12f 100644 --- a/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/if-statement/_input.java +++ b/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/if-statement/_input.java @@ -15,6 +15,26 @@ void commentsIfLineComment() { if ( // test t) { } + + if (true) // comment + System.out.println("Oops"); + + if (true) { + // comment + } + + if (true) // comment + {} + + if (true) // comment + { + System.out.println("Oops"); + } + + if (true) // comment + { + if (true) {} + } } void commentsIfBlockComment() { diff --git a/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/if-statement/_output.java b/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/if-statement/_output.java index 217808bd..2447014a 100644 --- a/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/if-statement/_output.java +++ b/packages/prettier-plugin-java/test/unit-test/comments/comments-blocks-and-statements/if-statement/_output.java @@ -14,6 +14,28 @@ void commentsIfLineComment() { if ( // test t ) {} + + if (true) + // comment + System.out.println("Oops"); + + if (true) { + // comment + } + + if (true) { + // comment + } + + if (true) { + // comment + System.out.println("Oops"); + } + + if (true) { + // comment + if (true) {} + } } void commentsIfBlockComment() { diff --git a/packages/prettier-plugin-java/test/unit-test/empty_statement/_output.java b/packages/prettier-plugin-java/test/unit-test/empty_statement/_output.java index f1b81fd0..2ddb7abd 100644 --- a/packages/prettier-plugin-java/test/unit-test/empty_statement/_output.java +++ b/packages/prettier-plugin-java/test/unit-test/empty_statement/_output.java @@ -37,7 +37,8 @@ public void forEachWithEmptyStatement(List list) { } public void ifElseWithEmptyStatements() { - if (test); else { + if (test); + else { System.out.println("one"); } @@ -45,15 +46,18 @@ public void ifElseWithEmptyStatements() { System.out.println("two"); } else; - if (test); else; + if (test); + else; } public void ifElseWithEmptyStatementsWithComments() { - if (test) /*test*/; else { + if (test)/*test*/; + else { System.out.println("one"); } - if (test); /*test*/else { + if (test); + /*test*/else { System.out.println("one"); } @@ -65,9 +69,11 @@ public void ifElseWithEmptyStatementsWithComments() { System.out.println("two"); } else;/*test*/ - if (test); /*test*/else;/*test*/ + if (test); + /*test*/else;/*test*/ - if (test) /*test*/; else /*test*/; + if (test)/*test*/; + else /*test*/; } public void simpleWhileWithEmptyStatement(boolean one) { diff --git a/packages/prettier-plugin-java/test/unit-test/sealed/_output.java b/packages/prettier-plugin-java/test/unit-test/sealed/_output.java index 992600be..97518d2d 100644 --- a/packages/prettier-plugin-java/test/unit-test/sealed/_output.java +++ b/packages/prettier-plugin-java/test/unit-test/sealed/_output.java @@ -46,11 +46,12 @@ default Shape rotate(double angle) { } default String areaMessage() { - if (this instanceof Circle) return "Circle: " + area(); else if ( - this instanceof Rectangle - ) return "Rectangle: " + area(); else if ( - this instanceof RightTriangle - ) return "Triangle: " + area(); + if (this instanceof Circle) + return "Circle: " + area(); + else if (this instanceof Rectangle) + return "Rectangle: " + area(); + else if (this instanceof RightTriangle) + return "Triangle: " + area(); // :( throw new IllegalArgumentException(); }