From 22b7af6762b2665758e22e3d345c8e2baca654c5 Mon Sep 17 00:00:00 2001 From: Mikhail Pyltsin Date: Tue, 29 Jul 2025 18:13:58 +0200 Subject: [PATCH] [java-highlighting] IDEA-267379 "Non-canonical record constructor must delegate to another constructor" fix could be provided GitOrigin-RevId: f27d1c6ae299debfaab0504921cf96487c9b62ab --- .../intention/QuickFixFactory.java | 6 +- .../messages/QuickFixBundle.properties | 4 +- .../analysis/DefaultJavaErrorFixProvider.java | 1 + .../impl/quickfix/RecordThisDelegateFix.java | 133 ++++++++++++++++++ .../impl/config/QuickFixFactoryImpl.java | 5 + .../afterEmptyConstructor.java | 11 ++ .../recordThisDelegate/afterEnoughParam.java | 11 ++ .../recordThisDelegate/beforeBrokenPsi.java | 12 ++ .../beforeEmptyConstructor.java | 10 ++ .../recordThisDelegate/beforeEnoughParam.java | 12 ++ .../beforeNotEnoughParam.java | 11 ++ .../quickfix/RecordThisDelegateFixTest.java | 14 ++ 12 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RecordThisDelegateFix.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/afterEmptyConstructor.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/afterEnoughParam.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeBrokenPsi.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeEmptyConstructor.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeEnoughParam.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeNotEnoughParam.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/RecordThisDelegateFixTest.java diff --git a/java/java-analysis-api/src/com/intellij/codeInsight/intention/QuickFixFactory.java b/java/java-analysis-api/src/com/intellij/codeInsight/intention/QuickFixFactory.java index a1db3d2fb8f7..e3614060524f 100644 --- a/java/java-analysis-api/src/com/intellij/codeInsight/intention/QuickFixFactory.java +++ b/java/java-analysis-api/src/com/intellij/codeInsight/intention/QuickFixFactory.java @@ -522,5 +522,9 @@ public abstract class QuickFixFactory { @ApiStatus.Experimental public abstract @NotNull IntentionAction createAddMainMethodFix(@NotNull PsiImplicitClass implicitClass); - public abstract @NotNull ModCommandAction createReplaceOnDemandImport(@NotNull PsiImportModuleStatement importModuleStatement, @NotNull @Nls String text); + public abstract @NotNull ModCommandAction createReplaceOnDemandImport(@NotNull PsiImportModuleStatement importModuleStatement, + @NotNull @Nls String text); + + public abstract @Nullable ModCommandAction createRecordThisDelegateFix(PsiMethod psi); + } \ No newline at end of file diff --git a/java/java-analysis-impl/resources/messages/QuickFixBundle.properties b/java/java-analysis-impl/resources/messages/QuickFixBundle.properties index e9af08f46019..f7173748afcf 100644 --- a/java/java-analysis-impl/resources/messages/QuickFixBundle.properties +++ b/java/java-analysis-impl/resources/messages/QuickFixBundle.properties @@ -456,4 +456,6 @@ qualify.method.call.family=Qualify method call remove.redundant.nested.patterns.fix.text=Remove redundant nested pattern{0, choice, 1#|2#s} add.missing.nested.patterns.fix.text=Add missing nested pattern{0, choice, 1#|2#s} -add.missing.str.processor=Add 'STR.' Processor \ No newline at end of file +add.missing.str.processor=Add 'STR.' Processor + +record.delegate.to.canonical.constructor.fix.name=Delegate to canonical constructor \ No newline at end of file diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/DefaultJavaErrorFixProvider.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/DefaultJavaErrorFixProvider.java index d937c4ae4238..bd82dc08b5b8 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/DefaultJavaErrorFixProvider.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/DefaultJavaErrorFixProvider.java @@ -1129,6 +1129,7 @@ public final class DefaultJavaErrorFixProvider extends AbstractJavaErrorFixProvi fix(RECORD_CONSTRUCTOR_STRONGER_ACCESS, error -> addModifierFix(error.psi(), error.context().toPsiModifier())); fix(RECORD_ACCESSOR_NON_PUBLIC, error -> addModifierFix(error.psi(), PsiModifier.PUBLIC)); fix(RECORD_ACCESSOR_WRONG_RETURN_TYPE, error -> myFactory.createMethodReturnFix(error.psi(), error.context().lType(), false)); + fix(RECORD_NO_CONSTRUCTOR_CALL_IN_NON_CANONICAL, error -> myFactory.createRecordThisDelegateFix(error.psi())); fix(RECORD_CANONICAL_CONSTRUCTOR_WRONG_PARAMETER_TYPE, error -> { PsiParameter parameter = error.psi(); PsiMethod method = (PsiMethod)parameter.getDeclarationScope(); diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RecordThisDelegateFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RecordThisDelegateFix.java new file mode 100644 index 000000000000..c030f624c25d --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RecordThisDelegateFix.java @@ -0,0 +1,133 @@ +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.codeInsight.daemon.impl.quickfix; + +import com.intellij.codeInsight.daemon.QuickFixBundle; +import com.intellij.modcommand.ActionContext; +import com.intellij.modcommand.ModCommandAction; +import com.intellij.modcommand.ModPsiUpdater; +import com.intellij.modcommand.PsiUpdateModCommandAction; +import com.intellij.psi.*; +import com.intellij.psi.codeStyle.CodeStyleManager; +import com.intellij.psi.util.JavaPsiRecordUtil; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.JavaPsiConstructorUtil; +import com.siyeh.ig.psiutils.CommentTracker; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +public class RecordThisDelegateFix extends PsiUpdateModCommandAction { + + private RecordThisDelegateFix(@NotNull PsiMethod element) { + super(element); + } + + @Override + protected void invoke(@NotNull ActionContext context, @NotNull PsiMethod element, @NotNull ModPsiUpdater updater) { + CommentTracker tracker = new CommentTracker(); + Map map = collectAssignedFields(element); + PsiClass containingClass = element.getContainingClass(); + if (containingClass == null) return; + PsiRecordComponent[] components = containingClass.getRecordComponents(); + if (components.length != map.size()) return; + StringBuilder text = new StringBuilder("this("); + for (int i = 0; i < components.length; i++) { + if (i > 0) text.append(", "); + PsiRecordComponent component = components[i]; + PsiField field = JavaPsiRecordUtil.getFieldForComponent(component); + if (field == null) continue; + PsiExpression expression = map.get(field); + if (expression == null) continue; + text.append(tracker.text(expression)); + } + text.append(");"); + ArrayList expressions = new ArrayList<>(map.values()); + PsiCodeBlock body = element.getBody(); + if (body == null || body.getLBrace() == null) { + return; + } + if (expressions.isEmpty()) { + PsiElementFactory factory = JavaPsiFacade.getElementFactory(context.project()); + body.addAfter(factory.createStatementFromText(text.toString(), element), body.getLBrace()); + CodeStyleManager.getInstance(context.project()).reformat(body); + return; + } + for (int i = 0; i < expressions.size(); i++) { + PsiExpression value = expressions.get(i); + PsiStatement statement = PsiTreeUtil.getParentOfType(value, PsiStatement.class, false); + if (statement == null) continue; + if (i == expressions.size() - 1) { + tracker.replaceAndRestoreComments(statement, text.toString()); + } + else { + tracker.delete(statement); + } + } + CodeStyleManager.getInstance(context.project()).reformat(body); + } + + @Override + public @Nls @NotNull String getFamilyName() { + return QuickFixBundle.message("record.delegate.to.canonical.constructor.fix.name"); + } + + + public static @Nullable ModCommandAction create(@NotNull PsiMethod method) { + if (!method.isConstructor()) return null; + PsiClass containingClass = method.getContainingClass(); + if (containingClass == null) return null; + if (!containingClass.isRecord()) return null; + if (JavaPsiRecordUtil.isCanonicalConstructor(method) || + JavaPsiRecordUtil.isExplicitCanonicalConstructor(method)) { + return null; + } + PsiMethodCallExpression call = JavaPsiConstructorUtil.findThisOrSuperCallInConstructor(method); + if (call != null) return null; + if (!canCollectFields(method, containingClass)) return null; + return new RecordThisDelegateFix(method); + } + + private static boolean canCollectFields(@NotNull PsiMethod method, PsiClass aClass) { + Map collected = collectAssignedFields(method); + PsiRecordComponent[] components = aClass.getRecordComponents(); + return components.length == collected.size(); + } + + @NotNull + private static Map collectAssignedFields(@NotNull PsiMethod method) { + Map assignedFields = new HashMap<>(); + PsiCodeBlock body = method.getBody(); + if (body == null) return assignedFields; + + PsiStatement[] statements = body.getStatements(); + for (PsiStatement statement : statements) { + if (PsiTreeUtil.hasErrorElements(statement)) break; + if (!(statement instanceof PsiExpressionStatement expressionStatement)) { + break; + } + PsiExpression expression = expressionStatement.getExpression(); + if (!(expression instanceof PsiAssignmentExpression assignment)) { + break; + } + if (assignment.getOperationTokenType() != JavaTokenType.EQ) { + break; + } + PsiExpression lExpression = assignment.getLExpression(); + PsiExpression rExpression = assignment.getRExpression(); + if (rExpression != null && lExpression instanceof PsiReferenceExpression referenceExpression) { + PsiElement resolved = referenceExpression.resolve(); + if (resolved instanceof PsiField field) { + if (assignedFields.containsKey(field)) return new HashMap<>(); + assignedFields.put(field, rExpression); + continue; + } + } + break; + } + return assignedFields; + } +} \ No newline at end of file diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java index 1882fb2ea766..970458157469 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java @@ -1134,6 +1134,11 @@ public final class QuickFixFactoryImpl extends QuickFixFactory { return new ReplaceOnDemandImportAction(importModuleStatement, text); } + @Override + public @Nullable ModCommandAction createRecordThisDelegateFix(@NotNull PsiMethod psi) { + return RecordThisDelegateFix.create(psi); + } + private static class ReplaceOnDemandImportAction extends PsiUpdateModCommandAction { @NlsSafe private final String text; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/afterEmptyConstructor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/afterEmptyConstructor.java new file mode 100644 index 000000000000..545b6a6833e2 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/afterEmptyConstructor.java @@ -0,0 +1,11 @@ +// "Delegate to canonical constructor" "true-preview" +record Foo() { + Foo(int i, String s) { + this(); + doSomething(); + } + + private void doSomething() { + + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/afterEnoughParam.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/afterEnoughParam.java new file mode 100644 index 000000000000..02bd7624c033 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/afterEnoughParam.java @@ -0,0 +1,11 @@ +// "Delegate to canonical constructor" "true-preview" +record Foo(int a, String b) { + Foo(String s, int i) { + this(i, s); + doSomething(); + } + + private void doSomething() { + + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeBrokenPsi.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeBrokenPsi.java new file mode 100644 index 000000000000..c97d1ccd6714 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeBrokenPsi.java @@ -0,0 +1,12 @@ +// "Delegate to canonical constructor" "false" +record Foo(int a, String b) { + Foo(String s, int i) { + b = s + a = i; + doSomething(); + } + + private void doSomething() { + + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeEmptyConstructor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeEmptyConstructor.java new file mode 100644 index 000000000000..7bd77b577c38 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeEmptyConstructor.java @@ -0,0 +1,10 @@ +// "Delegate to canonical constructor" "true-preview" +record Foo() { + Foo(int i, String s) { + doSomething(); + } + + private void doSomething() { + + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeEnoughParam.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeEnoughParam.java new file mode 100644 index 000000000000..81638382ca72 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeEnoughParam.java @@ -0,0 +1,12 @@ +// "Delegate to canonical constructor" "true-preview" +record Foo(int a, String b) { + Foo(String s, int i) { + b = s; + a = i; + doSomething(); + } + + private void doSomething() { + + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeNotEnoughParam.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeNotEnoughParam.java new file mode 100644 index 000000000000..db7240e612af --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate/beforeNotEnoughParam.java @@ -0,0 +1,11 @@ +// "Delegate to canonical constructor" "false" +record Foo(int a, String b) { + Foo(String s, int i) { + b = s; + doSomething(); + } + + private void doSomething() { + + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/RecordThisDelegateFixTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/RecordThisDelegateFixTest.java new file mode 100644 index 000000000000..980a85c934d9 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/RecordThisDelegateFixTest.java @@ -0,0 +1,14 @@ +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.codeInsight.daemon.impl.quickfix; + + +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; + +public class RecordThisDelegateFixTest extends LightQuickFixParameterizedTestCase { + + @Override + protected String getBasePath() { + return "/codeInsight/daemonCodeAnalyzer/quickFix/recordThisDelegate"; + } +} +