IDEA-65987 Java Formatter: Correct anonymous classes instances as method arguments processing

1. 'Enforce parent indent' option is added to indent construction API;
2. Corresponding support is added to formatter's core;
3. Corresponding support is added to java blocks construction algorithm;
4. Corresponding test is added;
This commit is contained in:
Denis Zhdanov
2011-03-01 08:24:42 +03:00
parent 01629855cc
commit 4b851fb856
8 changed files with 227 additions and 68 deletions
@@ -23,6 +23,7 @@ import com.intellij.lang.ASTNode;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.fileTypes.StdFileTypes;
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.formatter.FormatterUtil;
@@ -1023,6 +1024,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
) {
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);
setChildIndent(internalIndent);
setChildAlignment(alignmentStrategy.getAlignment(null));
@@ -1054,7 +1056,8 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
}
else {
final IElementType elementType = child.getElementType();
processChild(result, child, alignmentStrategy.getAlignment(elementType), wrappingStrategy.getWrap(elementType), internalIndent);
Indent indentToUse = shouldEnforceParentIndent(child) ? internalIndentEnforcedToParent : internalIndent;
processChild(result, child, alignmentStrategy.getAlignment(elementType), wrappingStrategy.getWrap(elementType), indentToUse);
if (to == null) {//process only one statement
return child;
}
@@ -1068,11 +1071,52 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
return prev;
}
private boolean shouldEnforceParentIndent(@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() {
// public void run() {
// }
// });
// }
// to this one:
// void test() {
// foo("test", new Runnable() {
// public void run() {
// }
// });
// }
ASTNode rBrace = myNode.getLastChildNode();
if (node == FormattingAstUtil.getPrevNonWhiteSpaceNode(rBrace)) {
return false;
}
// Filter only anonymous class instances as method call arguments
if (myNode.getElementType() != JavaElementType.EXPRESSION_LIST) {
return false;
}
ASTNode parent = myNode.getTreeParent();
if (parent == null || parent.getElementType() != JavaElementType.METHOD_CALL_EXPRESSION) {
return false;
}
if (node.getElementType() != JavaElementType.NEW_EXPRESSION) {
return false;
}
ASTNode lastChild = node.getLastChildNode();
if (lastChild == null || lastChild.getElementType() != JavaElementType.ANONYMOUS_CLASS) {
return false;
}
// 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());
}
@Nullable
private ASTNode processEnumBlock(List<Block> result,
ASTNode child,
ASTNode last) {
ASTNode last)
{
final WrappingStrategy wrappingStrategy = WrappingStrategy.createDoNotWrapCommaStrategy(Wrap
.createWrap(getWrapType(mySettings.ENUM_CONSTANTS_WRAP), true));
while (child != null) {
@@ -1120,26 +1164,34 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
}
protected Indent getCodeBlockInternalIndent(final int baseChildrenIndent) {
return getCodeBlockInternalIndent(baseChildrenIndent, false);
}
protected Indent getCodeBlockInternalIndent(final int baseChildrenIndent, boolean enforceParentIndent) {
if (isTopLevelClass() && mySettings.DO_NOT_INDENT_TOP_LEVEL_CLASS_MEMBERS) {
return Indent.getNoneIndent();
}
final int braceStyle = getBraceStyle();
return braceStyle == CodeStyleSettings.NEXT_LINE_SHIFTED ?
createNormalIndent(baseChildrenIndent - 1)
: createNormalIndent(baseChildrenIndent);
createNormalIndent(baseChildrenIndent - 1, enforceParentIndent)
: createNormalIndent(baseChildrenIndent, enforceParentIndent);
}
protected static Indent createNormalIndent(final int baseChildrenIndent) {
return createNormalIndent(baseChildrenIndent, false);
}
protected static Indent createNormalIndent(final int baseChildrenIndent, boolean enforceParentIndent) {
if (baseChildrenIndent == 1) {
return Indent.getNormalIndent();
return Indent.getIndent(Indent.Type.NORMAL, false, enforceParentIndent);
}
else if (baseChildrenIndent <= 0) {
return Indent.getNoneIndent();
}
else {
LOG.assertTrue(false);
return Indent.getNormalIndent();
return Indent.getIndent(Indent.Type.NORMAL, false, enforceParentIndent);
}
}
@@ -137,20 +137,14 @@ public class BlockContainingJavaBlock extends AbstractJavaBlock{
}
if (child.getElementType() == ElementType.ELSE_KEYWORD)
return Indent.getNoneIndent();
if (state == BEFORE_FIRST) {
return Indent.getNoneIndent();
}
else if (child.getElementType() == ElementType.WHILE_KEYWORD) {
if (state == BEFORE_FIRST || child.getElementType() == ElementType.WHILE_KEYWORD) {
return Indent.getNoneIndent();
}
else {
if (isPartOfCodeBlock(child)) {
return getCodeBlockExternalIndent();
}
else if (isSimpleStatement(child)){
return getCodeBlockInternalIndent(1);
}
else if (StdTokenSets.COMMENT_BIT_SET.contains(child.getElementType())) {
else if (isSimpleStatement(child) || StdTokenSets.COMMENT_BIT_SET.contains(child.getElementType())){
return getCodeBlockInternalIndent(1);
}
else {
@@ -307,4 +307,29 @@ public class JavaFormatterIndentationTest extends AbstractJavaFormatterTest {
doClassTest(precededBySingleLineComment, precededBySingleLineComment);
doClassTest(precededByMultiLineComment, precededByMultiLineComment);
}
public void testAnonymousClassInstancesAsMethodCallArguments() throws Exception {
// Inspired by IDEA-65987
doMethodTest(
"foo(\"long string as the first argument\", new Runnable() {\n" +
"public void run() { \n" +
"} \n" +
"}, \n" +
"new Runnable() { \n" +
"public void run() { \n" +
"} \n" +
"} \n" +
"); ",
"foo(\"long string as the first argument\", new Runnable() {\n" +
" public void run() { \n" +
" } \n" +
" }, \n" +
" new Runnable() { \n" +
" public void run() { \n" +
" } \n" +
" } \n" +
"); "
);
}
}
@@ -15,10 +15,15 @@
*/
package com.intellij.formatting;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
/**
* The indent setting for a formatting model block. Indicates how the block is indented
* relative to its parent block.
* <p/>
* <b>Relative indents</b>
* <p/>
* Number of factory methods of this class use <code>'indent relative to direct parent'</code> flag. It specified anchor parent block
* to use to apply indent.
* <p/>
@@ -52,6 +57,24 @@ package com.intellij.formatting;
* <p/>
* In contrast, it's possible to specify that direct parent block that starts on a line before target child block is used as an anchor.
* Initial formatting example illustrates such approach.
* <p/>
* <b>Parent indent enforcing</b>
* <p/>
* It's possible to configure indent to enforce indent from parent. Consider the following situation:
* <pre>
* foo("test", new Runnable() {
* public void run() {
* }
* },
* new Runnable() {
* public void run() {
* }
* }
* );
* </pre>
* We want the first {@code 'new Runnable() {...}'} block here to be indented to the method expression list element. However, formatter
* uses indents only if the block starts new line. Here the block doesn't start new line ({@code 'new Runnable() ...'}), hence
* we need to define <code>'enforce parent indent'</code> flag in order to instruct formatter to indent block's bottom lines.
*
* @see com.intellij.formatting.Block#getIndent()
* @see com.intellij.formatting.ChildAttributes#getChildIndent()
@@ -214,4 +237,38 @@ public abstract class Indent {
public static Indent getSpaceIndent(final int spaces, final boolean relativeToDirectParent) {
return myFactory.getSpaceIndent(spaces, relativeToDirectParent);
}
/**
* Base factory method for {@link Indent} objects construction, i.e. all other methods may be expressed in terms of this method.
*
* @param type indent type
* @param relativeToDirectParent flag the indicates if current indent object anchors direct block parent (feel free
* to get more information about that at class-level javadoc)
* @param enforceParentIndent flag the indicates if current indent object should be enforced for multiline block children
* (feel free to get more information about that at class-level javadoc)
* @return newly created indent configured in accordance with the given arguments
*/
public static Indent getIndent(@NotNull Type type, boolean relativeToDirectParent, boolean enforceParentIndent) {
return myFactory.getIndent(type, relativeToDirectParent, enforceParentIndent);
}
public static class Type {
private final String myName;
private Type(@NonNls final String name) {
myName = name;
}
public static final Type SPACES = new Type("SPACES");
public static final Type NONE = new Type("NONE");
public static final Type LABEL = new Type("LABEL");
public static final Type NORMAL = new Type("NORMAL");
public static final Type CONTINUATION = new Type("CONTINUATION");
public static final Type CONTINUATION_WITHOUT_FIRST = new Type("CONTINUATION_WITHOUT_FIRST");
public String toString() {
return myName;
}
}
}
@@ -15,6 +15,8 @@
*/
package com.intellij.formatting;
import org.jetbrains.annotations.NotNull;
/**
* Internal interface for creating indent instances.
* <p/>
@@ -30,4 +32,5 @@ interface IndentFactory {
Indent getContinuationIndent(boolean relativeToDirectParent);
Indent getContinuationWithoutFirstIndent(boolean relativeToDirectParent);
Indent getSpaceIndent(final int spaces, boolean relativeToDirectParent);
Indent getIndent(@NotNull Indent.Type type, boolean relativeToDirectParent, boolean enforceParentIndent);
}
@@ -32,7 +32,7 @@ import static java.util.Arrays.asList;
public abstract class AbstractBlockWrapper {
private static final Set<IndentImpl.Type> RELATIVE_INDENT_TYPES = new HashSet<IndentImpl.Type>(asList(
IndentImpl.Type.NORMAL, IndentImpl.Type.CONTINUATION, IndentImpl.Type.CONTINUATION_WITHOUT_FIRST
Indent.Type.NORMAL, Indent.Type.CONTINUATION, Indent.Type.CONTINUATION_WITHOUT_FIRST
));
protected WhiteSpace myWhiteSpace;
@@ -145,10 +145,10 @@ public abstract class AbstractBlockWrapper {
AbstractBlockWrapper block,
final int tokenBlockStartOffset) {
final IndentImpl indent = block.getIndent();
if (indent.getType() == IndentImpl.Type.CONTINUATION) {
if (indent.getType() == Indent.Type.CONTINUATION) {
return new IndentData(options.CONTINUATION_INDENT_SIZE);
}
if (indent.getType() == IndentImpl.Type.CONTINUATION_WITHOUT_FIRST) {
if (indent.getType() == Indent.Type.CONTINUATION_WITHOUT_FIRST) {
if (block.getStartOffset() != block.getParent().getStartOffset() && block.getStartOffset() == tokenBlockStartOffset) {
return new IndentData(options.CONTINUATION_INDENT_SIZE);
}
@@ -156,9 +156,9 @@ public abstract class AbstractBlockWrapper {
return new IndentData(0);
}
}
if (indent.getType() == IndentImpl.Type.LABEL) return new IndentData(options.LABEL_INDENT_SIZE);
if (indent.getType() == IndentImpl.Type.NONE) return new IndentData(0);
if (indent.getType() == IndentImpl.Type.SPACES) return new IndentData(0, indent.getSpaces());
if (indent.getType() == Indent.Type.LABEL) return new IndentData(options.LABEL_INDENT_SIZE);
if (indent.getType() == Indent.Type.NONE) return new IndentData(0);
if (indent.getType() == Indent.Type.SPACES) return new IndentData(0, indent.getSpaces());
return new IndentData(options.INDENT_SIZE);
}
@@ -166,7 +166,7 @@ public abstract class AbstractBlockWrapper {
public IndentData getChildOffset(AbstractBlockWrapper child, CodeStyleSettings.IndentOptions options, int targetBlockStartOffset) {
final boolean childStartsNewLine = child.getWhiteSpace().containsLineFeeds();
IndentImpl.Type childIndentType = child.getIndent().getType();
final IndentData childIndent;
IndentData childIndent;
// Calculate child indent.
if (childStartsNewLine
@@ -178,6 +178,33 @@ public abstract class AbstractBlockWrapper {
childIndent = new IndentData(0);
}
// Enforce indent if child doesn't start new line, e.g. prefer the code below:
// void test() {
// foo("test", new Runnable() {
// public void run() {
// }
// },
// new Runnable() {
// public void run() {
// }
// }
// );
// }
// to this one:
// void test() {
// foo("test", new Runnable() {
// public void run() {
// }
// },
// new Runnable() {
// public void run() {
// }
// }
// );
// }
if (child.getIndent().isEnforceParentIndent() && !child.getWhiteSpace().containsLineFeeds()) {
childIndent = childIndent.add(getIndent(options, child, getStartOffset()));
}
// Use child indent if it's absolute and the child is contained on new line.
if (childStartsNewLine) {
@@ -381,10 +408,10 @@ public abstract class AbstractBlockWrapper {
}
private static IndentData getIndent(final CodeStyleSettings.IndentOptions options, final int index, IndentImpl indent) {
if (indent.getType() == IndentImpl.Type.CONTINUATION) {
if (indent.getType() == Indent.Type.CONTINUATION) {
return new IndentData(options.CONTINUATION_INDENT_SIZE);
}
if (indent.getType() == IndentImpl.Type.CONTINUATION_WITHOUT_FIRST) {
if (indent.getType() == Indent.Type.CONTINUATION_WITHOUT_FIRST) {
if (index != 0) {
return new IndentData(options.CONTINUATION_INDENT_SIZE);
}
@@ -392,9 +419,9 @@ public abstract class AbstractBlockWrapper {
return new IndentData(0);
}
}
if (indent.getType() == IndentImpl.Type.LABEL) return new IndentData(options.LABEL_INDENT_SIZE);
if (indent.getType() == IndentImpl.Type.NONE) return new IndentData(0);
if (indent.getType() == IndentImpl.Type.SPACES) return new IndentData(indent.getSpaces(), 0);
if (indent.getType() == Indent.Type.LABEL) return new IndentData(options.LABEL_INDENT_SIZE);
if (indent.getType() == Indent.Type.NONE) return new IndentData(0);
if (indent.getType() == Indent.Type.SPACES) return new IndentData(indent.getSpaces(), 0);
return new IndentData(options.INDENT_SIZE);
}
@@ -52,18 +52,18 @@ public class FormatterImpl extends FormatterEx
private FormattingProgressIndicatorImpl myProgressIndicator;
private int myIsDisabledCount = 0;
private final IndentImpl NONE_INDENT = new IndentImpl(IndentImpl.Type.NONE, false, false);
private final IndentImpl myAbsoluteNoneIndent = new IndentImpl(IndentImpl.Type.NONE, true, false);
private final IndentImpl myLabelIndent = new IndentImpl(IndentImpl.Type.LABEL, false, false);
private final IndentImpl myContinuationIndentRelativeToDirectParent = new IndentImpl(IndentImpl.Type.CONTINUATION, false, true);
private final IndentImpl myContinuationIndentNotRelativeToDirectParent = new IndentImpl(IndentImpl.Type.CONTINUATION, false, false);
private final IndentImpl NONE_INDENT = new IndentImpl(Indent.Type.NONE, false, false);
private final IndentImpl myAbsoluteNoneIndent = new IndentImpl(Indent.Type.NONE, true, false);
private final IndentImpl myLabelIndent = new IndentImpl(Indent.Type.LABEL, false, false);
private final IndentImpl myContinuationIndentRelativeToDirectParent = new IndentImpl(Indent.Type.CONTINUATION, false, true);
private final IndentImpl myContinuationIndentNotRelativeToDirectParent = new IndentImpl(Indent.Type.CONTINUATION, false, false);
private final IndentImpl myContinuationWithoutFirstIndentRelativeToDirectParent
= new IndentImpl(IndentImpl.Type.CONTINUATION_WITHOUT_FIRST, false, true);
= new IndentImpl(Indent.Type.CONTINUATION_WITHOUT_FIRST, false, true);
private final IndentImpl myContinuationWithoutFirstIndentNotRelativeToDirectParent
= new IndentImpl(IndentImpl.Type.CONTINUATION_WITHOUT_FIRST, false, false);
private final IndentImpl myAbsoluteLabelIndent = new IndentImpl(IndentImpl.Type.LABEL, true, false);
private final IndentImpl myNormalIndentRelativeToDirectParent = new IndentImpl(IndentImpl.Type.NORMAL, false, true);
private final IndentImpl myNormalIndentNotRelativeToDirectParent = new IndentImpl(IndentImpl.Type.NORMAL, false, false);
= new IndentImpl(Indent.Type.CONTINUATION_WITHOUT_FIRST, false, false);
private final IndentImpl myAbsoluteLabelIndent = new IndentImpl(Indent.Type.LABEL, true, false);
private final IndentImpl myNormalIndentRelativeToDirectParent = new IndentImpl(Indent.Type.NORMAL, false, true);
private final IndentImpl myNormalIndentNotRelativeToDirectParent = new IndentImpl(Indent.Type.NORMAL, false, false);
private final SpacingImpl myReadOnlySpacing = new SpacingImpl(0, 0, 0, true, false, true, 0, false, 0);
public FormatterImpl() {
@@ -313,17 +313,16 @@ public class FormatterImpl extends FormatterEx
while (tokenBlock != null) {
final WhiteSpace whiteSpace = tokenBlock.getWhiteSpace();
if (whiteSpace.getEndOffset() < textRange.getStartOffset()) {
if (whiteSpace.getEndOffset() < textRange.getStartOffset() || whiteSpace.getEndOffset() > textRange.getEndOffset() + 1) {
whiteSpace.setIsReadOnly(true);
} else if (whiteSpace.getStartOffset() > textRange.getStartOffset() &&
whiteSpace.getEndOffset() < textRange.getEndOffset()){
whiteSpace.getEndOffset() < textRange.getEndOffset())
{
if (whiteSpace.containsLineFeeds()) {
whiteSpace.setLineFeedsAreReadOnly(true);
} else {
whiteSpace.setIsReadOnly(true);
}
} else if (whiteSpace.getEndOffset() > textRange.getEndOffset() + 1) {
whiteSpace.setIsReadOnly(true);
}
tokenBlock = tokenBlock.getNextBlock();
@@ -653,7 +652,12 @@ public class FormatterImpl extends FormatterEx
}
public Indent getSpaceIndent(final int spaces, final boolean relative) {
return new IndentImpl(IndentImpl.Type.SPACES, false, spaces, relative);
return new IndentImpl(Indent.Type.SPACES, false, spaces, relative, false);
}
@Override
public Indent getIndent(@NotNull Indent.Type type, boolean relativeToDirectParent, boolean enforceIndent) {
return new IndentImpl(type, false, 0, relativeToDirectParent, enforceIndent);
}
public Indent getAbsoluteLabelIndent() {
@@ -22,38 +22,20 @@ class IndentImpl extends Indent {
private final boolean myIsAbsolute;
private final boolean myRelativeToDirectParent;
static class Type{
private final String myName;
public Type(@NonNls final String name) {
myName = name;
}
public static final Type SPACES = new Type("SPACES");
public static final Type NONE = new Type("NONE");
public static final Type LABEL = new Type("LABEL");
public static final Type NORMAL = new Type("NORMAL");
public static final Type CONTINUATION = new Type("CONTINUATION");
public static final Type CONTINUATION_WITHOUT_FIRST = new Type("CONTINUATION_WITHOUT_FIRST");
public String toString() {
return myName;
}
}
private final Type myType;
private final int mySpaces;
private final boolean myEnforceParentIndent;
public IndentImpl(final Type type, boolean absolute, final int spaces, boolean relativeToDirectParent) {
public IndentImpl(final Type type, boolean absolute, boolean relativeToDirectParent) {
this(type, absolute, 0, relativeToDirectParent, false);
}
public IndentImpl(final Type type, boolean absolute, final int spaces, boolean relativeToDirectParent, boolean enforceParentIndent) {
myType = type;
myIsAbsolute = absolute;
mySpaces = spaces;
myRelativeToDirectParent = relativeToDirectParent;
}
public IndentImpl(final Type type, boolean absolute, boolean relativeToDirectParent) {
this(type, absolute, 0, relativeToDirectParent);
myEnforceParentIndent = enforceParentIndent;
}
Type getType() {
@@ -83,12 +65,27 @@ class IndentImpl extends Indent {
return myRelativeToDirectParent;
}
/**
* Allows to answer if current indent object is configured to enforce indent for sub-blocks of composite block that doesn't start
* new line.
* <p/>
* Feel free to check {@link Indent} javadoc for the more detailed explanation of this property usage.
*
* @return <code>true</code> if current indent object is configured to enforce indent for sub-blocks of composite block
* that doesn't start new line; <code>false</code> otherwise
*/
public boolean isEnforceParentIndent() {
return myEnforceParentIndent;
}
@NonNls
@Override
public String toString() {
if (myType == Type.SPACES) {
return "<Indent: SPACES(" + mySpaces + ")>";
}
return "<Indent: " + myType + (myIsAbsolute ? ":ABSOLUTE" : "") + ">";
return "<Indent: " + myType + (myIsAbsolute ? ":ABSOLUTE " : "")
+ (myRelativeToDirectParent ? " relative to direct parent " : "")
+ (myEnforceParentIndent ? " enforce parent indent" : "") + ">";
}
}