From 8bb764111b1fe6d589b7b429fa762c6635e15160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Dessoude?= Date: Wed, 22 Jan 2025 19:44:16 +0100 Subject: [PATCH] Fix/712 handle comments with static invocations (#715) * fix: handle edge cases with comments in fqn parts * fix: do not add dot before dims in fqn parts * refactor: simplify logic for printing fqn parts --- .../src/printers/expressions.ts | 87 +++++----------- .../test/unit-test/member_chain/_input.java | 98 +++++++++++++++---- .../test/unit-test/member_chain/_output.java | 67 ++++++++++++- 3 files changed, 164 insertions(+), 88 deletions(-) diff --git a/packages/prettier-plugin-java/src/printers/expressions.ts b/packages/prettier-plugin-java/src/printers/expressions.ts index 68b7ea30..6bec3e23 100644 --- a/packages/prettier-plugin-java/src/printers/expressions.ts +++ b/packages/prettier-plugin-java/src/printers/expressions.ts @@ -367,15 +367,10 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter { const fqnOrRefType = ctx.primaryPrefix[0].children.fqnOrRefType?.[0].children; const hasFqnRefPart = fqnOrRefType?.fqnOrRefTypePartRest !== undefined; - const { - isCapitalizedIdentifier, - isCapitalizedIdentifierWithoutTrailingComment - } = this.handleStaticInvocations(fqnOrRefType); + const isCapitalizedIdentifier = this.isCapitalizedIdentifier(fqnOrRefType); const shouldBreakBeforeFirstMethodInvocation = - countMethodInvocation > 1 && - hasFqnRefPart && - !isCapitalizedIdentifierWithoutTrailingComment; + countMethodInvocation > 1 && hasFqnRefPart && !isCapitalizedIdentifier; const shouldBreakBeforeMethodInvocations = shouldBreakBeforeFirstMethodInvocation || @@ -425,20 +420,6 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter { ); } - private handleStaticInvocations(fqnOrRefType: FqnOrRefTypeCtx | undefined) { - const lastFqnRefPartDot = this.lastFqnOrRefDot(fqnOrRefType); - const isCapitalizedIdentifier = this.isCapitalizedIdentifier(fqnOrRefType); - const isCapitalizedIdentifierWithoutTrailingComment = - isCapitalizedIdentifier && - (lastFqnRefPartDot === undefined || - !hasLeadingComments(lastFqnRefPartDot)); - - return { - isCapitalizedIdentifier, - isCapitalizedIdentifierWithoutTrailingComment - }; - } - primaryPrefix(ctx: PrimaryPrefixCtx, params: any) { if (ctx.This || ctx.Void) { return printTokenWithComments(this.getSingle(ctx) as IToken); @@ -468,47 +449,31 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter { const fqnOrRefTypePartFirst = this.visit(ctx.fqnOrRefTypePartFirst); const fqnOrRefTypePartRest = this.mapVisit(ctx.fqnOrRefTypePartRest); const dims = this.visit(ctx.dims); - const dots = ctx.Dot ? ctx.Dot : []; - const isMethodInvocation = ctx.Dot && ctx.Dot.length === 1; + const dots = ctx.Dot + ? ctx.Dot.map(dot => { + if (hasLeadingComments(dot)) { + return concat([softline, dot]); + } + return dot; + }) + : []; if ( - params !== undefined && - params.shouldBreakBeforeFirstMethodInvocation === true + params?.shouldBreakBeforeFirstMethodInvocation === true && + ctx.Dot !== undefined ) { - // when fqnOrRefType is a method call from an object - if (isMethodInvocation) { - return rejectAndConcat([ - indent( - rejectAndJoin(concat([softline, dots[0]]), [ - fqnOrRefTypePartFirst, - rejectAndJoinSeps(dots.slice(1), fqnOrRefTypePartRest), - dims - ]) - ) - ]); - // otherwise it is a fully qualified name but we need to exclude when it is just a method call - } else if (ctx.Dot) { - return indent( - rejectAndConcat([ - rejectAndJoinSeps(dots.slice(0, dots.length - 1), [ - fqnOrRefTypePartFirst, - ...fqnOrRefTypePartRest.slice(0, fqnOrRefTypePartRest.length - 1) - ]), - softline, - rejectAndConcat([ - dots[dots.length - 1], - fqnOrRefTypePartRest[fqnOrRefTypePartRest.length - 1] - ]), - dims - ]) - ); - } + dots[dots.length - 1] = concat([softline, ctx.Dot[ctx.Dot.length - 1]]); } - return rejectAndConcat([ - rejectAndJoinSeps(dots, [fqnOrRefTypePartFirst, ...fqnOrRefTypePartRest]), - dims - ]); + return indent( + rejectAndConcat([ + rejectAndJoinSeps(dots, [ + fqnOrRefTypePartFirst, + ...fqnOrRefTypePartRest + ]), + dims + ]) + ); } fqnOrRefTypePartFirst(ctx: FqnOrRefTypePartFirstCtx) { @@ -907,14 +872,6 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter { ); } - private lastFqnOrRefDot(fqnOrRefType: FqnOrRefTypeCtx | undefined) { - if (fqnOrRefType === undefined || fqnOrRefType.Dot === undefined) { - return undefined; - } - - return fqnOrRefType.Dot[fqnOrRefType.Dot.length - 1]; - } - private getPrimarySuffixes( ctx: PrimaryCtx, newExpression: NewExpressionCtx | undefined, diff --git a/packages/prettier-plugin-java/test/unit-test/member_chain/_input.java b/packages/prettier-plugin-java/test/unit-test/member_chain/_input.java index d0720d55..34a0134a 100644 --- a/packages/prettier-plugin-java/test/unit-test/member_chain/_input.java +++ b/packages/prettier-plugin-java/test/unit-test/member_chain/_input.java @@ -1,25 +1,83 @@ public class BreakLongFunctionCall { - public void doSomething() { - return new Object().something().more(); - } + public void doSomething() { + return new Object().something().more(); + } public void doSomethingNewWithComment() { - return new Object() + new Object() // comment .something().more(); + + new Object().something() + // comment + .more(); } public void doSomethingWithComment() { - return Object + Object + // comment + .something().more(); + + java.Object + // comment + .something().more(); + + java.averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.Object + // comment + .something().more(); + + java // comment + .averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.Object + .something().more(); + + java + .averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong + // comment + .Object + .something().more(); + + java.averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.Object .something().more(); + + Object.something() + // comment + .more(); + + averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.java + // comment + .util() + .java.java(); + + averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong + // comment + .java + .util() + .java.java(); + + averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.java + /* comment */ + .util() + .java.java(); + + averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.java/* comment */ + .util() + .java.java(); + + averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.java + /* comment */.util() + .java.java(); } public void doSomethingWithComment() { - return object + object // comment .something().more(); + + object.something() + // comment + .more(); } public void doSomethingNewWithComment() { @@ -40,28 +98,28 @@ public void doSomethingWithComment() { .something().more(); } - public void doSomethingLongNew() { - return something().more().and().that().as().well().but().not().something().something(); + public void doSomethingLongNew() { + return something().more().and().that().as().well().but().not().something().something(); } public void doSomethingLongWithArgument() { return something().more(firstArgument, secondArgument).and(firstArgument, secondArgument, thirdArgument, fourthArgument, fifthArgument); } - public void doSomethingLongNew2() { - return new Object().something().more().and().that().as().well().but().not().something(); - } + public void doSomethingLongNew2() { + return new Object().something().more().and().that().as().well().but().not().something(); + } - public void doSomethingLongStatic() { - return Object.something().more().and().that().as().well().but().not().something(); - } + public void doSomethingLongStatic() { + return Object.something().more().and().that().as().well().but().not().something(); + } - public void singleInvocationOnNewExpression() { - new Instance(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).invocation(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa); - } + public void singleInvocationOnNewExpression() { + new Instance(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).invocation(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa); + } - public void multipleInvocationsOnNewExpression() { - new Instance(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).invocation(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).andAnother(); - } + public void multipleInvocationsOnNewExpression() { + new Instance(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).invocation(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).andAnother(); + } } diff --git a/packages/prettier-plugin-java/test/unit-test/member_chain/_output.java b/packages/prettier-plugin-java/test/unit-test/member_chain/_output.java index 0ab139a7..a9d4abf4 100644 --- a/packages/prettier-plugin-java/test/unit-test/member_chain/_output.java +++ b/packages/prettier-plugin-java/test/unit-test/member_chain/_output.java @@ -5,24 +5,85 @@ public void doSomething() { } public void doSomethingNewWithComment() { - return new Object() + new Object() // comment .something() .more(); + + new Object() + .something() + // comment + .more(); } public void doSomethingWithComment() { - return Object + Object + // comment + .something() + .more(); + + java.Object // comment .something() .more(); + + java.averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.Object + // comment + .something() + .more(); + + java + // comment + .averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.Object.something() + .more(); + + java.averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong + // comment + .Object.something() + .more(); + + java.averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.Object.something() + .more(); + + Object.something() + // comment + .more(); + + averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.java + // comment + .util() + .java.java(); + + averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong + // comment + .java + .util() + .java.java(); + + averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.java + /* comment */ + .util() + .java.java(); + + averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.java/* comment */ + .util() + .java.java(); + + averyveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylong.java + /* comment */.util() + .java.java(); } public void doSomethingWithComment() { - return object + object // comment .something() .more(); + + object + .something() + // comment + .more(); } public void doSomethingNewWithComment() {