diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/classCanBeRecord/ConstructorBodyProcessor.java b/java/java-impl-inspections/src/com/intellij/codeInspection/classCanBeRecord/ConstructorBodyProcessor.java index 191c886a0942..71484c1bc58e 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/classCanBeRecord/ConstructorBodyProcessor.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/classCanBeRecord/ConstructorBodyProcessor.java @@ -9,6 +9,7 @@ import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.JavaPsiConstructorUtil; import com.intellij.util.containers.MultiMap; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.NotNullByDefault; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.UnmodifiableView; @@ -19,6 +20,7 @@ import static com.intellij.psi.PsiModifier.STATIC; @NotNullByDefault final class ConstructorBodyProcessor { + private final PsiClass containingClass; private final PsiMethod constructor; private final Map paramsToFields = new HashMap<>(); // TODO(bartekpacia): change type to SequencedMap once we move to Java 21 @@ -29,16 +31,23 @@ final class ConstructorBodyProcessor { private boolean delegating = false; private boolean hasUnresolvedRefs = false; private boolean tooComplex = false; + + /// Even if false, it doesn't necessarily mean that conversion to record isn't possible. + /// + /// It is OK to have statements before all fields are assigned if: + /// - this constructor is a canonical constructor, OR + /// - (JDK 25+) this statement is in constructor prologue (see JEP 513) private boolean statementsBeforeAllFieldsAssigned = false; private final List otherStatements = new ArrayList<>(); private final MultiMap fieldsToParams = new MultiMap<>(); ConstructorBodyProcessor(PsiMethod constructor, List instanceFields) { + this.containingClass = Objects.requireNonNull(constructor.getContainingClass(), "constructor must have containing class"); this.constructor = constructor; this.instanceFields = instanceFields; - assert constructor.getBody() != null; // The caller asserts this - for (PsiStatement statement : constructor.getBody().getStatements()) { + final PsiCodeBlock body = Objects.requireNonNull(constructor.getBody(), "constructor must have body"); + for (PsiStatement statement : body.getStatements()) { execute(statement); } @@ -52,22 +61,35 @@ final class ConstructorBodyProcessor { } final PsiExpression expression = expressionStatement.getExpression(); - if (expression instanceof PsiMethodCallExpression methodCallExpr) { - if (JavaPsiConstructorUtil.isChainedConstructorCall(methodCallExpr)) { - delegating = true; - return; + if (expression instanceof PsiMethodCallExpression methodCallExpr && JavaPsiConstructorUtil.isChainedConstructorCall(methodCallExpr)) { + delegating = true; + for (PsiExpression arg : methodCallExpr.getArgumentList().getExpressions()) { + if (hasReferenceToContainingClass(containingClass, arg)) { + statementsBeforeAllFieldsAssigned = true; + } } + return; } // Is it an assignment expression to an instance field? - // If not, then all instance variables must already be assigned. if (!expressionIsAssignmentToInstanceField(expression) && !delegating) { + // It is NOT an assignment expression to an instance field. + // This means that: + // - all instance variables must be already initialized, OR + // - (JDK 25+) this statement is inside early construction context, more specifically: in constructor prologue (see JEP 513). + // This means that it must NOT use use 'this', either implicitly or explicitly, except for simple assignment statements. + otherStatements.add(statement); - // If not all instance fields are assigned up to this point, - // then this constructor cannot be converted to a non-canonical record constructor. - if (fieldNamesToInitializers.size() < instanceFields.size()) { + if (fieldNamesToInitializers.isEmpty() && PsiUtil.isAvailable(JavaFeature.STATEMENTS_BEFORE_SUPER, statement)) { + PsiExpression exprToConsider = expression instanceof PsiAssignmentExpression assignExpr ? assignExpr.getRExpression() : expression; + if (hasReferenceToContainingClass(containingClass, exprToConsider)) { + statementsBeforeAllFieldsAssigned = true; + } + } + else if (fieldNamesToInitializers.size() < instanceFields.size()) { + // If not all instance fields are assigned up to this point, + // then this constructor cannot be converted to a non-canonical record constructor. statementsBeforeAllFieldsAssigned = true; - // It is OK to have statements before all fields are assigned if this constructor is a canonical constructor. } return; } @@ -225,4 +247,25 @@ final class ConstructorBodyProcessor { PsiElement resolved = referenceExpr.resolve(); return resolved instanceof PsiField psiField && !psiField.hasModifierProperty(STATIC); } + + private static boolean hasReferenceToContainingClass(@NotNull PsiClass containingClass, @Nullable PsiExpression expression) { + if (expression == null) return false; + Ref hasReferenceToClassUnderConstruction = new Ref<>(false); + expression.accept(new JavaRecursiveElementWalkingVisitor() { + @Override + public void visitReferenceExpression(PsiReferenceExpression expression) { + super.visitReferenceExpression(expression); + PsiElement resolved = expression.resolve(); + if (resolved instanceof PsiField field && !field.hasModifierProperty(STATIC) && field.getContainingClass() == containingClass) { + hasReferenceToClassUnderConstruction.set(true); + } + else if (resolved instanceof PsiMethod method && + !method.hasModifierProperty(STATIC) && + method.getContainingClass() == containingClass) { + hasReferenceToClassUnderConstruction.set(true); + } + } + }); + return hasReferenceToClassUnderConstruction.get(); + } } diff --git a/java/java-tests/testData/inspection/classCanBeRecord/beforeMultipleConstructors_Delegating_4_redCode.java b/java/java-tests/testData/inspection/classCanBeRecord/beforeMultipleConstructors_Delegating_4_redCode.java new file mode 100644 index 000000000000..970305c295d8 --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/beforeMultipleConstructors_Delegating_4_redCode.java @@ -0,0 +1,14 @@ +// "Convert to record class" "false" +class Person { + final String name; + final int age; + + Person(String name, int age) { + this.name = name; + this.age = age; + } + + Person(String name) { + this(name, age); // javac error + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterAdditionalStatements_1.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterAdditionalStatements_1.java new file mode 100644 index 000000000000..ac38dc6ee2a1 --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterAdditionalStatements_1.java @@ -0,0 +1,9 @@ +// "Convert to record class" "true-preview" +record Point(double x, double y) { + Point(double x, double y) { + System.out.println("Hello I am going to be created"); + this.x = x; + this.y = y; + System.out.println("Hello I was just created"); + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDelegating_1.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDelegating_1.java new file mode 100644 index 000000000000..d91f3a92e6f8 --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDelegating_1.java @@ -0,0 +1,8 @@ +// "Convert to record class" "true-preview" +record Person(String name, int age) { + + Person(String name) { + System.out.println("age not passed"); + this(name, 0); + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDelegating_2.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDelegating_2.java new file mode 100644 index 000000000000..5ee6efec6ee9 --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDelegating_2.java @@ -0,0 +1,8 @@ +// "Convert to record class" "true-preview" +record Person(String name, int age) { + + Person(String name) { + System.out.println("age not passed" + name); + this(name, 0); + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDelegating_4.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDelegating_4.java new file mode 100644 index 000000000000..7cbb85d73c25 --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDelegating_4.java @@ -0,0 +1,13 @@ +// "Convert to record class" "true-preview" +record Person(String name, int age) { + static int staticVar = 42; + + static void staticMethod() { + } + + Person(String name) { + System.out.println("age not passed" + staticVar); + staticMethod(); + this(name, 0); + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDirectFieldAssignments_1.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDirectFieldAssignments_1.java new file mode 100644 index 000000000000..d91f3a92e6f8 --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/afterDirectFieldAssignments_1.java @@ -0,0 +1,8 @@ +// "Convert to record class" "true-preview" +record Person(String name, int age) { + + Person(String name) { + System.out.println("age not passed"); + this(name, 0); + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeAdditionalStatements_1.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeAdditionalStatements_1.java new file mode 100644 index 000000000000..7b2d25868495 --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeAdditionalStatements_1.java @@ -0,0 +1,12 @@ +// "Convert to record class" "true-preview" +class Point { + final double x; + final double y; + + Point(double x, double y) { + System.out.println("Hello I am going to be created"); + this.x = x; + this.y = y; + System.out.println("Hello I was just created"); + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_1.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_1.java new file mode 100644 index 000000000000..0ce999c62fee --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_1.java @@ -0,0 +1,15 @@ +// "Convert to record class" "true-preview" +class Person { + final String name; + final int age; + + Person(String name, int age) { + this.name = name; + this.age = age; + } + + Person(String name) { + System.out.println("age not passed"); + this(name, 0); + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_2.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_2.java new file mode 100644 index 000000000000..5995c44a4a32 --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_2.java @@ -0,0 +1,15 @@ +// "Convert to record class" "true-preview" +class Person { + final String name; + final int age; + + Person(String name, int age) { + this.name = name; + this.age = age; + } + + Person(String name) { + System.out.println("age not passed" + name); + this(name, 0); + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_3.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_3.java new file mode 100644 index 000000000000..92cdaa2251bf --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_3.java @@ -0,0 +1,15 @@ +// "Convert to record class" "false" +class Person { + final String name; + final int age; + + Person(String name, int age) { + this.name = name; + this.age = age; + } + + Person(String name) { + System.out.println("age not passed" + this.name); // javac error: "cannot reference 'this' before superclass constructor is called" + this(name, 0); + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_4.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_4.java new file mode 100644 index 000000000000..354150c79955 --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_4.java @@ -0,0 +1,20 @@ +// "Convert to record class" "true-preview" +class Person { + final String name; + final int age; + static int staticVar = 42; + + static void staticMethod() { + } + + Person(String name, int age) { + this.name = name; + this.age = age; + } + + Person(String name) { + System.out.println("age not passed" + staticVar); + staticMethod(); + this(name, 0); + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_5.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_5.java new file mode 100644 index 000000000000..f926bb15d364 --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDelegating_5.java @@ -0,0 +1,14 @@ +// "Convert to record class" "false" +class Person { + final String name; + int age = 0; + + Person(String name, int age) { + this.name = name; + this.age = age; + } + + Person(String name) { + this(name, 0 + age); // javac error: "cannot reference age before supertype constructor has been called" + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDirectFieldAssignments_1.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDirectFieldAssignments_1.java new file mode 100644 index 000000000000..151fdbac418a --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDirectFieldAssignments_1.java @@ -0,0 +1,16 @@ +// "Convert to record class" "true-preview" +class Person { + final String name; + final int age; + + Person(String name, int age) { + this.name = name; + this.age = age; + } + + Person(String name) { + System.out.println("age not passed"); + this.name = name; + this.age = 0; + } +} diff --git a/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDirectFieldAssignments_2.java b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDirectFieldAssignments_2.java new file mode 100644 index 000000000000..f13e8ae51b2a --- /dev/null +++ b/java/java-tests/testData/inspection/classCanBeRecord/flexibleConstructorBodies/beforeDirectFieldAssignments_2.java @@ -0,0 +1,16 @@ +// "Convert to record class" "false" +class Person { + final String name; + final int age; + + Person(String name, int age) { + this.name = name; + this.age = age; + } + + Person(String name) { + this.name = name; + System.out.println("age not passed"); // cannot convert to delegating constructor call while preserving semantics + this.age = 0; + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/classCanBeRecord/ClassCanBeRecordFlexibleConstructorBodiesTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/classCanBeRecord/ClassCanBeRecordFlexibleConstructorBodiesTest.java new file mode 100644 index 000000000000..0a4ecdad4ad8 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/classCanBeRecord/ClassCanBeRecordFlexibleConstructorBodiesTest.java @@ -0,0 +1,44 @@ +// 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.java.codeInspection.classCanBeRecord; + +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.classCanBeRecord.ClassCanBeRecordInspection; +import com.intellij.refactoring.BaseRefactoringProcessor; +import com.intellij.testFramework.LightProjectDescriptor; +import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; +import org.jetbrains.annotations.NotNull; + +import static com.intellij.codeInspection.classCanBeRecord.ClassCanBeRecordInspection.ConversionStrategy; + +public class ClassCanBeRecordFlexibleConstructorBodiesTest extends LightQuickFixParameterizedTestCase { + @Override + protected LocalInspectionTool @NotNull [] configureLocalInspectionTools() { + ClassCanBeRecordInspection inspection = new ClassCanBeRecordInspection(ConversionStrategy.DO_NOT_SUGGEST, true); + return new LocalInspectionTool[]{inspection}; + } + + @Override + protected @NotNull LightProjectDescriptor getProjectDescriptor() { + return LightJavaCodeInsightFixtureTestCase.JAVA_LATEST; + } + + @Override + protected String getBasePath() { + return "/inspection/classCanBeRecord/flexibleConstructorBodies"; + } + + @Override + public void runSingle() throws Throwable { + try { + super.runSingle(); + } + catch (BaseRefactoringProcessor.ConflictsInTestsException e) { + // Verify that no content was changed. See IDEA-371645. + checkResultByFile(getTestName(false) + ".java", getBasePath() + "/before" + getTestName(false), false); + } + + + super.runSingle(); + } +}