diff --git a/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java b/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java index 97532880b769..aeccd35d1298 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java @@ -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 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) { diff --git a/java/java-tests/testData/refactoring/introduceVariable/TernaryBothBranches.after.java b/java/java-tests/testData/refactoring/introduceVariable/TernaryBothBranches.after.java new file mode 100644 index 000000000000..eaf54f26e32d --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceVariable/TernaryBothBranches.after.java @@ -0,0 +1,6 @@ +class A { + public Integer incrementInteger(Number n) { + int temp = (Integer) n + 1; + return n instanceof Integer ? temp : temp -1 ; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceVariable/TernaryBothBranches.java b/java/java-tests/testData/refactoring/introduceVariable/TernaryBothBranches.java new file mode 100644 index 000000000000..c1eb85934a86 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceVariable/TernaryBothBranches.java @@ -0,0 +1,5 @@ +class A { + public Integer incrementInteger(Number n) { + return n instanceof Integer ? (Integer)n + 1 : (Integer) n + 1 -1 ; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java index fc44ddd56d3e..a27ed183c09c 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java @@ -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));