From 8246a23c37f24a707e45f592b56a4acf04f08942 Mon Sep 17 00:00:00 2001 From: Clement Dessoude Date: Mon, 20 Jan 2025 21:34:37 +0100 Subject: [PATCH 1/3] fix: handle edge cases with comments in fqn parts --- .../src/printers/expressions.ts | 75 ++++++-------- .../test/unit-test/member_chain/_input.java | 98 +++++++++++++++---- .../test/unit-test/member_chain/_output.java | 67 ++++++++++++- 3 files changed, 173 insertions(+), 67 deletions(-) diff --git a/packages/prettier-plugin-java/src/printers/expressions.ts b/packages/prettier-plugin-java/src/printers/expressions.ts index 68b7ea30..f056f576 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,18 +449,25 @@ 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 dots = ctx.Dot + ? ctx.Dot.map(dot => { + if (hasLeadingComments(dot)) { + return concat([softline, dot]); + } + return dot; + }) + : []; const isMethodInvocation = ctx.Dot && ctx.Dot.length === 1; - if ( - params !== undefined && - params.shouldBreakBeforeFirstMethodInvocation === true - ) { + if (params?.shouldBreakBeforeFirstMethodInvocation === true) { // when fqnOrRefType is a method call from an object if (isMethodInvocation) { + const separator = hasLeadingComments(ctx.Dot![0]) + ? dots[0] + : concat([softline, dots[0]]); return rejectAndConcat([ indent( - rejectAndJoin(concat([softline, dots[0]]), [ + rejectAndJoin(separator, [ fqnOrRefTypePartFirst, rejectAndJoinSeps(dots.slice(1), fqnOrRefTypePartRest), dims @@ -488,27 +476,34 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter { ]); // otherwise it is a fully qualified name but we need to exclude when it is just a method call } else if (ctx.Dot) { + const lastDot = ctx.Dot[ctx.Dot.length - 1]; + const separator = hasLeadingComments(lastDot) + ? dots[dots.length - 1] + : concat([softline, lastDot]); + 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] - ]), + separator, + fqnOrRefTypePartRest[fqnOrRefTypePartRest.length - 1], dims ]) ); } } - return rejectAndConcat([ - rejectAndJoinSeps(dots, [fqnOrRefTypePartFirst, ...fqnOrRefTypePartRest]), - dims - ]); + return indent( + rejectAndConcat([ + rejectAndJoinSeps(dots, [ + fqnOrRefTypePartFirst, + ...fqnOrRefTypePartRest + ]), + dims + ]) + ); } fqnOrRefTypePartFirst(ctx: FqnOrRefTypePartFirstCtx) { @@ -907,14 +902,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() { From 2a1c78e3a71c2029b20162e9308303bef2be20af Mon Sep 17 00:00:00 2001 From: Clement Dessoude Date: Mon, 20 Jan 2025 21:42:57 +0100 Subject: [PATCH 2/3] fix: do not add dot before dims in fqn parts --- packages/prettier-plugin-java/src/printers/expressions.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/prettier-plugin-java/src/printers/expressions.ts b/packages/prettier-plugin-java/src/printers/expressions.ts index f056f576..afc5554d 100644 --- a/packages/prettier-plugin-java/src/printers/expressions.ts +++ b/packages/prettier-plugin-java/src/printers/expressions.ts @@ -467,8 +467,9 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter { : concat([softline, dots[0]]); return rejectAndConcat([ indent( - rejectAndJoin(separator, [ + rejectAndConcat([ fqnOrRefTypePartFirst, + separator, rejectAndJoinSeps(dots.slice(1), fqnOrRefTypePartRest), dims ]) From 4d366ee9e8ada82681f10847d8828c2da2ae93b1 Mon Sep 17 00:00:00 2001 From: Clement Dessoude Date: Mon, 20 Jan 2025 21:49:14 +0100 Subject: [PATCH 3/3] refactor: simplify logic for printing fqn parts --- .../src/printers/expressions.ts | 43 +++---------------- 1 file changed, 6 insertions(+), 37 deletions(-) diff --git a/packages/prettier-plugin-java/src/printers/expressions.ts b/packages/prettier-plugin-java/src/printers/expressions.ts index afc5554d..6bec3e23 100644 --- a/packages/prettier-plugin-java/src/printers/expressions.ts +++ b/packages/prettier-plugin-java/src/printers/expressions.ts @@ -457,43 +457,12 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter { return dot; }) : []; - const isMethodInvocation = ctx.Dot && ctx.Dot.length === 1; - - if (params?.shouldBreakBeforeFirstMethodInvocation === true) { - // when fqnOrRefType is a method call from an object - if (isMethodInvocation) { - const separator = hasLeadingComments(ctx.Dot![0]) - ? dots[0] - : concat([softline, dots[0]]); - return rejectAndConcat([ - indent( - rejectAndConcat([ - fqnOrRefTypePartFirst, - separator, - 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) { - const lastDot = ctx.Dot[ctx.Dot.length - 1]; - const separator = hasLeadingComments(lastDot) - ? dots[dots.length - 1] - : concat([softline, lastDot]); - - return indent( - rejectAndConcat([ - rejectAndJoinSeps(dots.slice(0, dots.length - 1), [ - fqnOrRefTypePartFirst, - ...fqnOrRefTypePartRest.slice(0, fqnOrRefTypePartRest.length - 1) - ]), - separator, - fqnOrRefTypePartRest[fqnOrRefTypePartRest.length - 1], - dims - ]) - ); - } + + if ( + params?.shouldBreakBeforeFirstMethodInvocation === true && + ctx.Dot !== undefined + ) { + dots[dots.length - 1] = concat([softline, ctx.Dot[ctx.Dot.length - 1]]); } return indent(