IDEA-87415 Formatter: cyclic alignment resolution

1. Skip cycled backward alignments instead of reporting an error;
2. Green code policy;
This commit is contained in:
Denis.Zhdanov
2012-07-02 08:58:56 +04:00
parent cb19f049ac
commit b40cedf906
7 changed files with 34 additions and 26 deletions
@@ -536,7 +536,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
}
else {
AlignmentStrategy alignmentStrategyToUse = AlignmentStrategy.wrap(arrangeChildAlignment(child, alignmentStrategy));
if (myAlignmentStrategy != null && myAlignmentStrategy.getAlignment(nodeType, childType) != null
if (myAlignmentStrategy.getAlignment(nodeType, childType) != null
&& (nodeType == JavaElementType.IMPLEMENTS_LIST || nodeType == JavaElementType.CLASS))
{
alignmentStrategyToUse = myAlignmentStrategy;
@@ -991,7 +991,7 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo
// The whole idea of variable declarations alignment is that complete declaration blocks which children are to be aligned hold
// reference to the same AlignmentStrategy object, hence, reuse the same Alignment objects. So, there is no point in checking
// if it's necessary to align sub-blocks if shared strategy is not defined.
if (myAlignmentStrategy == null || !mySettings.ALIGN_GROUP_FIELD_DECLARATIONS) {
if (!mySettings.ALIGN_GROUP_FIELD_DECLARATIONS) {
return null;
}
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2009 JetBrains s.r.o.
* Copyright 2000-2012 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.
@@ -36,7 +36,8 @@ public class LeafBlock implements ASTBlock{
public LeafBlock(final ASTNode node,
final Wrap wrap,
final Alignment alignment,
Indent indent) {
Indent indent)
{
myNode = node;
myWrap = wrap;
myAlignment = alignment;
@@ -68,7 +68,8 @@ public class SimpleJavaBlock extends AbstractJavaBlock {
while (child != null) {
if (!FormatterUtil.containsWhiteSpacesOnly(child) && child.getTextLength() > 0){
final ASTNode astNode = child;
AlignmentStrategy alignmentStrategyToUse = ALIGN_IN_COLUMNS_ELEMENT_TYPES.contains(myNode.getElementType()) ? myAlignmentStrategy
AlignmentStrategy alignmentStrategyToUse = ALIGN_IN_COLUMNS_ELEMENT_TYPES.contains(myNode.getElementType())
? myAlignmentStrategy
: AlignmentStrategy.wrap(chooseAlignment(myReservedAlignment, myReservedAlignment2, child));
child = processChild(result, astNode, alignmentStrategyToUse, childWrap, indent, offset);
if (astNode != child && child != null) {
@@ -84,25 +84,10 @@ public abstract class AbstractBlockAlignmentProcessor implements BlockAlignmentP
// to block 'i3' and reformatting starts back after 'i1'. Now 'i2' is shifted to left as well in order to align to the
// new 'i1' position. That changes 'i3' position as well that causes 'i1' to be shifted right one more time.
// Hence, we have endless cycle here. We remember information about blocks that caused indentation change because of
// alignment of blocks located before them and post error every time we detect endless cycle.
// alignment of blocks located before them and skip alignment every time we detect an endless cycle.
Set<LeafBlockWrapper> blocksCausedRealignment = context.backwardShiftedAlignedBlocks.get(offsetResponsibleBlock);
if (blocksCausedRealignment != null && blocksCausedRealignment.contains(context.targetBlock)) {
StringBuilder messageBuilder = new StringBuilder();
TextRange targetRange = context.targetBlock.getTextRange();
messageBuilder.append(
String.format("Formatting error - code block %s is set to be shifted right because of its alignment with "
+ "block %s more than once. I.e. moving the former block because of alignment algorithm causes "
+ "subsequent block to be shifted right as well - cyclic dependency.",
offsetResponsibleBlock.getTextRange(), targetRange
));
messageBuilder.append(context.targetBlock.getDebugInfo());
messageBuilder.append("\nBlock content: '")
.append(context.document.getText().substring(targetRange.getStartOffset(), targetRange.getEndOffset()))
.append("'\n");
messageBuilder.append("Note: document text is attached to this report.");
LogMessageEx.error(LOG, messageBuilder.toString(), context.document.getText());
blocksCausedRealignment.add(context.targetBlock);
return Result.UNABLE_TO_ALIGN_BACKWARD_BLOCK;
return Result.RECURSION_DETECTED;
}
WhiteSpace previousWhiteSpace = offsetResponsibleBlock.getWhiteSpace();
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2009 JetBrains s.r.o.
* Copyright 2000-2012 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.
@@ -79,7 +79,7 @@ class AlignmentImpl extends Alignment {
* Try to find out result from those filtered blocks using the following algorithm:
* <ol>
* <li>
* Use last block (block which has the greatest start offset) after the block which
* Use the last block (block which has the greatest start offset) after the block which
* {@link AbstractBlockWrapper#getWhiteSpace() white space} contains line feeds;
* </li>
* <li>
@@ -45,6 +45,9 @@ public interface BlockAlignmentProcessor {
/** Already processed block was realigned because of {@link AlignmentImpl#isAllowBackwardShift() backward alignment}. */
BACKWARD_BLOCK_ALIGNED,
/** Detected that backward alignment dependency graph is cycled. */
RECURSION_DETECTED,
/**
* It was necessary to align already processed block because of {@link AlignmentImpl#isAllowBackwardShift() backward alignment}
* but that can't be done (e.g. that backward block {@link AbstractBlockWrapper#getWhiteSpace() white space} is read-only).
@@ -667,6 +667,8 @@ class FormatProcessor {
myCurrentBlock = offsetResponsibleBlock.getNextBlock();
onCurrentLineChanged();
return false;
case RECURSION_DETECTED:
myCurrentBlock = offsetResponsibleBlock; // Fall through to the 'register alignment to skip'.
case UNABLE_TO_ALIGN_BACKWARD_BLOCK:
myAlignmentsToSkip.add(alignment);
return false;
@@ -1179,8 +1181,8 @@ class FormatProcessor {
}
private static int calcShift(final IndentInside lastLineIndent, final IndentInside whiteSpaceIndent,
final CommonCodeStyleSettings.IndentOptions options
) {
final CommonCodeStyleSettings.IndentOptions options)
{
if (lastLineIndent.equals(whiteSpaceIndent)) return 0;
if (options.USE_TAB_CHARACTER) {
if (lastLineIndent.whiteSpaces > 0) {
@@ -1200,6 +1202,22 @@ class FormatProcessor {
}
}
/**
* Utility method to use during debugging formatter processing.
*
* @return text that contains intermediate formatter-introduced changes (even not committed yet)
*/
@SuppressWarnings("UnusedDeclaration")
@NotNull
private String getCurrentText() {
StringBuilder result = new StringBuilder();
for (LeafBlockWrapper block = myFirstTokenBlock; block != null; block = block.getNextBlock()) {
result.append(block.getWhiteSpace().generateWhiteSpace(getIndentOptionsToUse(block, myDefaultIndentOption)));
result.append(myDocument.getCharsSequence().subSequence(block.getStartOffset(), block.getEndOffset()));
}
return result.toString();
}
private abstract class State {
private final FormattingStateId myStateId;