WI-9265 Ternary reformat hangs

Correct core formatting processing in case of dependent spacing that target region that looses line feeds during formatting
This commit is contained in:
Denis.Zhdanov
2012-01-11 15:34:41 +04:00
parent 5e39306022
commit 0a4bf8130f
4 changed files with 72 additions and 38 deletions
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2010 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.
@@ -16,7 +16,7 @@
package com.intellij.psi.formatter.java;
import com.intellij.openapi.fileTypes.StdFileTypes;
import com.intellij.psi.codeStyle.CodeStyleSettings;
import com.intellij.psi.codeStyle.CommonCodeStyleSettings;
import com.intellij.util.IncorrectOperationException;
/**
@@ -31,7 +31,7 @@ public class JavaFormatterAlignmentTest extends AbstractJavaFormatterTest {
public void testChainedMethodsAlignment() throws Exception {
// Inspired by IDEA-30369
getSettings().ALIGN_MULTILINE_CHAINED_METHODS = true;
getSettings().METHOD_CALL_CHAIN_WRAP = CodeStyleSettings.WRAP_AS_NEEDED;
getSettings().METHOD_CALL_CHAIN_WRAP = CommonCodeStyleSettings.WRAP_AS_NEEDED;
getSettings().getRootSettings().getIndentOptions(StdFileTypes.JAVA).CONTINUATION_INDENT_SIZE = 8;
doTest();
}
@@ -93,7 +93,7 @@ public class JavaFormatterAlignmentTest extends AbstractJavaFormatterTest {
public void testArrayInitializer() throws IncorrectOperationException {
// Inspired by IDEADEV-16136
getSettings().ARRAY_INITIALIZER_WRAP = CodeStyleSettings.WRAP_ALWAYS;
getSettings().ARRAY_INITIALIZER_WRAP = CommonCodeStyleSettings.WRAP_ALWAYS;
getSettings().ALIGN_MULTILINE_ARRAY_INITIALIZER_EXPRESSION = true;
doTextTest(
@@ -153,8 +153,8 @@ public class JavaFormatterAlignmentTest extends AbstractJavaFormatterTest {
public void testFieldInColumnsAlignment() {
// Inspired by IDEA-55147
getSettings().ALIGN_GROUP_FIELD_DECLARATIONS = true;
getSettings().FIELD_ANNOTATION_WRAP = CodeStyleSettings.DO_NOT_WRAP;
getSettings().VARIABLE_ANNOTATION_WRAP = CodeStyleSettings.DO_NOT_WRAP;
getSettings().FIELD_ANNOTATION_WRAP = CommonCodeStyleSettings.DO_NOT_WRAP;
getSettings().VARIABLE_ANNOTATION_WRAP = CommonCodeStyleSettings.DO_NOT_WRAP;
doTextTest(
"public class FormattingTest {\n" +
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2010 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.
@@ -16,7 +16,7 @@
package com.intellij.psi.formatter.java;
import com.intellij.openapi.fileTypes.StdFileTypes;
import com.intellij.psi.codeStyle.CodeStyleSettings;
import com.intellij.psi.codeStyle.CommonCodeStyleSettings;
import com.intellij.util.IncorrectOperationException;
/**
@@ -168,7 +168,7 @@ public class JavaFormatterNewLineTest extends AbstractJavaFormatterTest {
public void testArrayInitializer() throws IncorrectOperationException {
// Inspired by IDEADEV-6787
getSettings().ARRAY_INITIALIZER_WRAP = CodeStyleSettings.WRAP_ALWAYS;
getSettings().ARRAY_INITIALIZER_WRAP = CommonCodeStyleSettings.WRAP_ALWAYS;
getSettings().ARRAY_INITIALIZER_LBRACE_ON_NEXT_LINE = true;
getSettings().ARRAY_INITIALIZER_RBRACE_ON_NEXT_LINE = true;
doTextTest(
@@ -195,7 +195,7 @@ public class JavaFormatterNewLineTest extends AbstractJavaFormatterTest {
public void testSimpleAnnotatedMethodAndBraceOnNextLineStyle() throws Exception {
// Inspired by IDEA-53542
getSettings().METHOD_BRACE_STYLE = CodeStyleSettings.NEXT_LINE;
getSettings().METHOD_BRACE_STYLE = CommonCodeStyleSettings.NEXT_LINE;
getSettings().KEEP_SIMPLE_METHODS_IN_ONE_LINE = true;
getSettings().KEEP_LINE_BREAKS = true;
getSettings().KEEP_BLANK_LINES_IN_CODE = 2;
@@ -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.
@@ -26,7 +26,7 @@ import com.intellij.openapi.util.TextRange;
public class DependantSpacingImpl extends SpacingImpl {
private final TextRange myDependency;
private static final int DEPENDENCE_CONTAINS_LF_MASK = 0x10;
private static final int LF_WAS_USED_MASK = 0x20;
private static final int DEPENDENT_REGION_LF_CHANGED_MASK = 0x20;
public DependantSpacingImpl(final int minSpaces,
final int maxSpaces,
@@ -50,7 +50,10 @@ public class DependantSpacingImpl extends SpacingImpl {
}
public void refresh(FormatProcessor formatter) {
final boolean value = wasLFUsed() || formatter.containsLineFeeds(myDependency);
if (isDependentRegionChanged()) {
return;
}
final boolean value = formatter.containsLineFeeds(myDependency);
if (value) myFlags |= DEPENDENCE_CONTAINS_LF_MASK;
else myFlags &= ~DEPENDENCE_CONTAINS_LF_MASK;
}
@@ -59,13 +62,22 @@ public class DependantSpacingImpl extends SpacingImpl {
return myDependency;
}
public final void setLFWasUsed(final boolean value) {
if (value) myFlags |= LF_WAS_USED_MASK;
else myFlags &=~ LF_WAS_USED_MASK;
/**
* Allows to answer whether the target dependent regions has been changed during formatting.
*
* @return <code>true</code> if target dependent region has been changed during formatting; <code>false</code> otherwise
*/
public final boolean isDependentRegionChanged() {
return (myFlags & DEPENDENT_REGION_LF_CHANGED_MASK) != 0;
}
public final boolean wasLFUsed() {
return (myFlags & LF_WAS_USED_MASK) != 0;
/**
* Allows to set {@link #isDependentRegionChanged() 'dependent region changed'} property.
*/
public final void setDependentRegionChanged() {
myFlags |= DEPENDENT_REGION_LF_CHANGED_MASK;
if (getMinLineFeeds() <= 0) myFlags |= DEPENDENCE_CONTAINS_LF_MASK;
else myFlags &=~DEPENDENCE_CONTAINS_LF_MASK;
}
@Override
@@ -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.
@@ -23,7 +23,6 @@ import com.intellij.openapi.editor.ex.DocumentEx;
import com.intellij.openapi.editor.impl.BulkChangesMerger;
import com.intellij.openapi.editor.impl.TextChangeImpl;
import com.intellij.openapi.fileTypes.StdFileTypes;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.util.TextRange;
import com.intellij.psi.codeStyle.CodeStyleSettings;
import com.intellij.psi.codeStyle.CommonCodeStyleSettings;
@@ -91,9 +90,35 @@ class FormatProcessor {
private LeafBlockWrapper myFirstTokenBlock;
private LeafBlockWrapper myLastTokenBlock;
private SortedMap<TextRange, Pair<AbstractBlockWrapper, Boolean>> myPreviousDependencies =
new TreeMap<TextRange, Pair<AbstractBlockWrapper, Boolean>>(new Comparator<TextRange>() {
/**
* Formatter provides a notion of {@link DependantSpacingImpl dependent spacing}, i.e. spacing that insist on line feed if target
* dependent region contains line feed.
* <p/>
* Example:
* <pre>
* int[] data = {1, 2, 3};
* </pre>
* We want to keep that in one line with possible but place curly braces on separate lines if the width is not enough:
* <pre>
* int[] data = { | &lt; right margin
* 1, 2, 3 |
* } |
* </pre>
* There is a possible case that particular block has dependent spacing property that targets region that lays beyond the
* current block. E.g. consider example above - <code>'1'</code> block has dependent spacing that targets the whole
* <code>'{1, 2, 3}'</code> block. So, it's not possible to answer whether line feed should be used during processing block
* <code>'1'</code>.
* <p/>
* We store such 'forward dependencies' at the current collection where the key is the range of the target 'dependent forward
* region' and value is dependent spacing object.
* <p/>
* Every time we detect that formatter changes 'has line feeds' status of such dependent region, we
* {@link DependantSpacingImpl#setDependentRegionChanged() mark} the dependent spacing as changed and schedule one more
* formatting iteration.
*/
private SortedMap<TextRange, DependantSpacingImpl> myPreviousDependencies =
new TreeMap<TextRange, DependantSpacingImpl>(new Comparator<TextRange>() {
public int compare(final TextRange o1, final TextRange o2) {
int offsetsDelta = o1.getEndOffset() - o2.getEndOffset();
@@ -434,37 +459,33 @@ class FormatProcessor {
}
private boolean shouldReformatBecauseOfBackwardDependency(TextRange changed) {
final SortedMap<TextRange, Pair<AbstractBlockWrapper, Boolean>> sortedHeadMap = myPreviousDependencies.tailMap(changed);
final SortedMap<TextRange, DependantSpacingImpl> sortedHeadMap = myPreviousDependencies.tailMap(changed);
for (final Map.Entry<TextRange, Pair<AbstractBlockWrapper, Boolean>> entry : sortedHeadMap.entrySet()) {
boolean result = false;
for (final Map.Entry<TextRange, DependantSpacingImpl> entry : sortedHeadMap.entrySet()) {
final TextRange textRange = entry.getKey();
if (textRange.contains(changed)) {
final Pair<AbstractBlockWrapper, Boolean> pair = entry.getValue();
final boolean containedLineFeeds = pair.getSecond().booleanValue();
final DependantSpacingImpl dependentSpacing = entry.getValue();
final boolean containedLineFeeds = dependentSpacing.getMinLineFeeds() > 0;
final boolean containsLineFeeds = containsLineFeeds(textRange);
if (containedLineFeeds != containsLineFeeds) {
return true;
dependentSpacing.setDependentRegionChanged();
result = true;
}
}
}
return false;
return result;
}
private void saveDependency(final SpacingImpl spaceProperty) {
final DependantSpacingImpl dependantSpaceProperty = (DependantSpacingImpl)spaceProperty;
final TextRange dependency = dependantSpaceProperty.getDependency();
if (dependantSpaceProperty.wasLFUsed()) {
myPreviousDependencies.put(dependency, new Pair<AbstractBlockWrapper, Boolean>(myCurrentBlock, Boolean.TRUE));
}
else {
final boolean value = containsLineFeeds(dependency);
if (value) {
dependantSpaceProperty.setLFWasUsed(true);
}
myPreviousDependencies.put(dependency, new Pair<AbstractBlockWrapper, Boolean>(myCurrentBlock, value));
if (dependantSpaceProperty.isDependentRegionChanged()) {
return;
}
myPreviousDependencies.put(dependency, dependantSpaceProperty);
}
private static boolean shouldSaveDependency(final SpacingImpl spaceProperty, WhiteSpace whiteSpace) {
@@ -1293,6 +1314,7 @@ class FormatProcessor {
}
else {
myAlignAgain.clear();
myPreviousDependencies.clear();
myCurrentBlock = myFirstTokenBlock;
}
}