From af674d6ad91a94f86d748cc6b93b39e9ed4abe77 Mon Sep 17 00:00:00 2001 From: Yaroslav Lepenkin Date: Mon, 20 Jan 2014 19:52:33 +0400 Subject: [PATCH] IDEA-118374 Formatter chop down method arguments when long is not working correctly [CR-IC-4041] --- .../psi/formatter/java/AbstractJavaBlock.java | 24 +++---- .../psi/formatter/java/JavaFormatterUtil.java | 32 +++++++--- .../java/JavaSpacePropertyProcessor.java | 3 +- .../formatter/java/JavaFormatterWrapTest.java | 62 +++++++++++++++++++ 4 files changed, 99 insertions(+), 22 deletions(-) diff --git a/java/java-impl/src/com/intellij/psi/formatter/java/AbstractJavaBlock.java b/java/java-impl/src/com/intellij/psi/formatter/java/AbstractJavaBlock.java index 87f932309963..38ef6b8fbc27 100644 --- a/java/java-impl/src/com/intellij/psi/formatter/java/AbstractJavaBlock.java +++ b/java/java-impl/src/com/intellij/psi/formatter/java/AbstractJavaBlock.java @@ -31,7 +31,7 @@ import com.intellij.psi.formatter.common.AbstractBlock; import com.intellij.psi.formatter.java.wrap.JavaWrapManager; import com.intellij.psi.formatter.java.wrap.ReservedWrapsProvider; import com.intellij.psi.impl.source.SourceTreeToPsiMap; -import com.intellij.psi.impl.source.codeStyle.ShiftIndentInsideHelper; +import com.intellij.psi.impl.source.codeStyle.*; import com.intellij.psi.impl.source.tree.*; import com.intellij.psi.impl.source.tree.injected.InjectedLanguageUtil; import com.intellij.psi.impl.source.tree.java.ClassElement; @@ -457,7 +457,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo if (childType == JavaElementType.METHOD_CALL_EXPRESSION) { result.add(createMethodCallExpressionBlock(child, arrangeChildWrap(child, defaultWrap), - arrangeChildAlignment(child, alignmentStrategy))); + arrangeChildAlignment(child, alignmentStrategy), childIndent)); } else { IElementType nodeType = myNode.getElementType(); @@ -710,10 +710,10 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo } @NotNull - private Block createMethodCallExpressionBlock(@NotNull final ASTNode node, final Wrap blockWrap, final Alignment alignment) { + private Block createMethodCallExpressionBlock(@NotNull ASTNode node, Wrap blockWrap, Alignment alignment, Indent indent) { final ArrayList nodes = new ArrayList(); collectNodes(nodes, node); - return new ChainMethodCallsBlockBuilder(alignment, blockWrap).build(nodes); + return new ChainMethodCallsBlockBuilder(alignment, blockWrap, indent).build(nodes); } @NotNull @@ -1028,6 +1028,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo ASTNode prev = child; boolean afterAnonymousClass = false; + final boolean enforceIndent = shouldEnforceIndentToChildren(); while (child != null) { isAfterIncomplete = isAfterIncomplete || child.getElementType() == TokenType.ERROR_ELEMENT || child.getElementType() == JavaElementType.EMPTY_EXPRESSION; @@ -1044,7 +1045,6 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo } else { final IElementType elementType = child.getElementType(); - final boolean enforceIndent = shouldEnforceIndentToChildren(child); Indent indentToUse = enforceIndent ? internalIndentEnforcedToChildren : internalIndent; AlignmentStrategy alignmentStrategyToUse = canUseAnonymousClassAlignment(child) ? anonymousClassStrategy : alignmentStrategy; processChild(result, child, alignmentStrategyToUse.getAlignment(elementType), wrappingStrategy.getWrap(elementType), indentToUse); @@ -1105,7 +1105,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo return true; } - private boolean shouldEnforceIndentToChildren(@NotNull ASTNode node) { + private boolean shouldEnforceIndentToChildren() { if (myNode.getElementType() != JavaElementType.EXPRESSION_LIST) { return false; } @@ -1114,9 +1114,9 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo return false; } - PsiExpressionList methodParamsList = (PsiExpressionList)myNode.getPsi(); - return JavaFormatterUtil.hasMultilineArguments(methodParamsList) - && JavaFormatterUtil.isMultilineExceptArguments(methodParamsList); + PsiExpression[] arguments = ((PsiExpressionList)myNode.getPsi()).getExpressions(); + return (JavaFormatterUtil.hasMultilineArguments(arguments) || JavaFormatterUtil.canHaveMultilineArgumentsAfterWrap(arguments, mySettings)) + && (JavaFormatterUtil.isMultilineExceptArguments(arguments) || JavaFormatterUtil.canBeMultilineExceptArgumentsAfterWrap(arguments, mySettings)); } private static boolean isAnonymousClass(@Nullable ASTNode node) { @@ -1432,13 +1432,15 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo private class ChainMethodCallsBlockBuilder { private Wrap blockWrap; private Alignment blockAlignment; + private Indent blockIndent; private Wrap myWrap; private Alignment myChainedCallsAlignment; - public ChainMethodCallsBlockBuilder(Alignment alignment, Wrap wrap) { + public ChainMethodCallsBlockBuilder(Alignment alignment, Wrap wrap, Indent indent) { blockWrap = wrap; blockAlignment = alignment; + blockIndent = indent; } public Block build(List nodes) { @@ -1446,8 +1448,8 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo myChainedCallsAlignment = getNewAlignment(); List blocks = buildBlocksFrom(nodes); - Indent indent = Indent.getContinuationWithoutFirstIndent(myIndentSettings.USE_RELATIVE_INDENTS); + Indent indent = blockIndent != null ? blockIndent : Indent.getContinuationWithoutFirstIndent(myIndentSettings.USE_RELATIVE_INDENTS); return new SyntheticCodeBlock(blocks, blockAlignment, mySettings, indent, blockWrap); } diff --git a/java/java-impl/src/com/intellij/psi/formatter/java/JavaFormatterUtil.java b/java/java-impl/src/com/intellij/psi/formatter/java/JavaFormatterUtil.java index 3f63eda2fb11..5c1b5b7e84d4 100644 --- a/java/java-impl/src/com/intellij/psi/formatter/java/JavaFormatterUtil.java +++ b/java/java-impl/src/com/intellij/psi/formatter/java/JavaFormatterUtil.java @@ -16,10 +16,8 @@ package com.intellij.psi.formatter.java; import com.intellij.lang.ASTNode; -import com.intellij.psi.PsiExpression; -import com.intellij.psi.PsiExpressionList; -import com.intellij.psi.PsiPolyadicExpression; -import com.intellij.psi.PsiWhiteSpace; +import com.intellij.psi.*; +import com.intellij.psi.codeStyle.CommonCodeStyleSettings; import com.intellij.psi.impl.source.tree.JavaElementType; import com.intellij.psi.tree.IElementType; import org.jetbrains.annotations.NotNull; @@ -75,10 +73,7 @@ public class JavaFormatterUtil { return expression1.getOperationTokenType() == expression2.getOperationTokenType(); } - - public static boolean hasMultilineArguments(@NotNull PsiExpressionList list) { - PsiExpression[] arguments = list.getExpressions(); - + public static boolean hasMultilineArguments(@NotNull PsiExpression[] arguments) { for (PsiExpression argument: arguments) { ASTNode node = argument.getNode(); if (node.textContains('\n')) @@ -88,9 +83,21 @@ public class JavaFormatterUtil { return false; } - public static boolean isMultilineExceptArguments(@NotNull PsiExpressionList list) { - PsiExpression[] arguments = list.getExpressions(); + public static boolean canHaveMultilineArgumentsAfterWrap(@NotNull PsiExpression[] arguments, @NotNull CommonCodeStyleSettings settings) { + for (PsiExpression argument: arguments) { + ASTNode node = argument.getNode(); + if (node instanceof PsiMethodCallExpression) { + if (settings.CALL_PARAMETERS_LPAREN_ON_NEXT_LINE || settings.CALL_PARAMETERS_RPAREN_ON_NEXT_LINE) { + if (((PsiMethodCallExpression)node).getArgumentList().getExpressions().length > 0) return true; + } + } + } + + return false; + } + + public static boolean isMultilineExceptArguments(@NotNull PsiExpression[] arguments) { for (PsiExpression argument : arguments) { ASTNode beforeArgument = argument.getNode().getTreePrev(); if (isWhiteSpaceWithLineFeed(beforeArgument)) @@ -102,6 +109,11 @@ public class JavaFormatterUtil { return isWhiteSpaceWithLineFeed(afterLastArgument); } + public static boolean canBeMultilineExceptArgumentsAfterWrap(@NotNull PsiExpression[] arguments, @NotNull CommonCodeStyleSettings settings) { + return arguments.length > 0 + && (settings.CALL_PARAMETERS_LPAREN_ON_NEXT_LINE || settings.CALL_PARAMETERS_RPAREN_ON_NEXT_LINE); + } + private static boolean isWhiteSpaceWithLineFeed(@NotNull ASTNode node) { return node instanceof PsiWhiteSpace && node.textContains('\n'); diff --git a/java/java-impl/src/com/intellij/psi/formatter/java/JavaSpacePropertyProcessor.java b/java/java-impl/src/com/intellij/psi/formatter/java/JavaSpacePropertyProcessor.java index 1d64cb909506..244ad4357e4c 100644 --- a/java/java-impl/src/com/intellij/psi/formatter/java/JavaSpacePropertyProcessor.java +++ b/java/java-impl/src/com/intellij/psi/formatter/java/JavaSpacePropertyProcessor.java @@ -1151,7 +1151,8 @@ public class JavaSpacePropertyProcessor extends JavaElementVisitor { createParenthSpace(mySettings.CALL_PARAMETERS_LPAREN_ON_NEXT_LINE, mySettings.SPACE_WITHIN_EMPTY_METHOD_CALL_PARENTHESES); } else if (myRole2 == ChildRole.RPARENTH) { - if (JavaFormatterUtil.hasMultilineArguments(list) && JavaFormatterUtil.isMultilineExceptArguments(list)) { + PsiExpression[] arguments = list.getExpressions(); + if (JavaFormatterUtil.hasMultilineArguments(arguments) && JavaFormatterUtil.isMultilineExceptArguments(arguments)) { myResult = Spacing.createSpacing(0, 0, 1, mySettings.KEEP_LINE_BREAKS, 0); } else { diff --git a/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavaFormatterWrapTest.java b/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavaFormatterWrapTest.java index c5853deddbec..5e545ecb8be6 100644 --- a/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavaFormatterWrapTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavaFormatterWrapTest.java @@ -331,4 +331,66 @@ public class JavaFormatterWrapTest extends AbstractJavaFormatterTest { "int j = 2;"; doMethodTest(text, text); } + + public void testEnforceIndent_MethodCallParamWrap() throws Exception { + getSettings().WRAP_LONG_LINES = true; + getSettings().getRootSettings().RIGHT_MARGIN = 140; + getSettings().PREFER_PARAMETERS_WRAP = true; + getSettings().CALL_PARAMETERS_WRAP = CommonCodeStyleSettings.WRAP_ON_EVERY_ITEM; + + String before = "processingEnv.getMessenger().printMessage(Diagnostic.Kind.ERROR, String.format(\"Could not process annotations: %s%n%s\", e.toString(), writer.toString()));"; + + String afterFirstReformat = "processingEnv.getMessenger().printMessage(\n" + + " Diagnostic.Kind.ERROR, String.format(\n" + + " \"Could not process annotations: %s%n%s\",\n" + + " e.toString(),\n" + + " writer.toString()\n" + + ")\n" + + ");"; + + String after = "processingEnv.getMessenger().printMessage(\n" + + " Diagnostic.Kind.ERROR, String.format(\n" + + " \"Could not process annotations: %s%n%s\",\n" + + " e.toString(),\n" + + " writer.toString()\n" + + " )\n" + + ");"; + + doMethodTest(afterFirstReformat, after); + + getSettings().CALL_PARAMETERS_RPAREN_ON_NEXT_LINE = true; + getSettings().CALL_PARAMETERS_LPAREN_ON_NEXT_LINE = true; + doMethodTest(before, after); + + before = "processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, call(\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\"));\n"; + after = "processingEnv.getMessager().printMessage(\n" + + " Diagnostic.Kind.ERROR, call(\n" + + " \"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\"\n" + + " )\n" + + ");\n"; + + doMethodTest(before, after); + } + + public void testDoNotWrap_MethodsWithMethodCallAsParameters() throws Exception { + getSettings().WRAP_LONG_LINES = true; + getSettings().getRootSettings().RIGHT_MARGIN = 140; + getSettings().PREFER_PARAMETERS_WRAP = true; + getSettings().CALL_PARAMETERS_WRAP = CommonCodeStyleSettings.WRAP_ON_EVERY_ITEM; + getSettings().CALL_PARAMETERS_RPAREN_ON_NEXT_LINE = true; + getSettings().CALL_PARAMETERS_LPAREN_ON_NEXT_LINE = true; + + String before = " processingEnv.getMessenger().printMessage(Diagnostic.Kind.ERROR, getMessage());"; + String after = "processingEnv.getMessenger().printMessage(Diagnostic.Kind.ERROR, getMessage());"; + + doMethodTest(before, after); + + before = " processingEnv.getMessenger().printMessage(Diagnostic.Kind.ERROR, getMessage(loooooooooooooooooongParamName));"; + after = "processingEnv.getMessenger().printMessage(Diagnostic.Kind.ERROR, getMessage(loooooooooooooooooongParamName));"; + + doMethodTest(before, after); + } + + + }