IDEA-251722 - remove plain hashCode method during class-to-record conversion
authorIlyas Selimov <ilyas.selimov@jetbrains.com>
Fri, 3 Dec 2021 06:55:33 +0000 (13:55 +0700)
committerintellij-monorepo-bot <intellij-monorepo-bot-no-reply@jetbrains.com>
Fri, 3 Dec 2021 07:04:39 +0000 (07:04 +0000)
GitOrigin-RevId: 1db63c1bdeec14f3ec6ef2aced3e089572633dcc

java/java-impl-inspections/src/com/intellij/codeInspection/classCanBeRecord/ConvertToRecordFix.java
java/java-impl-inspections/src/com/intellij/codeInspection/classCanBeRecord/ConvertToRecordProcessor.java
java/java-tests/testData/inspection/classCanBeRecord/afterObjectsHashNegative.java [new file with mode: 0644]
java/java-tests/testData/inspection/classCanBeRecord/afterObjectsHashPositive.java [new file with mode: 0644]
java/java-tests/testData/inspection/classCanBeRecord/afterOldStyleHashNegative.java [new file with mode: 0644]
java/java-tests/testData/inspection/classCanBeRecord/afterOldStyleHashPositive.java [new file with mode: 0644]
java/java-tests/testData/inspection/classCanBeRecord/beforeObjectsHashNegative.java [new file with mode: 0644]
java/java-tests/testData/inspection/classCanBeRecord/beforeObjectsHashPositive.java [new file with mode: 0644]
java/java-tests/testData/inspection/classCanBeRecord/beforeOldStyleHashNegative.java [new file with mode: 0644]
java/java-tests/testData/inspection/classCanBeRecord/beforeOldStyleHashPositive.java [new file with mode: 0644]

index 89444770e683732394d498e0d664d9af99561beb..89830dd8800fcf2b4b86c8fbd10efb001ab2df35 100644 (file)
@@ -22,6 +22,7 @@ import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.MultiMap;
 import com.siyeh.ig.InspectionGadgetsFix;
 import com.siyeh.ig.callMatcher.CallMatcher;
+import com.siyeh.ig.psiutils.MethodUtils;
 import com.siyeh.ig.psiutils.TypeUtils;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -116,6 +117,9 @@ public class ConvertToRecordFix extends InspectionGadgetsFix {
     private final MultiMap<PsiField, FieldAccessorCandidate> myFieldAccessors = new MultiMap<>(new LinkedHashMap<>());
     private final List<PsiMethod> myOrdinaryMethods = new SmartList<>();
     private final List<RecordConstructorCandidate> myConstructors = new SmartList<>();
+    
+    private PsiMethod myEqualsMethod;
+    private PsiMethod myHashCodeMethod;
 
     private Map<PsiField, FieldAccessorCandidate> myFieldAccessorsCache;
 
@@ -153,6 +157,16 @@ public class ConvertToRecordFix extends InspectionGadgetsFix {
       return myConstructors.size() == 1 ? myConstructors.get(0).myConstructor : null;
     }
 
+    @Nullable
+    PsiMethod getEqualsMethod() {
+      return myEqualsMethod;
+    }
+
+    @Nullable
+    PsiMethod getHashCodeMethod() {
+      return myHashCodeMethod;
+    }
+
     private boolean isValid() {
       if (myConstructors.size() > 1) return false;
       if (myConstructors.size() == 1) {
@@ -190,6 +204,14 @@ public class ConvertToRecordFix extends InspectionGadgetsFix {
           myConstructors.add(new RecordConstructorCandidate(method, myFieldAccessors.keySet()));
           continue;
         }
+        if (MethodUtils.isEquals(method)) {
+          myEqualsMethod = method;
+          continue;
+        }
+        if (MethodUtils.isHashCode(method)) {
+          myHashCodeMethod = method;
+          continue;
+        }
         if (!throwsOnlyUncheckedExceptions(method)) {
           myOrdinaryMethods.add(method);
           continue;
index 53ae6a31edae732126974ba29dc01a901967a111..0026f9a8d6f20103eeb564e1d8be93b2c75064e5 100644 (file)
@@ -14,6 +14,7 @@ import com.intellij.psi.*;
 import com.intellij.psi.codeStyle.CodeStyleManager;
 import com.intellij.psi.javadoc.PsiDocComment;
 import com.intellij.psi.javadoc.PsiDocTag;
+import com.intellij.psi.tree.IElementType;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.refactoring.BaseRefactoringProcessor;
 import com.intellij.refactoring.rename.RenameProcessor;
@@ -28,6 +29,7 @@ import com.intellij.util.*;
 import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.MultiMap;
 import com.intellij.util.containers.SmartHashSet;
+import com.siyeh.ig.callMatcher.CallMatcher;
 import one.util.streamex.StreamEx;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -219,9 +221,10 @@ public class ConvertToRecordProcessor extends BaseRefactoringProcessor {
       }
       nextElement = nextElement.getNextSibling();
     }
-
+    List<PsiMethod> redundantObjectMethods = findRedundantObjectMethods();
     PsiClass result = (PsiClass)psiClass.replace(recordBuilder.build());
     tryToCompactCanonicalCtor(result);
+    removeRedundantObjectMethods(result, redundantObjectMethods);
     generateJavaDocForDocumentedFields(result);
     CodeStyleManager.getInstance(myProject).reformat(result);
   }
@@ -237,6 +240,118 @@ public class ConvertToRecordProcessor extends BaseRefactoringProcessor {
     }
   }
 
+  @NotNull
+  private List<PsiMethod> findRedundantObjectMethods() {
+    PsiMethod equalsMethod = myRecordCandidate.getEqualsMethod();
+    PsiMethod hashCodeMethod = myRecordCandidate.getHashCodeMethod();
+    if (equalsMethod == null && hashCodeMethod == null) return Collections.emptyList();
+    List<PsiMethod> result = new SmartList<>();
+    Set<PsiField> fields = myRecordCandidate.getFieldAccessors().keySet();
+    // todo for equals method
+    if (hashCodeMethod != null) {
+      var hashCodeVisitor = new HashCodeVisitor(fields);
+      hashCodeMethod.accept(hashCodeVisitor);
+      if (hashCodeVisitor.myNonVisitedFields.isEmpty()) {
+        result.add(hashCodeMethod);
+      }
+    }
+    return result;
+  }
+
+  private static class HashCodeVisitor extends JavaRecursiveElementWalkingVisitor {
+    private final CallMatcher OBJECTS_CALL = CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_OBJECTS, "hash", "hashCode");
+    private final CallMatcher FLOAT_CALL = CallMatcher.staticCall(CommonClassNames.JAVA_LANG_FLOAT, "floatToIntBits");
+    private final CallMatcher DOUBLE_CALL = CallMatcher.staticCall(CommonClassNames.JAVA_LANG_DOUBLE, "doubleToLongBits");
+
+    private final CallMatcher INSTANCE_CALL = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "hashCode");
+
+    private final Set<PsiField> myNonVisitedFields;
+
+    private HashCodeVisitor(@NotNull Set<PsiField> fields) {
+      myNonVisitedFields = new SmartHashSet<>(fields);
+    }
+
+    @Override
+    public void visitReferenceExpression(PsiReferenceExpression expression) {
+      PsiField field = ObjectUtils.tryCast(expression.resolve(), PsiField.class);
+      if (field == null) return;
+      PsiType fieldType = field.getType();
+      if (PsiType.CHAR.equals(fieldType) || PsiType.SHORT.equals(fieldType)) {
+        PsiElement parent = PsiTreeUtil.skipParentsOfType(expression, PsiParenthesizedExpression.class);
+        if (parent instanceof PsiTypeCastExpression) {
+          PsiTypeElement castType = ((PsiTypeCastExpression)parent).getCastType();
+          if (castType != null && PsiType.INT.equals(castType.getType())) {
+            myNonVisitedFields.remove(field);
+          }
+        }
+      }
+      else {
+        myNonVisitedFields.remove(field);
+      }
+    }
+
+    @Override
+    public void visitMethodCallExpression(PsiMethodCallExpression expression) {
+      if (OBJECTS_CALL.test(expression)) {
+        for (PsiExpression argument : expression.getArgumentList().getExpressions()) {
+          PsiReferenceExpression fieldRef = ObjectUtils.tryCast(argument, PsiReferenceExpression.class);
+          if (fieldRef == null || !myNonVisitedFields.remove(fieldRef.resolve())) {
+            stopWalking();
+            return;
+          }
+        }
+        return;
+      }
+
+      PsiType expectedType = null;
+      if (FLOAT_CALL.test(expression)) {
+        expectedType = PsiType.FLOAT;
+      }
+      else if (DOUBLE_CALL.test(expression)) {
+        expectedType = PsiType.DOUBLE;
+      }
+      if (expectedType != null) {
+        PsiExpression[] expressions = expression.getArgumentList().getExpressions();
+        if (expressions.length == 1) {
+          PsiReferenceExpression refExpr = ObjectUtils.tryCast(expressions[0], PsiReferenceExpression.class);
+          if (refExpr != null) {
+            PsiField field = ObjectUtils.tryCast(refExpr.resolve(), PsiField.class);
+            if (field != null && expectedType.equals(field.getType())) {
+              myNonVisitedFields.remove(field);
+              return;
+            }
+          }
+        }
+        stopWalking();
+        return;
+      }
+
+      if (INSTANCE_CALL.test(expression)) {
+        PsiExpression qualifier = expression.getMethodExpression().getQualifierExpression();
+        PsiReferenceExpression fieldRef = ObjectUtils.tryCast(qualifier, PsiReferenceExpression.class);
+        if (fieldRef != null) {
+          PsiField field = ObjectUtils.tryCast(fieldRef.resolve(), PsiField.class);
+          if (field != null) {
+            myNonVisitedFields.remove(field);
+            return;
+          }
+        }
+      }
+      stopWalking();
+    }
+
+    @Override
+    public void visitPolyadicExpression(PsiPolyadicExpression expression) {
+      super.visitPolyadicExpression(expression);
+      IElementType operationType = expression.getOperationTokenType();
+      if (!JavaTokenType.PLUS.equals(operationType) && !JavaTokenType.MINUS.equals(operationType) &&
+          !JavaTokenType.ASTERISK.equals(operationType) && !JavaTokenType.GTGTGT.equals(operationType) &&
+          !JavaTokenType.EQEQ.equals(operationType) && !JavaTokenType.NE.equals(operationType)) {
+        stopWalking();
+      }
+    }
+  }
+
   private static void tryToCompactCanonicalCtor(@NotNull PsiClass record) {
     PsiMethod canonicalCtor = ArrayUtil.getFirstElement(record.getConstructors());
     if (canonicalCtor != null) {
@@ -254,6 +369,12 @@ public class ConvertToRecordProcessor extends BaseRefactoringProcessor {
     }
   }
 
+  private static void removeRedundantObjectMethods(@NotNull PsiClass record, @NotNull List<PsiMethod> redundantObjectMethods) {
+    redundantObjectMethods.stream().map(method -> record.findMethodBySignature(method, false))
+      .filter(Objects::nonNull)
+      .forEach(PsiMethod::delete);
+  }
+
   private void generateJavaDocForDocumentedFields(@NotNull PsiClass record) {
     Map<String, String> comments = new LinkedHashMap<>();
     for (PsiField field : myRecordCandidate.getFieldAccessors().keySet()) {
diff --git a/java/java-tests/testData/inspection/classCanBeRecord/afterObjectsHashNegative.java b/java/java-tests/testData/inspection/classCanBeRecord/afterObjectsHashNegative.java
new file mode 100644 (file)
index 0000000..cd6f588
--- /dev/null
@@ -0,0 +1,10 @@
+// "Convert to a record" "true"
+import java.util.Objects;
+
+record Test(boolean booleanValue, char charValue, String stringValue, long longValue, float floatValue,
+            double doubleValue, double[] arrayValue) {
+    @Override
+    public int hashCode() {
+        return Objects.hash(booleanValue, stringValue, longValue, floatValue, doubleValue, arrayValue);
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/inspection/classCanBeRecord/afterObjectsHashPositive.java b/java/java-tests/testData/inspection/classCanBeRecord/afterObjectsHashPositive.java
new file mode 100644 (file)
index 0000000..b8c7986
--- /dev/null
@@ -0,0 +1,5 @@
+// "Convert to a record" "true"
+
+record Test(boolean booleanValue, char charValue, String stringValue, long longValue, float floatValue,
+            double doubleValue, double[] arrayValue) {
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/inspection/classCanBeRecord/afterOldStyleHashNegative.java b/java/java-tests/testData/inspection/classCanBeRecord/afterOldStyleHashNegative.java
new file mode 100644 (file)
index 0000000..949266f
--- /dev/null
@@ -0,0 +1,11 @@
+// "Convert to a record" "true"
+record Test(double[] arrayValue) {
+    @Override
+    public int hashCode() {
+        int result = 17;
+
+        result = 37 * result + Arrays.hashCode(arrayValue);
+
+        return result;
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/inspection/classCanBeRecord/afterOldStyleHashPositive.java b/java/java-tests/testData/inspection/classCanBeRecord/afterOldStyleHashPositive.java
new file mode 100644 (file)
index 0000000..e018771
--- /dev/null
@@ -0,0 +1,4 @@
+// "Convert to a record" "true"
+record Test(boolean booleanValue, char charValue, String stringValue, long longValue, float floatValue,
+            double doubleValue, double[] arrayValue) {
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/inspection/classCanBeRecord/beforeObjectsHashNegative.java b/java/java-tests/testData/inspection/classCanBeRecord/beforeObjectsHashNegative.java
new file mode 100644 (file)
index 0000000..c6c68be
--- /dev/null
@@ -0,0 +1,17 @@
+// "Convert to a record" "true"
+import java.util.Objects;
+
+class <caret>Test {
+  final boolean booleanValue;
+  final  char charValue;
+  final String stringValue;
+  final long longValue;
+  final float floatValue;
+  final double doubleValue;
+  final double[] arrayValue;
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(booleanValue, stringValue, longValue, floatValue, doubleValue, arrayValue);
+  }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/inspection/classCanBeRecord/beforeObjectsHashPositive.java b/java/java-tests/testData/inspection/classCanBeRecord/beforeObjectsHashPositive.java
new file mode 100644 (file)
index 0000000..e8aea86
--- /dev/null
@@ -0,0 +1,17 @@
+// "Convert to a record" "true"
+import java.util.Objects;
+
+class <caret>Test {
+  final boolean booleanValue;
+  final  char charValue;
+  final String stringValue;
+  final long longValue;
+  final float floatValue;
+  final double doubleValue;
+  final double[] arrayValue;
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(booleanValue, charValue, stringValue, longValue, floatValue, doubleValue, arrayValue);
+  }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/inspection/classCanBeRecord/beforeOldStyleHashNegative.java b/java/java-tests/testData/inspection/classCanBeRecord/beforeOldStyleHashNegative.java
new file mode 100644 (file)
index 0000000..dedadc0
--- /dev/null
@@ -0,0 +1,13 @@
+// "Convert to a record" "true"
+class <caret>Test {
+  final double[] arrayValue;
+
+  @Override
+  public int hashCode() {
+    int result = 17;
+
+    result = 37 * result + Arrays.hashCode(arrayValue);
+
+    return result;
+  }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/inspection/classCanBeRecord/beforeOldStyleHashPositive.java b/java/java-tests/testData/inspection/classCanBeRecord/beforeOldStyleHashPositive.java
new file mode 100644 (file)
index 0000000..87e311c
--- /dev/null
@@ -0,0 +1,26 @@
+// "Convert to a record" "true"
+class <caret>Test {
+  final boolean booleanValue;
+  final char charValue;
+  final String stringValue;
+  final long longValue;
+  final float floatValue;
+  final double doubleValue;
+  final double[] arrayValue;
+
+  @Override
+  public int hashCode() {
+    int result = 17;
+
+    result = 37 * result + (booleanValue ? 1 : 0);
+    result = 37 * result + (int) charValue;
+    result = 37 * result + (stringValue == null ? 0 : stringValue.hashCode());
+    result = 37 * result + (int) (longValue - (longValue >>> 32));
+    result = 37 * result + Float.floatToIntBits(floatValue);
+    result = 37 * result + (int) (Double.doubleToLongBits(doubleValue) - (Double.doubleToLongBits(doubleValue) >>> 32));
+
+    result = 37 * result + arrayValue.hashCode();
+
+    return result;
+  }
+}
\ No newline at end of file