Skip to content

Commit

Permalink
feat: avoid breaking on certain method chains and arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
jtkiesel committed Jan 9, 2024
1 parent 1f7f1d7 commit 56a1f93
Show file tree
Hide file tree
Showing 16 changed files with 310 additions and 138 deletions.
140 changes: 82 additions & 58 deletions packages/prettier-plugin-java/src/printers/expressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,11 @@ import forEach from "lodash/forEach";
import { builders } from "prettier/doc";
import { BaseCstPrettierPrinter } from "../base-cst-printer";
import { isAnnotationCstNode } from "../types/utils";
import { isArgumentListSingleLambda } from "../utils/expressions-utils";
import {
printSingleLambdaInvocation,
printArgumentListWithBraces
} from "../utils";
import { isArgumentListHuggable } from "../utils/expressions-utils";
import { printArgumentListWithBraces } from "../utils";
import { printTokenWithComments } from "./comments/format-comments";
import { handleCommentsBinaryExpression } from "./comments/handle-comments";
import { concat, dedent, group, indent } from "./prettier-builder";
import { concat, group, indent } from "./prettier-builder";
import {
binary,
findDeepElementInPartsArray,
Expand Down Expand Up @@ -324,10 +321,50 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter {
params?: { addParenthesisToWrapStatement?: boolean }
) {
const countMethodInvocation = isUniqueMethodInvocation(ctx.primarySuffix);
const newExpression =
ctx.primaryPrefix[0].children.newExpression?.[0].children;
const arrayCreationExpression =
newExpression?.arrayCreationExpression?.[0].children;
const classInstanceCreationExpression =
newExpression?.unqualifiedClassInstanceCreationExpression?.[0].children;
const isBreakableNewExpression =
countMethodInvocation <= 1 &&
[
arrayCreationExpression?.classOrInterfaceType?.[0].children.classType[0]
.children.typeArguments,
arrayCreationExpression?.arrayCreationExplicitInitSuffix?.[0].children
.arrayInitializer[0].children.variableInitializerList,
classInstanceCreationExpression?.classOrInterfaceTypeToInstantiate[0]
.children.typeArgumentsOrDiamond?.[0].children.typeArguments,
classInstanceCreationExpression?.argumentList
].some(breakablePart => breakablePart);
const fqnOrRefType =
ctx.primaryPrefix[0].children.fqnOrRefType?.[0].children;
const fqnOrRefTypeParts = [
fqnOrRefType?.fqnOrRefTypePartFirst[0],
...(fqnOrRefType?.fqnOrRefTypePartRest ?? [])
];
const nextToLastIdentifier =
fqnOrRefTypeParts[fqnOrRefTypeParts.length - 2]?.children
.fqnOrRefTypePartCommon[0].children.Identifier?.[0].image;
const firstMethodInvocation = ctx.primarySuffix
?.map(suffix => suffix.children.methodInvocationSuffix?.[0].children)
.find(methodInvocationSuffix => methodInvocationSuffix);
const isCapitalizedIdentifier =
nextToLastIdentifier && /^\p{Lu}/u.test(nextToLastIdentifier);
const shouldBreakBeforeFirstMethodInvocation =
countMethodInvocation > 1 &&
!(isCapitalizedIdentifier ?? true) &&
firstMethodInvocation !== undefined;
const shouldBreakBeforeMethodInvocations =
shouldBreakBeforeFirstMethodInvocation ||
countMethodInvocation > 2 ||
(countMethodInvocation > 1 && newExpression) ||
!firstMethodInvocation?.argumentList;

const primaryPrefix = this.visit(ctx.primaryPrefix, {
...params,
shouldBreakBeforeFirstMethodInvocation: countMethodInvocation > 1
shouldBreakBeforeFirstMethodInvocation
});

const suffixes = [];
Expand All @@ -347,37 +384,17 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter {
}

if (
ctx.primarySuffix[0].children.Dot !== undefined &&
ctx.primaryPrefix[0].children.newExpression !== undefined
newExpression &&
!isBreakableNewExpression &&
ctx.primarySuffix[0].children.Dot !== undefined
) {
suffixes.push(softline);
}
suffixes.push(
this.visit(ctx.primarySuffix[0], {
shouldDedent:
// dedent when simple method invocation
countMethodInvocation !== 1 &&
// dedent when (chain) method invocation
ctx.primaryPrefix[0] &&
ctx.primaryPrefix[0].children.fqnOrRefType &&
!(
ctx.primaryPrefix[0].children.fqnOrRefType[0].children.Dot !==
undefined
) &&
// indent when lambdaExpression
ctx.primarySuffix[0].children.methodInvocationSuffix &&
ctx.primarySuffix[0].children.methodInvocationSuffix[0].children
.argumentList &&
ctx.primarySuffix[0].children.methodInvocationSuffix[0].children
.argumentList[0].children.expression &&
ctx.primarySuffix[0].children.methodInvocationSuffix[0].children
.argumentList[0].children.expression[0].children
.lambdaExpression === undefined
})
);
suffixes.push(this.visit(ctx.primarySuffix[0]));

for (let i = 1; i < ctx.primarySuffix.length; i++) {
if (
shouldBreakBeforeMethodInvocations &&
ctx.primarySuffix[i].children.Dot !== undefined &&
ctx.primarySuffix[i - 1].children.methodInvocationSuffix !== undefined
) {
Expand All @@ -386,10 +403,7 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter {
suffixes.push(this.visit(ctx.primarySuffix[i]));
}

if (
countMethodInvocation === 1 &&
ctx.primaryPrefix[0].children.newExpression === undefined
) {
if (!newExpression && countMethodInvocation === 1) {
return group(
rejectAndConcat([
primaryPrefix,
Expand All @@ -400,8 +414,24 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter {
}
}

const methodInvocation =
ctx.primarySuffix?.[0].children.methodInvocationSuffix;
const isMethodInvocationWithArguments =
methodInvocation?.[0].children.argumentList !== undefined;
const isUnqualifiedMethodInvocation =
methodInvocation !== undefined &&
!ctx.primaryPrefix[0].children.fqnOrRefType?.[0].children.Dot;
return group(
rejectAndConcat([primaryPrefix, indent(rejectAndConcat(suffixes))])
rejectAndConcat([
primaryPrefix,
isCapitalizedIdentifier || isUnqualifiedMethodInvocation
? suffixes.shift()
: "",
!isBreakableNewExpression &&
(shouldBreakBeforeMethodInvocations || !isMethodInvocationWithArguments)
? indent(concat(suffixes))
: concat(suffixes)
])
);
}

Expand Down Expand Up @@ -625,25 +655,12 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter {
}

methodInvocationSuffix(ctx: MethodInvocationSuffixCtx, params: any) {
const isSingleLambda = isArgumentListSingleLambda(ctx.argumentList);
if (isSingleLambda) {
return printSingleLambdaInvocation.call(
this,
ctx.argumentList,
ctx.RBrace[0],
ctx.LBrace[0]
);
}

const argumentList = this.visit(ctx.argumentList);

if (params && params.shouldDedent) {
return dedent(
putIntoBraces(argumentList, softline, ctx.LBrace[0], ctx.RBrace[0])
);
}

return putIntoBraces(argumentList, softline, ctx.LBrace[0], ctx.RBrace[0]);
return printArgumentListWithBraces.call(
this,
ctx.argumentList,
ctx.RBrace[0],
ctx.LBrace[0]
);
}

argumentList(
Expand All @@ -654,8 +671,15 @@ export class ExpressionsPrettierVisitor extends BaseCstPrettierPrinter {
}
) {
const expressions = this.mapVisit(ctx.expression, params);
const commas = ctx.Comma ? ctx.Comma.map(elt => concat([elt, line])) : [];
return rejectAndJoinSeps(commas, expressions);

const lastArgument = expressions.pop();
const commas = ctx.Comma?.map(comma => concat([comma, line]));
const otherArguments = rejectAndJoinSeps(commas, expressions);

return rejectAndConcat([
isArgumentListHuggable(ctx) ? group(otherArguments) : otherArguments,
lastArgument
]);
}

arrayCreationExpression(ctx: ArrayCreationExpressionCtx) {
Expand Down
43 changes: 9 additions & 34 deletions packages/prettier-plugin-java/src/utils/expressions-utils.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,12 @@
import { ArgumentListCstNode } from "java-parser";
import { ArgumentListCtx } from "java-parser";

export function isArgumentListSingleLambda(
argumentList: ArgumentListCstNode[] | undefined
) {
if (argumentList === undefined) {
return false;
}

const args = argumentList[0].children.expression;
if (args.length !== 1) {
return false;
}

const argument = args[0];
return argument.children.lambdaExpression !== undefined;
}

export const isSingleArgumentLambdaExpressionWithBlock = (
argumentList: ArgumentListCstNode[] | undefined
) => {
if (argumentList === undefined) {
return false;
}

const args = argumentList[0].children.expression;
if (args.length !== 1) {
return false;
}

const argument = args[0];
export function isArgumentListHuggable(argumentList: ArgumentListCtx) {
const expressions = argumentList.expression;
return (
argument.children.lambdaExpression !== undefined &&
argument.children.lambdaExpression[0].children.lambdaBody[0].children
.block !== undefined
expressions.filter(expression => expression.children.lambdaExpression)
.length === 1 &&
(expressions.length === 1 ||
expressions[expressions.length - 1].children.lambdaExpression?.[0]
.children.lambdaBody[0].children.block !== undefined)
);
};
}
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
import { ArgumentListCstNode, IToken } from "java-parser";
import { builders } from "prettier/doc";
import { isArgumentListSingleLambda } from "./expressions-utils";
import { isArgumentListHuggable } from "./expressions-utils";
import { putIntoBraces } from "../printers/printer-utils";
import printSingleLambdaInvocation from "./printSingleLambdaInvocation";

const { softline } = builders;

export default function printArgumentListWithBraces(
argumentListCtx: ArgumentListCstNode[] | undefined,
argumentListNodes: ArgumentListCstNode[] | undefined,
rBrace: IToken,
lBrace: IToken
) {
const isSingleLambda = isArgumentListSingleLambda(argumentListCtx);
if (isSingleLambda) {
const argumentListCtx = argumentListNodes?.[0].children;
if (argumentListCtx && isArgumentListHuggable(argumentListCtx)) {
return printSingleLambdaInvocation.call(
this,
argumentListCtx,
argumentListNodes,
rBrace,
lBrace
);
}

const argumentList = this.visit(argumentListCtx, {
const argumentList = this.visit(argumentListNodes, {
isInsideMethodInvocationSuffix: true
});
return putIntoBraces(argumentList, softline, lBrace, rBrace);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,31 +1,31 @@
import { ArgumentListCstNode, IToken } from "java-parser";
import { builders } from "prettier/doc";
import { isSingleArgumentLambdaExpressionWithBlock } from "./expressions-utils";
import { printTokenWithComments } from "../printers/comments/format-comments";
import { concat, dedent, indent } from "../printers/prettier-builder";
import { putIntoBraces } from "../printers/printer-utils";

const { softline, ifBreak } = builders;

export default function printSingleLambdaInvocation(
argumentListCtx: ArgumentListCstNode[] | undefined,
argumentListNodes: ArgumentListCstNode[],
rBrace: IToken,
lBrace: IToken
) {
const lambdaParametersGroupId = Symbol("lambdaParameters");
const argumentList = this.visit(argumentListCtx, {
const argumentList = this.visit(argumentListNodes, {
lambdaParametersGroupId,
isInsideMethodInvocationSuffix: true
});

const formattedRBrace = isSingleArgumentLambdaExpressionWithBlock(
argumentListCtx
)
? ifBreak(
indent(concat([softline, rBrace])),
printTokenWithComments(rBrace),
{ groupId: lambdaParametersGroupId }
)
: indent(concat([softline, rBrace]));
const expressions = argumentListNodes[0].children.expression;
const isLastArgumentLambdaWithBlock =
expressions[expressions.length - 1].children.lambdaExpression?.[0].children
.lambdaBody[0].children.block !== undefined;
const breakableRBrace = indent(concat([softline, rBrace]));
const formattedRBrace = isLastArgumentLambdaWithBlock
? ifBreak(breakableRBrace, printTokenWithComments(rBrace), {
groupId: lambdaParametersGroupId
})
: breakableRBrace;
return dedent(putIntoBraces(argumentList, "", lBrace, formattedRBrace));
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
class Array {
boolean[] skip = new boolean[candidates.length];

Class<?> aaaaaaaaaaaaaaaa = new Aaaaaaaaaaaaaaaa<Bbbbbbbbbbbbbbbb>[1].getClass();
Class<?> aaaaaaaaaaaaaaaa = new Aaaaaaaaaaaaaaaa[1111111111111111111].getClass();
Class<?> aaaaaaaaaaaaaaaa = new Aaaaaaaaaaaaaaaa[]{ new Aaaaaaaaaaaaaaaa() }.getClass();
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
class Array {

boolean[] skip = new boolean[candidates.length];

Class<?> aaaaaaaaaaaaaaaa = new Aaaaaaaaaaaaaaaa<
Bbbbbbbbbbbbbbbb
>[1].getClass();
Class<?> aaaaaaaaaaaaaaaa = new Aaaaaaaaaaaaaaaa[1111111111111111111]
.getClass();
Class<?> aaaaaaaaaaaaaaaa = new Aaaaaaaaaaaaaaaa[] {
new Aaaaaaaaaaaaaaaa(),
}.getClass();
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@ public boolean binaryOperationWithComments() {
}

public void method() {
new Foo(stuff, thing, "auaaaaaaaaa some very long stuff", "some more")
.bar(10);
foo(stuff, thing, "some very longuuuuuuuuuuuuuu stuff", "some more")
.bar(10);
new Foo(stuff, thing, "auaaaaaaaaa some very long stuff", "some more").bar(
10
);
foo(stuff, thing, "some very longuuuuuuuuuuuuuu stuff", "some more").bar(
10
);
}

public void binaryExpressionWithCast() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -866,8 +866,9 @@ public void getAllCountries() throws Exception {
jsonPath("$.[*].display").value(hasItem(DEFAULT_DISPLAY.booleanValue()))
)
.andExpect(
jsonPath("$.[*].internationalDialingCode")
.value(hasItem(DEFAULT_INTERNATIONAL_DIALING_CODE.toString()))
jsonPath("$.[*].internationalDialingCode").value(
hasItem(DEFAULT_INTERNATIONAL_DIALING_CODE.toString())
)
);
}
}
Loading

0 comments on commit 56a1f93

Please sign in to comment.