diff --git a/java/java-impl/src/com/intellij/codeInspection/RedundantRecordConstructorInspection.java b/java/java-impl/src/com/intellij/codeInspection/RedundantRecordConstructorInspection.java index 81129297205d..5e377e55981b 100644 --- a/java/java-impl/src/com/intellij/codeInspection/RedundantRecordConstructorInspection.java +++ b/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 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())); } } } diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonical.java b/java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonical.java index 26fadef81efe..b5c51ee009ff 100644 --- a/java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonical.java +++ b/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 } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonicalAssignSame.java b/java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonicalAssignSame.java index 4f07c3f10377..1779c71170b5 100644 --- a/java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonicalAssignSame.java +++ b/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 } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/afterCompactAssignment.java b/java/java-tests/testData/inspection/redundantRecordConstructor/afterCompactAssignment.java new file mode 100644 index 000000000000..d489a4be9307 --- /dev/null +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/afterCompactAssignment.java @@ -0,0 +1,3 @@ +// "Remove constructor" "true" +record Foo(int x) { +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/afterCompactCheckAndAssignment.java b/java/java-tests/testData/inspection/redundantRecordConstructor/afterCompactCheckAndAssignment.java new file mode 100644 index 000000000000..e6d9f4dcb024 --- /dev/null +++ b/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() + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonical.java b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonical.java index 3cce217ebb24..d925950b0f09 100644 --- a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonical.java +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonical.java @@ -1,7 +1,10 @@ // "Remove constructor" "true" record Rec(int x, int y) { public Rec(int x, int y) { - this.x = x; - this.y = y; + this.x = x; // 1 + /*2*/ + this.y =/*3*/ y; // 4 + /*5*/ + //6 } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignSame.java b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignSame.java index 25d320999f10..684769b47f31 100644 --- a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignSame.java +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignSame.java @@ -2,6 +2,8 @@ record Rec(int x, int y) { public Rec(int x, int y) { this.x = y; - this.y = y; + // 1 + this.y /*2*/= y; //3 + //4 } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalReturn.java b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalReturn.java new file mode 100644 index 000000000000..0babfaa43094 --- /dev/null +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalReturn.java @@ -0,0 +1,12 @@ +// "Remove constructor" "false" +record Rec(int x, int y) { + public Rec(int x, int y) { + if (x < 0) { + this.x = 0; + this.y = 0; + return; + } + this.x = x; + this.y = y; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCompactAssignment.java b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCompactAssignment.java new file mode 100644 index 000000000000..f2f9a8ade588 --- /dev/null +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCompactAssignment.java @@ -0,0 +1,6 @@ +// "Remove constructor" "true" +record Foo(int x) { + public Foo { + this.x = x; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCompactCheckAndAssignment.java b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCompactCheckAndAssignment.java new file mode 100644 index 000000000000..e43aecdda614 --- /dev/null +++ b/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.x = x; + } +} \ No newline at end of file diff --git a/platform/analysis-api/resources/messages/InspectionsBundle.properties b/platform/analysis-api/resources/messages/InspectionsBundle.properties index 9b0f5fadac41..dcda93247e8f 100644 --- a/platform/analysis-api/resources/messages/InspectionsBundle.properties +++ b/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 \ No newline at end of file