IDEA-210815 Extract variable should not perform reordering magic in case if both condition branches have occurrences

GitOrigin-RevId: 09b6ff937cd677dca490517e9ddce2db0de72e0a
This commit is contained in:
Tagir Valeev
2019-05-21 12:13:18 +03:00
committed by intellij-monorepo-bot
parent 4a3360e219
commit 797496b8a9
4 changed files with 26 additions and 9 deletions
@@ -33,10 +33,7 @@ import com.siyeh.ig.psiutils.ReorderingUtils;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.NotNull;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
import java.util.*;
/**
* Performs actual write action (see {@link #extractVariable()}) which introduces new variable and replaces all occurrences.
@@ -286,8 +283,8 @@ class VariableExtractor {
expr = (PsiExpression)anchor;
}
}
PsiExpression firstOccurrence = StreamEx.of(occurrences).append(expr)
.minBy(e -> e.getTextRange().getStartOffset()).orElse(null);
Set<PsiExpression> allOccurrences = StreamEx.of(occurrences).append(expr).toSet();
PsiExpression firstOccurrence = Collections.min(allOccurrences, Comparator.comparing(e -> e.getTextRange().getStartOffset()));
if (anchor instanceof PsiWhileStatement) {
PsiExpression condition = ((PsiWhileStatement)anchor).getCondition();
if (condition != null) {
@@ -305,9 +302,14 @@ class VariableExtractor {
if (firstOccurrence != null && ControlFlowUtils.canExtractStatement(firstOccurrence) &&
!PsiUtil.isAccessedForWriting(firstOccurrence)) {
PsiExpression ancestorCandidate = ExpressionUtils.getTopLevelExpression(firstOccurrence);
if (PsiTreeUtil.isAncestor(anchor, ancestorCandidate, false) &&
ReorderingUtils.canExtract(ancestorCandidate, firstOccurrence) == ThreeState.NO) {
return firstOccurrence;
if (PsiTreeUtil.isAncestor(anchor, ancestorCandidate, false)) {
PsiElement statement = RefactoringUtil.getParentStatement(ancestorCandidate, false);
if (allOccurrences.stream().allMatch(occurrence ->
PsiTreeUtil.isAncestor(statement, occurrence, false) &&
(!PsiTreeUtil.isAncestor(ancestorCandidate, occurrence, false) ||
ReorderingUtils.canExtract(ancestorCandidate, occurrence) == ThreeState.NO))) {
return firstOccurrence;
}
}
}
if (anchor instanceof PsiTryStatement && firstOccurrence != null) {
@@ -0,0 +1,6 @@
class A {
public Integer incrementInteger(Number n) {
int temp = (Integer) n + 1;
return n instanceof Integer ? temp : temp -1 ;
}
}
@@ -0,0 +1,5 @@
class A {
public Integer incrementInteger(Number n) {
return n instanceof Integer ? <selection>(Integer)n + 1</selection> : (Integer) n + 1 -1 ;
}
}
@@ -244,6 +244,10 @@ public class IntroduceVariableTest extends LightCodeInsightTestCase {
}
fail("Should not be able to perform refactoring");
}
public void testTernaryBothBranches() {
doTest(new MockIntroduceVariableHandler("temp", true, false, false, "int"));
}
public void testIfConditionAndChain() {
doTest(new MockIntroduceVariableHandler("temp", true, false, false, CommonClassNames.JAVA_LANG_STRING));