Java: Fixed handling of complex constant expressions in parametrized duplicates matching (IDEA-179924)

java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java

@@ -313,7 +313,14 @@ public class DuplicatesFinder {
     final ASTNode node1 = pattern.getNode();
     final ASTNode node2 = candidate.getNode();
     if (node1 == null || node2 == null) return false;
-    return node1.getElementType() == node2.getElementType();
+    if (node1.getElementType() != node2.getElementType()) return false;
+    if (pattern instanceof PsiUnaryExpression) {
+      return ((PsiUnaryExpression)pattern).getOperationTokenType() == ((PsiUnaryExpression)candidate).getOperationTokenType();
+    }
+    if (pattern instanceof PsiPolyadicExpression) {
+      return ((PsiPolyadicExpression)pattern).getOperationTokenType() == ((PsiPolyadicExpression)candidate).getOperationTokenType();
+    }
+    return true;
   }
 
   private boolean matchPattern(PsiElement pattern,

java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExtractableExpressionPart.java

@@ -72,18 +72,18 @@ public class ExtractableExpressionPart {
   static ExtractableExpressionPart match(@NotNull PsiExpression expression,
                                          @NotNull List<PsiElement> scope,
                                          @Nullable ComplexityHolder complexityHolder) {
-    if (PsiUtil.isConstantExpression(expression)) {
-      if (PsiTreeUtil.findChildOfType(expression, PsiJavaCodeReferenceElement.class) != null) {
-        // Avoid coincidental replacement of equal expressions containing different constant fields
-        // E.g. don't count as equal (Foo.A + 1) and (Bar.B - 2) because most likely it's not what the users expect
-        return null;
-      }
-      return matchConstant(expression);
-    }
     if (expression instanceof PsiReferenceExpression) {
       return matchVariable((PsiReferenceExpression)expression, scope);
     }
-    if (complexityHolder != null && complexityHolder.isAcceptableExpression(expression)) {
+    boolean isConstant = PsiUtil.isConstantExpression(expression);
+    if (isConstant) {
+      // Avoid replacement of coincidentally equal expressions containing different constant fields
+      // E.g. don't count as equal values expressions like (Foo.A + 1) and (Bar.B - 2)
+      if (PsiTreeUtil.findChildOfType(expression, PsiJavaCodeReferenceElement.class) == null) {
+        return matchConstant(expression);
+      }
+    }
+    if (complexityHolder != null && (isConstant || complexityHolder.isAcceptableExpression(expression))) {
       PsiType type = expression.getType();
       if (type != null && !PsiType.VOID.equals(type)) {
         return new ExtractableExpressionPart(expression, null, null, type);

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureEqualConstExprFolding.java

@@ -0,0 +1,16 @@
+class C {
+    public static final int A = 4 - 1;
+    public static final int B = 4 + 2;
+
+    void foo(int x) {
+        <selection>
+        if(x == A + 1)
+            bar(A + 1);
+        </selection>
+
+        if(x == B - 2)
+            bar(B - 2);
+    }
+
+    void bar(int n) {}
+}

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureEqualConstExprFolding_after.java

@@ -0,0 +1,19 @@
+class C {
+    public static final int A = 4 - 1;
+    public static final int B = 4 + 2;
+
+    void foo(int x) {
+
+        newMethod(x, A + 1);
+
+
+        newMethod(x, B - 2);
+    }
+
+    private void newMethod(int x, int i) {
+        if (x == i)
+            bar(i);
+    }
+
+    void bar(int n) {}
+}

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureLongConstExprFolding.java

@@ -0,0 +1,18 @@
+class C {
+    public static final int A = 1, B = 2, C = 3, D = 4, E = 5;
+
+    void foo(int x) {
+        <selection>
+        if (x >= A + B + C + D + E + 1)
+            bar("A" + "B" + "C" + "D" + "E" + 1);
+        </selection>
+
+        if (x >= A + B + C + D + E + 2)
+            bar("A" + "B" + "C" + "D" + "E" + 2);
+
+        if (x <= A + B + C + D + E + 2)
+            bar("A" + "B" + "C" + "D" + "E" + 2);
+    }
+
+    void bar(String s) {}
+}

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureLongConstExprFolding_after.java

@@ -0,0 +1,21 @@
+class C {
+    public static final int A = 1, B = 2, C = 3, D = 4, E = 5;
+
+    void foo(int x) {
+
+        newMethod(x, 1, "A" + "B" + "C" + "D" + "E" + 1);
+
+
+        newMethod(x, 2, "A" + "B" + "C" + "D" + "E" + 2);
+
+        if (x <= A + B + C + D + E + 2)
+            bar("A" + "B" + "C" + "D" + "E" + 2);
+    }
+
+    private void newMethod(int x, int i, String s) {
+        if (x >= A + B + C + D + E + i)
+            bar(s);
+    }
+
+    void bar(String s) {}
+}

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

@@ -815,6 +815,14 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
     doDuplicatesTest();
   }
 
+  public void testSuggestChangeSignatureEqualConstExprFolding() throws Exception {
+    doDuplicatesTest();
+  }
+
+  public void testSuggestChangeSignatureLongConstExprFolding() throws Exception {
+    doDuplicatesTest();
+  }
+
   public void testSuggestChangeSignatureWithChangedParameterName() throws Exception {
     configureByFile(BASE_PATH + getTestName(false) + ".java");
     boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, false, "p");