extract method: allow to move constant expressions to the call place

GitOrigin-RevId: 36724921cb14c49bec12b529500856c5368af504

java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodAnalyzer.kt

@@ -15,6 +15,7 @@ import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.findUs
 import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.getExpressionType
 import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.guessName
 import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.hasExplicitModifier
+import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.haveReferenceToScope
 import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.inputParameterOf
 import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.normalizedAnchor
 import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.uniqueNameOf
@@ -39,11 +40,17 @@ fun findExtractOptions(elements: List<PsiElement>): ExtractOptions {
 
   val expression = elements.singleOrNull() as? PsiExpression
 
+
+  fun canExtractStatementsFromScope(statements: List<PsiStatement>, scope: List<PsiElement>): Boolean {
+    return ExtractMethodHelper.areSemanticallySame(statements) && !haveReferenceToScope(statements, scope)
+  }
+
   //TODO use correct error messages
   val dataOutput = when {
     expression != null  -> ExpressionOutput(getExpressionType(expression), null, listOf(expression), CodeFragmentAnalyzer.inferNullability(listOf(expression)))
     variableData is VariableOutput -> when {
-      ! ExtractMethodHelper.areSemanticallySame(flowOutput.statements) && flowOutput is ConditionalFlow -> throw PrepareFailedException("Out var and different flow statements", flowOutput.statements.first())
+      flowOutput is ConditionalFlow && ! canExtractStatementsFromScope(flowOutput.statements, elements)
+        -> throw PrepareFailedException("Out var and different flow statements", flowOutput.statements.first())
       variableData.nullability != Nullability.NOT_NULL && flowOutput is ConditionalFlow -> throw PrepareFailedException("Nullable out var and branching", variableData.variable)
       flowOutput is ConditionalFlow -> variableData.copy(nullability = Nullability.NULLABLE)
       else -> variableData

java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodHelper.kt

@@ -180,12 +180,14 @@ object ExtractMethodHelper {
     if (statements.isEmpty()) return true
     if (! areSame(statements)) return false
     val returnExpressions = statements.mapNotNull { statement -> (statement as? PsiReturnStatement)?.returnValue }
-    return returnExpressions.none { expression -> hasReference(expression) }
-    /* TODO it's also possible to extract single return expression with external references */
+    return returnExpressions.all { expression -> PsiUtil.isConstantExpression(expression) || expression.type == PsiType.NULL }
   }
 
-  private fun hasReference(element: PsiElement): Boolean {
-    return PsiTreeUtil.findChildOfType(element, PsiJavaCodeReferenceElement::class.java, false) != null
+  fun haveReferenceToScope(elements: List<PsiElement>, scope: List<PsiElement>): Boolean {
+    val scopeRange = TextRange(scope.first().textRange.startOffset, scope.last().textRange.endOffset)
+    return elements.asSequence()
+      .flatMap { PsiTreeUtil.findChildrenOfAnyType(it, false, PsiJavaCodeReferenceElement::class.java).asSequence() }
+      .mapNotNull { reference -> reference.resolve() }
+      .any{ referencedElement -> referencedElement.textRange in scopeRange }
   }
-
 }

java/java-tests/testData/refactoring/extractMethodNew/DontExtractCustomFinalObjects.java

@@ -0,0 +1,12 @@
+import java.io.InputStream;
+
+class Test {
+    InputStream test(boolean condition){
+        final InputStream stream = System.in;
+        <selection>int x = 42;
+        if (condition) return stream;
+        if (!condition) return stream;</selection>
+        System.out.println(x);
+        return null;
+    }
+}

java/java-tests/testData/refactoring/extractMethodNew/DontExtractLocalConstant.java

@@ -0,0 +1,11 @@
+class Test {
+    final String f1 = "field";
+    String test(boolean condition){
+        <selection>int x = 42;
+        final String f2 = "variable";
+        if (condition) return f1 + f2 + "literal";
+        if (!condition) return f1+f2+"literal";</selection>
+        System.out.println(x);
+        return "default";
+    }
+}

java/java-tests/testData/refactoring/extractMethodNew/ExtractConstantExpressions.java

@@ -0,0 +1,11 @@
+class Test {
+    final String f1 = "field";
+    String test(boolean condition){
+        final String f2 = "variable";
+        <selection>int x = 42;
+        if (condition) return f1 + f2 + "literal";
+        if (!condition) return f1+f2+"literal";</selection>
+        System.out.println(x);
+        return "default";
+    }
+}

java/java-tests/testData/refactoring/extractMethodNew/ExtractConstantExpressions_after.java

@@ -0,0 +1,20 @@
+import org.jetbrains.annotations.Nullable;
+
+class Test {
+    final String f1 = "field";
+    String test(boolean condition){
+        final String f2 = "variable";
+        Integer x = newMethod(condition, f2);
+        if (x == null) return f1 + f2 + "literal";
+        System.out.println(x);
+        return "default";
+    }
+
+    @Nullable
+    private Integer newMethod(boolean condition, String f2) {
+        int x = 42;
+        if (condition) return null;
+        if (!condition) return null;
+        return x;
+    }
+}

java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodNewTest.java

@@ -735,6 +735,26 @@ public class ExtractMethodNewTest extends LightJavaCodeInsightTestCase {
     }
   }
 
+  public void testExtractConstantExpressions() throws Exception {
+      doTest();
+  }
+
+  public void testDontExtractLocalConstant() throws Exception {
+    try {
+      doTest();
+      fail("Should fail if expression is linked to the scope");
+    } catch (PrepareFailedException e){
+    }
+  }
+
+  public void testDontExtractCustomFinalObjects() throws Exception {
+    try {
+      doTest();
+      fail("Should fail if expression contains mutable object");
+    } catch (PrepareFailedException e){
+    }
+  }
+
   public void testReturnStatementFolding() throws Exception {
     doTest();
   }