Redundant record constructors: check redundant field assignments in compact; more tests

Review ID: IDEA-CR-57720

GitOrigin-RevId: 6a76b3d39cee8cb01ef3f07db17a5f5ee7be88b0

java/java-impl/src/com/intellij/codeInspection/RedundantRecordConstructorInspection.java

@@ -13,7 +13,7 @@ import com.intellij.psi.util.PsiUtil;
 import com.intellij.util.ArrayUtil;
 import com.intellij.util.ObjectUtils;
 import com.intellij.util.containers.ContainerUtil;
-import com.siyeh.ig.psiutils.ControlFlowUtils;
+import com.siyeh.ig.psiutils.CommentTracker;
 import com.siyeh.ig.psiutils.ExpressionUtils;
 import one.util.streamex.EntryStream;
 import org.jetbrains.annotations.Nls;
@@ -68,7 +68,22 @@ public class RedundantRecordConstructorInspection extends AbstractBaseJavaLocalI
 
       private void checkCompact(PsiMethod ctor) {
         PsiCodeBlock body = ctor.getBody();
-        if (ControlFlowUtils.isEmptyCodeBlock(body) && ctor.getModifierList().getAnnotations().length == 0 &&
+        if (body == null) return;
+        PsiStatement[] statements = body.getStatements();
+        if (statements.length > 0) {
+          PsiParameter[] parameters = ctor.getParameterList().getParameters();
+          PsiRecordComponent[] components = Objects.requireNonNull(ctor.getContainingClass()).getRecordComponents();
+          int count = getAssignedComponentsCount(components, parameters, statements);
+          if (count < statements.length) {
+            for (int i = statements.length - count; i < statements.length; i++) {
+              holder.registerProblem(statements[i],
+                                     InspectionsBundle.message("inspection.redundant.record.constructor.statement.message"),
+                                     ProblemHighlightType.LIKE_UNUSED_SYMBOL, new DeleteElementFix(statements[i]));
+            }
+            return;
+          }
+        }
+        if (ctor.getModifierList().getAnnotations().length == 0 &&
             ctor.getDocComment() == null) {
           holder.registerProblem(Objects.requireNonNull(ctor.getNameIdentifier()),
                                  InspectionsBundle.message("inspection.redundant.record.constructor.compact.message"),
@@ -81,6 +96,7 @@ public class RedundantRecordConstructorInspection extends AbstractBaseJavaLocalI
   private static int getAssignedComponentsCount(PsiRecordComponent @NotNull [] components,
                                                 PsiParameter @NotNull [] parameters,
                                                 PsiStatement @NotNull [] statements) {
+    assert parameters.length == components.length;
     Set<PsiRecordComponent> unprocessed = new HashSet<>(Arrays.asList(components));
     int i = statements.length - 1;
     while (i >= 0 && !unprocessed.isEmpty()) {
@@ -132,18 +148,20 @@ public class RedundantRecordConstructorInspection extends AbstractBaseJavaLocalI
         resultText.append(child.getText());
       }
       boolean skipStatements = false;
+      CommentTracker ct = new CommentTracker();
       for (PsiElement child : body.getChildren()) {
         if (child == firstStatementToDelete) {
           skipStatements = true;
         }
-        if (skipStatements &&
-            (child instanceof PsiStatement || child instanceof PsiWhiteSpace && !(child.getPrevSibling() instanceof PsiComment))) {
+        if (skipStatements && child.getNextSibling() != null) {
+          ct.grabComments(child);
           continue;
         }
         resultText.append(child.getText());
       }
       PsiMethod compactCtor = JavaPsiFacade.getElementFactory(project).createMethodFromText(resultText.toString(), ctor);
-      ctor.replace(compactCtor);
+      PsiMethod result = (PsiMethod)ctor.replace(compactCtor);
+      ct.insertCommentsBefore(Objects.requireNonNull(Objects.requireNonNull(result.getBody()).getRBrace()));
     }
   }
 }

java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonical.java

@@ -1,3 +1,9 @@
 // "Remove constructor" "true" 
 record Rec(int x, int y) {
+    // 1
+    /*2*/
+    /*3*/
+    // 4
+    /*5*/
+    //6
 }

java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonicalAssignSame.java

@@ -2,5 +2,9 @@
 record Rec(int x, int y) {
     public Rec {
         this.x = y;
+        // 1
+        /*2*/
+        //3
+        //4
     }
 }

java/java-tests/testData/inspection/redundantRecordConstructor/afterCompactAssignment.java

@@ -0,0 +1,3 @@
+// "Remove constructor" "true"
+record Foo(int x) {
+}

java/java-tests/testData/inspection/redundantRecordConstructor/afterCompactCheckAndAssignment.java

@@ -0,0 +1,8 @@
+// "Remove statement" "true"
+record Foo(int x) {
+  public Foo {
+    if (x < 0) {
+      throw new IllegalArgumentException()
+    }
+  }
+}

java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonical.java

@@ -1,7 +1,10 @@
 // "Remove constructor" "true" 
 record Rec(int x, int y) {
   public R<caret>ec(int x, int y) {
-    this.x = x;
-    this.y = y;
+    this.x = x; // 1
+    /*2*/
+    this.y =/*3*/ y; // 4
+    /*5*/
+    //6
   }
 }

java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignSame.java

@@ -2,6 +2,8 @@
 record Rec(int x, int y) {
   public Rec(int x<caret>, int y) {
     this.x = y;
-    this.y = y;
+    // 1
+    this.y /*2*/= y; //3
+    //4
   }
 }

java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalReturn.java

@@ -0,0 +1,12 @@
+// "Remove constructor" "false" 
+record Rec(int x, int y) {
+  public R<caret>ec(int x, int y) {
+    if (x < 0) {
+      this.x = 0;
+      this.y = 0;
+      return;
+    }
+    this.x = x;
+    this.y = y;
+  }
+}

java/java-tests/testData/inspection/redundantRecordConstructor/beforeCompactAssignment.java

@@ -0,0 +1,6 @@
+// "Remove constructor" "true"
+record Foo(int x) {
+  public <caret>Foo {
+    this.x = x;
+  }
+}

java/java-tests/testData/inspection/redundantRecordConstructor/beforeCompactCheckAndAssignment.java

@@ -0,0 +1,9 @@
+// "Remove statement" "true"
+record Foo(int x) {
+  public Foo {
+    if (x < 0) {
+      throw new IllegalArgumentException()
+    }
+    this.<caret>x = x;
+  }
+}

platform/analysis-api/resources/messages/InspectionsBundle.properties

@@ -1118,6 +1118,7 @@ action.DumbAware.SingleInspectionProfilePanel.description.reset.to.empty=Reset t
 
 inspection.redundant.record.constructor.description=Redundant record constructor
 inspection.redundant.record.constructor.compact.message=Redundant compact constructor
+inspection.redundant.record.constructor.statement.message=Redundant field assignment in compact constructor
 inspection.redundant.record.constructor.canonical.message=Redundant canonical constructor
 inspection.redundant.record.constructor.can.be.compact.message=Canonical constructor can be converted to compact form
 inspection.redundant.record.constructor.fix.family.name=Convert canonical constructor to compact form