MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-03-22 15:19:59 +07:00
Code Issues Packages Projects Releases Wiki Activity

[Java. Code Formatting] Attempt to fix non-deterministic formatting when dependent whitespaces are used

Browse Source 
IDEA-181768

GitOrigin-RevId: a31e63ad2a6145aec5ddfe329c9a7cf10d116cc7
This commit is contained in:
Georgii Ustinov
2024-07-08 15:36:50 +00:00
committed by intellij-monorepo-bot
parent 1ab7c19466
commit dbc682a07e
20 changed files with 338 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/ChopDownIfLongMultipleLines.java Normal file
View File

@@ -0,0 +1,6 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Main {
private static final LocalDateTime localDateTime = LocalDateTime.of(2017, Month.NOVEMBER, 1, 2, 3);
}

11
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/ChopDownIfLongMultipleLines_after.java Normal file
View File

@@ -0,0 +1,11 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Main {
private static final LocalDateTime localDateTime = LocalDateTime.of(
2017,
Month.NOVEMBER,
1,
2,
3);
}

6
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/ChopDownIfLongSingleLine.java Normal file
View File

@@ -0,0 +1,6 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Main {
private static final LocalDateTime localDateTime = LocalDateTime.of(2017, Month.NOVEMBER, 1, 2, 3);
}

6
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/ChopDownIfLongSingleLine_after.java Normal file
View File

@@ -0,0 +1,6 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Main {
private static final LocalDateTime localDateTime = LocalDateTime.of(2017, Month.NOVEMBER, 1, 2, 3);
}

18
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/MultipleCalls.java Normal file
View File

@@ -0,0 +1,18 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Test {
private static class SomeClass {
SomeClass(int... args) {
}
}
private static final SomeClass someClassInstance = new SomeClass(2017, 100000, 100000, 100000, 100000, 100000, 100000);
private static final LocalDateTime localDateTime = LocalDateTime.of(1000, Month.NOVEMBER, 300000, 10000, 100000, 5000);
public void fooBarBaz(int... args) {
fooBarBaz(100000, 100000, 100000, 100000, 100000, 100000, 100000, 100000, 100000, 100000, 100000, 100000, 100000);
}
}

44
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/MultipleCalls_after.java Normal file
View File

@@ -0,0 +1,44 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Test {
private static class SomeClass {
SomeClass(int... args) {
}
}
private static final SomeClass someClassInstance = new SomeClass(
2017,
100000,
100000,
100000,
100000,
100000,
100000);
private static final LocalDateTime localDateTime = LocalDateTime.of(
1000,
Month.NOVEMBER,
300000,
10000,
100000,
5000);
public void fooBarBaz(int... args) {
fooBarBaz(
100000,
100000,
100000,
100000,
100000,
100000,
100000,
100000,
100000,
100000,
100000,
100000,
100000);
}
}

6
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/WrapAlways.java Normal file
View File

@@ -0,0 +1,6 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Main {
private static final LocalDateTime localDateTime = LocalDateTime.of(2017, Month.NOVEMBER, 1, 2, 3);
}

11
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/WrapAlways_after.java Normal file
View File

@@ -0,0 +1,11 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Main {
private static final LocalDateTime localDateTime = LocalDateTime.of(
2017,
Month.NOVEMBER,
1,
2,
3);
}

5
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/WrapAsNeedMultipleLines.java Normal file
View File

@@ -0,0 +1,5 @@
public class Test {
public void foo1(int... args) {
foo1(100000, 10000, 10000, 10000, 10000, 10000, 10000, 10000, 10000, 10000, 10000, 10000, 10000);
}
}

7
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/WrapAsNeedMultipleLines_after.java Normal file
View File

@@ -0,0 +1,7 @@
public class Test {
public void foo1(int... args) {
foo1(
100000, 10000, 10000, 10000, 10000, 10000, 10000, 10000, 10000, 10000, 10000, 10000,
10000);
}
}

6
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/WrapAsNeedSingleLine.java Normal file
View File

@@ -0,0 +1,6 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Main {
private static final LocalDateTime localDateTime = LocalDateTime.of(2017, Month.NOVEMBER, 1, 2, 3);
}

7
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/WrapAsNeedSingleLine_after.java Normal file
View File

@@ -0,0 +1,7 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Main {
private static final LocalDateTime localDateTime = LocalDateTime.of(
2017, Month.NOVEMBER, 1, 2, 3);
}

6
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/doNotWrap.java Normal file
View File

@@ -0,0 +1,6 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Main {
private static final LocalDateTime localDateTime = LocalDateTime.of(2017, Month.NOVEMBER, 1, 2, 3);
}

6
java/java-tests/testData/psi/formatter/java/newLineAfterLBrace/doNotWrap_after.java Normal file
View File

@@ -0,0 +1,6 @@
import java.time.LocalDateTime;
import java.time.Month;
public class Main {
private static final LocalDateTime localDateTime = LocalDateTime.of(2017, Month.NOVEMBER, 1, 2, 3);
}

60
java/java-tests/testSrc/com/intellij/java/psi/formatter/java/JavaFormatterNewLineAfterLBraceTest.kt Normal file
View File

@@ -0,0 +1,60 @@
// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package com.intellij.java.psi.formatter.java
import com.intellij.lang.java.JavaLanguage
import com.intellij.psi.codeStyle.CommonCodeStyleSettings
class JavaFormatterNewLineAfterLBraceTest : JavaFormatterTestCase() {
override fun getBasePath(): String = "psi/formatter/java/newLineAfterLBrace"
private val commonSettings: CommonCodeStyleSettings
get() = getSettings(JavaLanguage.INSTANCE)
override fun setUp() {
super.setUp()
commonSettings.CALL_PARAMETERS_LPAREN_ON_NEXT_LINE = true
commonSettings.RIGHT_MARGIN = 100
}
fun testWrapAlways() {
commonSettings.CALL_PARAMETERS_WRAP = CommonCodeStyleSettings.WRAP_ALWAYS
doIdempotentTest()
}
fun testChopDownIfLongMultipleLines() {
commonSettings.CALL_PARAMETERS_WRAP = CommonCodeStyleSettings.WRAP_ON_EVERY_ITEM
doIdempotentTest()
}
fun testChopDownIfLongSingleLine() {
commonSettings.RIGHT_MARGIN = 200
commonSettings.CALL_PARAMETERS_WRAP = CommonCodeStyleSettings.WRAP_ON_EVERY_ITEM
doIdempotentTest()
}
fun testDoNotWrap() {
commonSettings.CALL_PARAMETERS_WRAP = CommonCodeStyleSettings.DO_NOT_WRAP
doTest()
}
fun testWrapAsNeedSingleLine() {
commonSettings.CALL_PARAMETERS_WRAP = CommonCodeStyleSettings.WRAP_AS_NEEDED
doIdempotentTest()
}
fun testWrapAsNeedMultipleLines() {
commonSettings.CALL_PARAMETERS_WRAP = CommonCodeStyleSettings.WRAP_AS_NEEDED
doIdempotentTest()
}
fun testMultipleCalls() {
commonSettings.CALL_PARAMETERS_WRAP = CommonCodeStyleSettings.WRAP_ON_EVERY_ITEM
doIdempotentTest()
}
private fun doIdempotentTest() {
val testName = getTestName(false)
doTest(testName, "${testName}_after")
doTest("${testName}_after", "${testName}_after")
}
}

58
platform/code-style-impl/src/com/intellij/formatting/DependantSpacingImpl.java
View File

@@ -2,6 +2,7 @@
package com.intellij.formatting;
import com.intellij.formatting.engine.AdjustWhiteSpacesState;
import com.intellij.formatting.engine.BlockRangesMap;
import com.intellij.openapi.util.TextRange;
import com.intellij.openapi.util.text.StringUtil;
@@ -18,8 +19,16 @@ import java.util.List;
*/
@ApiStatus.Internal
public class DependantSpacingImpl extends SpacingImpl {
private static final int DEPENDENCE_CONTAINS_LF_MASK = 0x10;
private static final int DEPENDENT_REGION_LF_CHANGED_MASK = 0x20;
private static final int DEPENDENCE_CONTAINS_LF_MASK = 0x10;
/**
* {@link AdjustWhiteSpacesState} can traverse tree of {@link LeafBlockWrapper} multiple times, because of dependent spacings.
* This field represents the index of last iteration
* when there were changes in {@link #myFlags 'DEPENDENCE_CONTAINS_LF_MASK'}.
* During the first iteration, it is allowed both to remove and add line feeds.
* During the next iterations, it is allowed only to add line feeds.
* @see com.intellij.formatting.engine.DependentSpacingEngine#shouldReformatPreviouslyLocatedDependentSpacing
*/
private int myLastLFChangedIteration = 0;
private final @NotNull List<TextRange> myDependentRegionRanges;
private final @NotNull DependentSpacingRule myRule;
@@ -76,11 +85,25 @@ public class DependantSpacingImpl extends SpacingImpl {
return 0;
}
/**
* Checks if the dependent spacing has been changed at least once.
* @return true if the dependent spacing has been changed at least once, false otherwise
*/
public boolean isDependantSpacingChangedAtLeastOnce() {
return myLastLFChangedIteration > 0;
}
/**
*
* Refreshes line feed status of {@link DependantSpacingImpl}.
* Refreshment happens only during the first iteration through all blocks
* @param helper - map of ranges dependent regions
* @see DependantSpacingImpl#setDependentRegionLinefeedStatusChanged(int)
* @see DependantSpacingImpl#isDependantSpacingChangedAtLeastOnce()
*/
@Override
public void refresh(BlockRangesMap helper) {
if (isDependentRegionLinefeedStatusChanged()) {
return;
}
if (isDependantSpacingChangedAtLeastOnce()) return;
boolean atLeastOneDependencyRangeContainsLf = false;
for (TextRange dependency : myDependentRegionRanges) {
@@ -95,21 +118,36 @@ public class DependantSpacingImpl extends SpacingImpl {
return myDependentRegionRanges;
}
/**
* @deprecated use {@link DependantSpacingImpl#isDependentRegionLinefeedStatusChanged(int)} instead
*/
@Deprecated
@ApiStatus.ScheduledForRemoval
public final boolean isDependentRegionLinefeedStatusChanged() {
return isDependantSpacingChangedAtLeastOnce();
}
/**
* Allows to answer whether 'contains line feed' status has been changed for the target dependent region during formatting.
*
* @return {@code true} if target 'contains line feed' status has been changed for the target dependent region during formatting;
* {@code false} otherwise
*/
public final boolean isDependentRegionLinefeedStatusChanged() {
return (myFlags & DEPENDENT_REGION_LF_CHANGED_MASK) != 0;
public final boolean isDependentRegionLinefeedStatusChanged(int currentIteration) {
return currentIteration == myLastLFChangedIteration;
}
/**
* Allows to set {@link #isDependentRegionLinefeedStatusChanged() 'dependent region changed'} property.
* Updates (flips) the line feed status based on a dependent region.
* This method is used
* in {@link com.intellij.formatting.engine.DependentSpacingEngine#shouldReformatPreviouslyLocatedDependentSpacing(WhiteSpace) DependentSpacingEngine#shouldReformatPreviouslyLocatedDependentSpacing(WhiteSpace)}
* Update might happen only once per iteration over all {@link LeafBlockWrapper}
* @param currentIteration number of block iteration
* @see AdjustWhiteSpacesState
* @see com.intellij.formatting.engine.DependentSpacingEngine DependentSpacingEngine
*/
public final void setDependentRegionLinefeedStatusChanged() {
myFlags |= DEPENDENT_REGION_LF_CHANGED_MASK;
public final void setDependentRegionLinefeedStatusChanged(int currentIteration) {
myLastLFChangedIteration = currentIteration;
if (getMinLineFeeds() <= 0) myFlags |= DEPENDENCE_CONTAINS_LF_MASK;
else myFlags &=~DEPENDENCE_CONTAINS_LF_MASK;
}

28
platform/code-style-impl/src/com/intellij/formatting/engine/AdjustWhiteSpacesState.java
View File

@@ -70,12 +70,24 @@ public final class AdjustWhiteSpacesState extends State {
}
if (myAlignAgain.isEmpty()) {
postProcessDependantWhitespaces();
setDone(true);
}
else {
myAlignAgain.clear();
myDependentSpacingEngine.clear();
myCurrentBlock = myFirstBlock;
myDependentSpacingEngine.incrementIteration();
}
}
private void postProcessDependantWhitespaces() {
for (LeafBlockWrapper block : myDependentSpacingEngine.getLeafBlocksToReformat()) {
WhiteSpace whiteSpace = block.getWhiteSpace();
SpacingImpl spaceProperty = block.getSpaceProperty();
updateSpacing(spaceProperty, whiteSpace);
myIndentAdjuster.adjustIndent(block);
}
}
@@ -140,11 +152,7 @@ public final class AdjustWhiteSpacesState extends State {
}
}
whiteSpace.arrangeLineFeeds(spaceProperty, myBlockRangesMap);
if (!whiteSpace.containsLineFeeds()) {
whiteSpace.arrangeSpaces(spaceProperty);
}
updateSpacing(spaceProperty, whiteSpace);
try {
LeafBlockWrapper newBlock = myWrapProcessor.processWrap(myCurrentBlock);
@@ -175,7 +183,7 @@ public final class AdjustWhiteSpacesState extends State {
final List<TextRange> ranges = getDependentRegionRangesAfterCurrentWhiteSpace(spaceProperty, whiteSpace);
if (!ranges.isEmpty()) {
myDependentSpacingEngine.registerUnresolvedDependentSpacingRanges(spaceProperty, ranges);
myDependentSpacingEngine.registerUnresolvedDependentSpacingRanges(myCurrentBlock, ranges);
}
if (!whiteSpace.isIsReadOnly() && myDependentSpacingEngine.shouldReformatPreviouslyLocatedDependentSpacing(whiteSpace)) {
@@ -187,4 +195,12 @@ public final class AdjustWhiteSpacesState extends State {
myCurrentBlock = myCurrentBlock.getNextBlock();
}
private void updateSpacing(SpacingImpl spaceProperty, WhiteSpace whiteSpace) {
whiteSpace.arrangeLineFeeds(spaceProperty, myBlockRangesMap);
if (!whiteSpace.containsLineFeeds()) {
whiteSpace.arrangeSpaces(spaceProperty);
}
}
}

75
platform/code-style-impl/src/com/intellij/formatting/engine/DependentSpacingEngine.java
View File

@@ -2,15 +2,12 @@
package com.intellij.formatting.engine;
import com.intellij.formatting.DependantSpacingImpl;
import com.intellij.formatting.SpacingImpl;
import com.intellij.formatting.LeafBlockWrapper;
import com.intellij.formatting.WhiteSpace;
import com.intellij.openapi.util.TextRange;
import org.jetbrains.annotations.ApiStatus;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.*;
/**
* Formatter provides a notion of {@link DependantSpacingImpl dependent spacing}, i.e. spacing that insist on line feed if target
@@ -35,14 +32,22 @@ import java.util.TreeMap;
* 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#setDependentRegionLinefeedStatusChanged() mark} the dependent spacing as changed and schedule one more
* {@link DependantSpacingImpl#setDependentRegionLinefeedStatusChanged(int)} () mark} the dependent spacing as changed and schedule one more
* formatting iteration.
*/
@ApiStatus.Internal
final class DependentSpacingEngine {
/**
* {@link AdjustWhiteSpacesState} can iterate tree of {@link LeafBlockWrapper} multiple times, because of dependent spacings.
* This field represents current iteration number
*/
private int myIteration = 1;
private final BlockRangesMap myBlockRangesMap;
private final SortedMap<TextRange, DependantSpacingImpl> myPreviousDependencies =
private final Set<LeafBlockWrapper> myLeafBlocksToReformat = new HashSet<>();
private final SortedMap<TextRange, LeafBlockWrapper> myPreviousDependencies =
new TreeMap<>((o1, o2) -> {
int offsetsDelta = o1.getEndOffset() - o2.getEndOffset();
@@ -56,16 +61,23 @@ final class DependentSpacingEngine {
myBlockRangesMap = helper;
}
/**
* Reformat all dependent spaces, which have a dependency on a {@link TextRange} corresponding to the 'space'.
* {@link AdjustWhiteSpacesState} iterates through all leafs again in case if there was a first change.
* @param space - candidate who could potentially be located in one of the dependent regions
* @return true if it was the first change, and false otherwise
*/
boolean shouldReformatPreviouslyLocatedDependentSpacing(WhiteSpace space) {
final TextRange changed = space.getTextRange();
final SortedMap<TextRange, DependantSpacingImpl> sortedHeadMap = myPreviousDependencies.tailMap(changed);
final SortedMap<TextRange, LeafBlockWrapper> sortedHeadMap = myPreviousDependencies.tailMap(changed);
for (final Map.Entry<TextRange, DependantSpacingImpl> entry : sortedHeadMap.entrySet()) {
for (final Map.Entry<TextRange, LeafBlockWrapper> entry : sortedHeadMap.entrySet()) {
final TextRange textRange = entry.getKey();
if (textRange.contains(changed)) {
final DependantSpacingImpl spacing = entry.getValue();
if (spacing.isDependentRegionLinefeedStatusChanged()) {
final LeafBlockWrapper currentBlock = entry.getValue();
if (!(currentBlock.getSpaceProperty() instanceof DependantSpacingImpl spacing) ||
spacing.isDependentRegionLinefeedStatusChanged(myIteration)) {
continue;
}
@@ -73,24 +85,47 @@ final class DependentSpacingEngine {
final boolean containsLineFeeds = myBlockRangesMap.containsLineFeedsOrTooLong(textRange);
if (containedLineFeeds != containsLineFeeds) {
spacing.setDependentRegionLinefeedStatusChanged();
return true;
if (!spacing.isDependantSpacingChangedAtLeastOnce()) {
spacing.setDependentRegionLinefeedStatusChanged(myIteration);
return true;
}
else if (containsLineFeeds) {
spacing.setDependentRegionLinefeedStatusChanged(myIteration);
myLeafBlocksToReformat.add(currentBlock);
return false;
}
}
}
}
return false;
}
void registerUnresolvedDependentSpacingRanges(final SpacingImpl spaceProperty, List<? extends TextRange> unprocessedRanges) {
final DependantSpacingImpl dependantSpaceProperty = (DependantSpacingImpl)spaceProperty;
if (dependantSpaceProperty.isDependentRegionLinefeedStatusChanged()) return;
void incrementIteration() {
myIteration++;
}
/**
* Registers all dependant ranges for the current 'leafBlockWrapper'. It happens only once per iteration
* @param leafBlockWrapper - block, from which dependent spacing is extracted
* @param unprocessedRanges - list of dependent ranges
* @see DependantSpacingImpl#isDependantSpacingChangedAtLeastOnce()
*/
void registerUnresolvedDependentSpacingRanges(final LeafBlockWrapper leafBlockWrapper, List<? extends TextRange> unprocessedRanges) {
if (!(leafBlockWrapper.getSpaceProperty() instanceof DependantSpacingImpl dependantSpaceProperty)) return;
if (dependantSpaceProperty.isDependentRegionLinefeedStatusChanged(myIteration)) return;
for (TextRange range: unprocessedRanges) {
myPreviousDependencies.put(range, dependantSpaceProperty);
myPreviousDependencies.put(range, leafBlockWrapper);
}
}
/**
* Returns a list of blocks with dependant spaces which should be reformatted after {@link AdjustWhiteSpacesState} is finished.
*/
Set<LeafBlockWrapper> getLeafBlocksToReformat() { return myLeafBlocksToReformat; }
void clear() {
myPreviousDependencies.clear();
}

7
plugins/kotlin/idea/tests/testData/refactoring/introduceParameter/callableReferenceToShorten.kt.after
View File

@@ -16,7 +16,8 @@ fun m1(p: Any) {
}
fun m() {
m1(ExampleClass::class.members
.first()
.apply { isAccessible = true })
m1(
ExampleClass::class.members
.first()
.apply { isAccessible = true })
}

7
plugins/kotlin/idea/tests/testData/refactoring/introduceParameter/callableReferenceToShorten.kt.fir.after
View File

@@ -16,7 +16,8 @@ fun m1(callable: KCallable<*>) {
}
fun m() {
m1(ExampleClass::class.members
.first()
.apply { isAccessible = true })
m1(
ExampleClass::class.members
.first()
.apply { isAccessible = true })
}