IDEA-68003 Java Formatter: Correct formatting of anonymous classes at method call arguments

1. Corrected formatting of anonymous class used as aligned method call arguments;
2. Corresponding tests are added;
3. Green code policy is applied at affected classes;
This commit is contained in:
Denis Zhdanov
2011-04-13 12:11:38 +04:00
parent 513edb767d
commit dbb27d2579
10 changed files with 372 additions and 95 deletions
@@ -26,6 +26,7 @@ import com.intellij.openapi.util.TextRange;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleSettings;
import com.intellij.psi.codeStyle.CommonCodeStyleSettings;
import com.intellij.psi.formatter.FormatterUtil;
import com.intellij.psi.formatter.common.AbstractBlock;
import com.intellij.psi.formatter.java.wrap.JavaWrapManager;
@@ -44,6 +45,7 @@ import org.jetbrains.annotations.Nullable;
import java.util.*;
import static com.intellij.psi.formatter.java.JavaFormatterUtil.isFirstAmongOthersAnonymousClassMethodCallArguments;
import static java.util.Arrays.asList;
public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlock, ReservedWrapsProvider {
@@ -258,7 +260,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
if (parentType == JavaElementType.MODIFIER_LIST) return Indent.getNoneIndent();
if (parentType == JspElementType.JSP_CODE_BLOCK) return Indent.getNormalIndent();
if (parentType == JspElementType.JSP_CLASS_LEVEL_DECLARATION_STATEMENT) return Indent.getNormalIndent();
if (parentType == ElementType.DUMMY_HOLDER) return Indent.getNoneIndent();
if (parentType == TokenType.DUMMY_HOLDER) return Indent.getNoneIndent();
if (parentType == JavaElementType.CLASS) return Indent.getNoneIndent();
if (parentType == JavaElementType.IF_STATEMENT) return Indent.getNoneIndent();
if (parentType == JavaElementType.TRY_STATEMENT) return Indent.getNoneIndent();
@@ -285,7 +287,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
}
protected static boolean isRBrace(final ASTNode child) {
return child.getElementType() == ElementType.RBRACE;
return child.getElementType() == JavaTokenType.RBRACE;
}
public Spacing getSpacing(Block child1, Block child2) {
@@ -363,7 +365,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
else if (nodeType == JavaElementType.CLASS || nodeType == JavaElementType.METHOD) {
return Alignment.createAlignment();
}
else if (nodeType == JavaElementType.MODIFIER_LIST) {
else if (nodeType == JavaElementType.MODIFIER_LIST || nodeType == JavaElementType.NEW_EXPRESSION) {
return myAlignment;
}
@@ -838,7 +840,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
// Here '@NotNull' has a 'class' node as a parent but we want to use field annotation setting value. Hence, we check if subsequent
// parsed info is valid.
for (ASTNode node = child.getTreeNext(); node != null; node = node.getTreeNext()) {
if (JavaTokenType.WHITE_SPACE == node.getElementType() || node instanceof PsiTypeElement) {
if (TokenType.WHITE_SPACE == node.getElementType() || node instanceof PsiTypeElement) {
continue;
}
if (node instanceof PsiErrorElement) {
@@ -856,7 +858,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
if (nodeType == JavaElementType.LOCAL_VARIABLE) {
return mySettings.VARIABLE_ANNOTATION_WRAP;
}
return CodeStyleSettings.DO_NOT_WRAP;
return CommonCodeStyleSettings.DO_NOT_WRAP;
}
@Nullable
@@ -922,7 +924,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
// There is a special case - comment block that is located at the very start of the line. We don't reformat such a blocks,
// hence, no alignment should be applied to them in order to avoid subsequent blocks aligned with the same alignment to
// be located at the left editor edge as well.
if (previous != null && previous.getElementType() == JavaTokenType.WHITE_SPACE && previous.getChars().length() > 0
if (previous != null && previous.getElementType() == TokenType.WHITE_SPACE && previous.getChars().length() > 0
&& previous.getChars().charAt(previous.getChars().length() - 1) == '\n') {
return null;
} else {
@@ -940,6 +942,12 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
return null;
}
else if (nodeType == JavaElementType.ANONYMOUS_CLASS && role == ChildRole.RBRACE
&& isFirstAmongOthersAnonymousClassMethodCallArguments(myNode))
{
return myAlignment;
}
else {
return defaultAlignment;
}
@@ -1015,11 +1023,11 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
private static WrapType getWrapType(final int wrap) {
switch (wrap) {
case CodeStyleSettings.WRAP_ALWAYS:
case CommonCodeStyleSettings.WRAP_ALWAYS:
return WrapType.ALWAYS;
case CodeStyleSettings.WRAP_AS_NEEDED:
case CommonCodeStyleSettings.WRAP_AS_NEEDED:
return WrapType.NORMAL;
case CodeStyleSettings.DO_NOT_WRAP:
case CommonCodeStyleSettings.DO_NOT_WRAP:
return WrapType.NONE;
default:
return WrapType.CHOP_DOWN_IF_LONG;
@@ -1040,12 +1048,12 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
private ASTNode processParenthesisBlock(final IElementType from,
final IElementType to, final List<Block> result, ASTNode child,
final WrappingStrategy wrappingStrategy, final boolean doAlign
) {
final WrappingStrategy wrappingStrategy, final boolean doAlign)
{
final Indent externalIndent = Indent.getNoneIndent();
final Indent internalIndent = Indent.getContinuationIndent(myIndentSettings.USE_RELATIVE_INDENTS);
final Indent internalIndentEnforcedToParent = Indent.getIndent(Indent.Type.CONTINUATION, myIndentSettings.USE_RELATIVE_INDENTS, true);
AlignmentStrategy alignmentStrategy = AlignmentStrategy.wrap(createAlignment(doAlign, null), ElementType.COMMA);
final Indent internalIndent = Indent.getContinuationWithoutFirstIndent(myIndentSettings.USE_RELATIVE_INDENTS);
final Indent internalIndentEnforcedToChildren = Indent.getIndent(Indent.Type.CONTINUATION, myIndentSettings.USE_RELATIVE_INDENTS, true);
AlignmentStrategy alignmentStrategy = AlignmentStrategy.wrap(createAlignment(doAlign, null), JavaTokenType.COMMA);
setChildIndent(internalIndent);
setChildAlignment(alignmentStrategy.getAlignment(null));
boolean methodParametersBlock = true;
@@ -1076,7 +1084,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
}
else {
final IElementType elementType = child.getElementType();
Indent indentToUse = shouldEnforceParentIndent(child) ? internalIndentEnforcedToParent : internalIndent;
Indent indentToUse = shouldEnforceIndentToChildren(child) ? internalIndentEnforcedToChildren : internalIndent;
processChild(result, child, alignmentStrategy.getAlignment(elementType), wrappingStrategy.getWrap(elementType), indentToUse);
if (to == null) {//process only one statement
return child;
@@ -1091,7 +1099,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
return prev;
}
private boolean shouldEnforceParentIndent(@NotNull ASTNode node) {
private boolean shouldEnforceIndentToChildren(@NotNull ASTNode node) {
// Don't enforce indent if given node is the last argument, i.e. prefer the code below
// void test() {
// foo("test", new Runnable() {
@@ -1129,7 +1137,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
// Enforce indent only if anonymous class instance expression doesn't start new line.
ASTNode prev = node.getTreePrev();
return prev == null || prev.getElementType() != JavaTokenType.WHITE_SPACE || !StringUtil.containsLineBreak(prev.getChars());
return prev == null || prev.getElementType() != TokenType.WHITE_SPACE || !StringUtil.containsLineBreak(prev.getChars());
}
@Nullable
@@ -1193,7 +1201,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
}
final int braceStyle = getBraceStyle();
return braceStyle == CodeStyleSettings.NEXT_LINE_SHIFTED ?
return braceStyle == CommonCodeStyleSettings.NEXT_LINE_SHIFTED ?
createNormalIndent(baseChildrenIndent - 1, enforceParentIndent)
: createNormalIndent(baseChildrenIndent, enforceParentIndent);
}
@@ -1202,16 +1210,16 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
return createNormalIndent(baseChildrenIndent, false);
}
protected static Indent createNormalIndent(final int baseChildrenIndent, boolean enforceParentIndent) {
protected static Indent createNormalIndent(final int baseChildrenIndent, boolean enforceIndentToChildren) {
if (baseChildrenIndent == 1) {
return Indent.getIndent(Indent.Type.NORMAL, false, enforceParentIndent);
return Indent.getIndent(Indent.Type.NORMAL, false, enforceIndentToChildren);
}
else if (baseChildrenIndent <= 0) {
return Indent.getNoneIndent();
}
else {
LOG.assertTrue(false);
return Indent.getIndent(Indent.Type.NORMAL, false, enforceParentIndent);
return Indent.getIndent(Indent.Type.NORMAL, false, enforceIndentToChildren);
}
}
@@ -1222,8 +1230,8 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
protected Indent getCodeBlockExternalIndent() {
final int braceStyle = getBraceStyle();
if (braceStyle == CodeStyleSettings.END_OF_LINE || braceStyle == CodeStyleSettings.NEXT_LINE ||
braceStyle == CodeStyleSettings.NEXT_LINE_IF_WRAPPED) {
if (braceStyle == CommonCodeStyleSettings.END_OF_LINE || braceStyle == CommonCodeStyleSettings.NEXT_LINE ||
braceStyle == CommonCodeStyleSettings.NEXT_LINE_IF_WRAPPED) {
return Indent.getNoneIndent();
}
else {
@@ -1236,9 +1244,9 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
if (!isAfterCodeBlock(newChildIndex)) {
return Indent.getNormalIndent();
}
else if (braceStyle == CodeStyleSettings.NEXT_LINE ||
braceStyle == CodeStyleSettings.NEXT_LINE_IF_WRAPPED ||
braceStyle == CodeStyleSettings.END_OF_LINE) {
else if (braceStyle == CommonCodeStyleSettings.NEXT_LINE ||
braceStyle == CommonCodeStyleSettings.NEXT_LINE_IF_WRAPPED ||
braceStyle == CommonCodeStyleSettings.END_OF_LINE) {
return Indent.getNoneIndent();
}
else {
@@ -1364,9 +1372,10 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
);
}
final boolean rBrace = isRBrace(child);
Indent childIndent = rBrace ? Indent.getNoneIndent() : getCodeBlockInternalIndent(childrenIndent);
Indent childIndent = rBrace ? Indent.getNoneIndent() : getCodeBlockInternalIndent(childrenIndent, true);
if (!rBrace && child.getElementType() == JavaElementType.CODE_BLOCK
&& (getBraceStyle() == CodeStyleSettings.NEXT_LINE_SHIFTED || getBraceStyle() == CodeStyleSettings.NEXT_LINE_SHIFTED2))
&& (getBraceStyle() == CommonCodeStyleSettings.NEXT_LINE_SHIFTED
|| getBraceStyle() == CommonCodeStyleSettings.NEXT_LINE_SHIFTED2))
{
childIndent = Indent.getNormalIndent();
}
@@ -18,12 +18,12 @@ package com.intellij.psi.formatter.java;
import com.intellij.formatting.*;
import com.intellij.lang.ASTNode;
import com.intellij.psi.JavaTokenType;
import com.intellij.psi.TokenType;
import com.intellij.psi.codeStyle.CodeStyleSettings;
import com.intellij.psi.formatter.FormatterUtil;
import com.intellij.psi.formatter.common.AbstractBlock;
import com.intellij.formatting.alignment.AlignmentStrategy;
import com.intellij.psi.impl.source.jsp.jspJava.JspClass;
import com.intellij.psi.impl.source.tree.ElementType;
import com.intellij.psi.impl.source.tree.JavaDocElementType;
import com.intellij.psi.impl.source.tree.JavaElementType;
import com.intellij.psi.impl.source.tree.StdTokenSets;
@@ -72,7 +72,7 @@ public class CodeBlockBlock extends AbstractJavaBlock {
continue;
}
ASTNode lastChildNode = node.getLastChildNode();
if (lastChildNode != null && lastChildNode.getElementType() == JavaTokenType.ERROR_ELEMENT) {
if (lastChildNode != null && lastChildNode.getElementType() == TokenType.ERROR_ELEMENT) {
Alignment alignmentToUse = alignment;
if (alignment == null) {
alignmentToUse = Alignment.createAlignment();
@@ -87,7 +87,7 @@ public class CodeBlockBlock extends AbstractJavaBlock {
}
private boolean isSwitchCodeBlock() {
return myNode.getTreeParent().getElementType() == ElementType.SWITCH_STATEMENT;
return myNode.getTreeParent().getElementType() == JavaElementType.SWITCH_STATEMENT;
}
protected List<Block> buildChildren() {
@@ -115,13 +115,13 @@ public class CodeBlockBlock extends AbstractJavaBlock {
final Indent indent = calcCurrentIndent(child, state);
state = calcNewState(child, state);
if (child.getElementType() == ElementType.SWITCH_LABEL_STATEMENT) {
if (child.getElementType() == JavaElementType.SWITCH_LABEL_STATEMENT) {
child = processCaseAndStatementAfter(result, child, childAlignment, childWrap, indent);
}
else if (myNode.getElementType() == ElementType.CLASS && child.getElementType() == ElementType.LBRACE) {
else if (myNode.getElementType() == JavaElementType.CLASS && child.getElementType() == JavaTokenType.LBRACE) {
child = composeCodeBlock(result, child, getCodeBlockExternalIndent(), myChildrenIndent, null);
}
else if (myNode.getElementType() == ElementType.CODE_BLOCK && child.getElementType() == ElementType.LBRACE
else if (myNode.getElementType() == JavaElementType.CODE_BLOCK && child.getElementType() == JavaTokenType.LBRACE
&& myNode.getTreeParent().getElementType() == JavaElementType.METHOD)
{
child = composeCodeBlock(result, child, indent, myChildrenIndent, childWrap);
@@ -146,14 +146,14 @@ public class CodeBlockBlock extends AbstractJavaBlock {
child = child.getTreeNext();
Indent childIndent = Indent.getNormalIndent();
while (child != null) {
if (child.getElementType() == ElementType.SWITCH_LABEL_STATEMENT || isRBrace(child)) {
if (child.getElementType() == JavaElementType.SWITCH_LABEL_STATEMENT || isRBrace(child)) {
result.add(createCaseSectionBlock(localResult, childAlignment, indent, childWrap));
return child.getTreePrev();
}
if (!FormatterUtil.containsWhiteSpacesOnly(child)) {
if (child.getElementType() == ElementType.BLOCK_STATEMENT) {
if (child.getElementType() == JavaElementType.BLOCK_STATEMENT) {
childIndent = Indent.getNoneIndent();
}
@@ -189,9 +189,9 @@ public class CodeBlockBlock extends AbstractJavaBlock {
}
}
if (prevElementType == ElementType.BLOCK_STATEMENT
|| prevElementType == ElementType.BREAK_STATEMENT
|| prevElementType == ElementType.RETURN_STATEMENT) {
if (prevElementType == JavaElementType.BLOCK_STATEMENT
|| prevElementType == JavaElementType.BREAK_STATEMENT
|| prevElementType == JavaElementType.RETURN_STATEMENT) {
return new ChildAttributes(Indent.getNoneIndent(), null);
}
else {
@@ -231,7 +231,7 @@ public class CodeBlockBlock extends AbstractJavaBlock {
}
private static boolean isLBrace(final ASTNode child) {
return child.getElementType() == ElementType.LBRACE;
return child.getElementType() == JavaTokenType.LBRACE;
}
private Indent calcCurrentIndent(final ASTNode child, final int state) {
@@ -241,7 +241,7 @@ public class CodeBlockBlock extends AbstractJavaBlock {
if (state == BEFORE_FIRST) return Indent.getNoneIndent();
if (child.getElementType() == ElementType.SWITCH_LABEL_STATEMENT) {
if (child.getElementType() == JavaElementType.SWITCH_LABEL_STATEMENT) {
return getCodeBlockInternalIndent(myChildrenIndent);
}
if (state == BEFORE_LBRACE) {
@@ -0,0 +1,176 @@
/*
* Copyright 2000-2011 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
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.impl.source.tree.ElementType;
import com.intellij.psi.impl.source.tree.JavaElementType;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
/**
* @author Denis Zhdanov
* @since 4/12/11 3:26 PM
*/
public class JavaFormatterUtil {
private JavaFormatterUtil() {
}
public static boolean isFirstMethodCallArgument(@NotNull ASTNode node) {
ASTNode firstArgCandidate = node;
ASTNode expressionList = node.getTreeParent();
if (expressionList == null) {
return false;
}
if (expressionList.getElementType() != JavaElementType.EXPRESSION_LIST
&& expressionList.getElementType() == JavaElementType.NEW_EXPRESSION)
{
firstArgCandidate = expressionList;
expressionList = expressionList.getTreeParent();
}
if (expressionList == null || expressionList.getElementType() != JavaElementType.EXPRESSION_LIST) {
return false;
}
ASTNode methodCallExpression = expressionList.getTreeParent();
if (methodCallExpression == null || methodCallExpression.getElementType() != JavaElementType.METHOD_CALL_EXPRESSION) {
return false;
}
ASTNode lbrace = expressionList.getFirstChildNode();
ASTNode firstArg = lbrace.getTreeNext();
if (firstArg != null && ElementType.WHITE_SPACE_BIT_SET.contains(firstArg.getElementType())) {
firstArg = firstArg.getTreeNext();
}
return firstArg == firstArgCandidate;
}
/**
* Allows to check if given node references anonymous class instance used as a method call argument. The most important thing
* is that that method call expression should have other anonymous classes as well.
* <p/>
* <b>Examples</b>
* <pre>
* test(new Runnable() { &lt;-- true is returned for this node
* public void run() {
* }
* },
* new Runnable() { &lt;-- false is returned for this node
* public void run() {
* }
* }
* );
*
* test(1234, "text", new Runnable() { &lt;-- true is returned for this node because there are no other anonymous
* public void run() { class objects at method call expression before it
* }
* },
* new Runnable() { &lt;-- false is returned for this node
* public void run() {
* }
* }
* );
*
* test(1234, "text", new Runnable() { &lt;-- false is returned for this node because there are no other anonymous
* public void run() { class objects at method call expression after it
* }
* });
* </pre>
*
* @param node node to process
* @return
*/
public static boolean isFirstAmongOthersAnonymousClassMethodCallArguments(@NotNull ASTNode node) {
ASTNode expressionList = node.getTreeParent();
ASTNode firstAnonymousClassCandidate = node;
if (expressionList == null) {
return false;
}
if (expressionList.getElementType() != JavaElementType.EXPRESSION_LIST
&& expressionList.getElementType() == JavaElementType.NEW_EXPRESSION)
{
firstAnonymousClassCandidate = expressionList;
expressionList = expressionList.getTreeParent();
}
if (expressionList == null || expressionList.getElementType() != JavaElementType.EXPRESSION_LIST) {
return false;
}
ASTNode methodCallExpression = expressionList.getTreeParent();
if (methodCallExpression == null || methodCallExpression.getElementType() != JavaElementType.METHOD_CALL_EXPRESSION) {
return false;
}
ASTNode lbrace = expressionList.getFirstChildNode();
boolean firstAnonymousClass = false;
for (ASTNode arg = lbrace.getTreeNext(); arg != null; arg = FormattingAstUtil.getNextNonWhiteSpaceNode(arg)) {
if (!isAnonymousClass(arg)) {
continue;
}
if (firstAnonymousClass) {
// Other anonymous class is found at the method call expression after the target one.
return true;
}
else if (arg != firstAnonymousClassCandidate) {
return false;
}
else {
firstAnonymousClass = true;
}
}
return false;
}
/**
* Allows to check if given expression list has given number of anonymous classes.
*
* @param count interested number of anonymous classes used at the given expression list
* @return <code>true</code> if given expression list contains given number of anonymous classes;
* <code>false</code> otherwise
*/
public static boolean hasAnonymousClassesArguments(@NotNull PsiExpressionList expressionList, int count) {
int found = 0;
for (PsiExpression expression : expressionList.getExpressions()) {
ASTNode node = expression.getNode();
if (isAnonymousClass(node)) {
found++;
}
if (found >= count) {
return true;
}
}
return false;
}
private static boolean isAnonymousClass(@Nullable final ASTNode node) {
if (node == null) {
return false;
}
ASTNode nodeToCheck = node;
if (node.getElementType() == JavaElementType.NEW_EXPRESSION) {
nodeToCheck = node.getLastChildNode();
}
return nodeToCheck != null && nodeToCheck.getElementType() == JavaElementType.ANONYMOUS_CLASS;
}
}
@@ -1100,8 +1100,13 @@ public class JavaSpacePropertyProcessor extends JavaElementVisitor {
createParenthSpace(mySettings.CALL_PARAMETERS_LPAREN_ON_NEXT_LINE, false);
}
else if (myRole2 == ChildRole.RPARENTH) {
createParenthSpace(mySettings.CALL_PARAMETERS_RPAREN_ON_NEXT_LINE,
myRole1 == ChildRole.COMMA || mySettings.SPACE_WITHIN_METHOD_CALL_PARENTHESES);
if (JavaFormatterUtil.hasAnonymousClassesArguments(list, 2)) {
myResult = Spacing.createSpacing(0, 0, 1, mySettings.KEEP_LINE_BREAKS, 0);
}
else {
createParenthSpace(mySettings.CALL_PARAMETERS_RPAREN_ON_NEXT_LINE,
myRole1 == ChildRole.COMMA || mySettings.SPACE_WITHIN_METHOD_CALL_PARENTHESES);
}
}
else if (myRole1 == ChildRole.LPARENTH) {
createParenthSpace(mySettings.CALL_PARAMETERS_LPAREN_ON_NEXT_LINE, mySettings.SPACE_WITHIN_METHOD_CALL_PARENTHESES);