[java-inspections] ClassCanBeRecord: fix broken undo and exception being thrown when ctor param names don't match instance field names

#IDEA-371645 fixed

Merge-request: IJ-MR-161862
Merged-by: Bartek Pacia <bartek.pacia@jetbrains.com>

GitOrigin-RevId: e151e27ffc84aa8263f1b67d6c80662614bec6ac

java/java-impl-inspections/src/com/intellij/codeInspection/classCanBeRecord/ConvertToRecordProcessor.java

@@ -24,8 +24,6 @@ import com.intellij.psi.util.PropertyUtilBase;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.refactoring.BaseRefactoringProcessor;
-import com.intellij.refactoring.RefactoringFactory;
-import com.intellij.refactoring.RenameRefactoring;
 import com.intellij.refactoring.rename.RenameProcessor;
 import com.intellij.refactoring.rename.RenamePsiElementProcessor;
 import com.intellij.refactoring.rename.RenameUtil;
@@ -90,37 +88,17 @@ final class ConvertToRecordProcessor extends BaseRefactoringProcessor {
   @Override
   protected void doRun() {
     prepareRenameOfAccessors();
-    prepareRenameOfConstructorParameters();
 
     super.doRun();
   }
 
-  private void prepareRenameOfConstructorParameters() {
-    RecordConstructorCandidate ctorCandidate = myRecordCandidate.getCanonicalConstructorCandidate();
-    if (ctorCandidate == null) return;
-
-    ctorCandidate.getCtorParamsToFields().forEach((ctorParam, field) -> {
-      if (!ctorParam.getName().equals(field.getName())) {
-        RenameRefactoring renameRefactoring = RefactoringFactory.getInstance(myProject).createRename(ctorParam, field.getName());
-        renameRefactoring.setPreviewUsages(false);
-        renameRefactoring.setSearchInComments(false);
-        // The below line is required to not show conflicts midway and break the refactoring flow.
-        renameRefactoring.setSearchInNonJavaFiles(false);
-        renameRefactoring.setInteractive(null);
-        renameRefactoring.run();
-      }
-    });
-  }
-
   private void prepareRenameOfAccessors() {
     List<FieldAccessorCandidate> accessorsToRename = getAccessorsToRename();
-
     for (var fieldAccessorCandidate : accessorsToRename) {
       String backingFieldName = fieldAccessorCandidate.backingField().getName();
 
       List<PsiMethod> methods = substituteWithSuperMethodsIfPossible(fieldAccessorCandidate.method());
       RenamePsiElementProcessor methodRenameProcessor = RenamePsiElementProcessor.forElement(methods.get(0));
-
       methods.forEach(method -> {
         myAllRenames.put(method, backingFieldName);
         methodRenameProcessor.prepareRenaming(method, backingFieldName, myAllRenames);
@@ -265,6 +243,7 @@ final class ConvertToRecordProcessor extends BaseRefactoringProcessor {
   @Override
   protected void performRefactoring(UsageInfo @NotNull [] usages) {
     renameMembers(usages);
+    renameConstructorParameters();
 
     final PsiClass psiClass = myRecordCandidate.getPsiClass();
     final RecordConstructorCandidate canonicalCtorCandidate = myRecordCandidate.getCanonicalConstructorCandidate();
@@ -366,6 +345,30 @@ final class ConvertToRecordProcessor extends BaseRefactoringProcessor {
                     place, null, null);
   }
 
+  private void renameConstructorParameters() {
+    RecordConstructorCandidate ctorCandidate = myRecordCandidate.getCanonicalConstructorCandidate();
+    if (ctorCandidate == null) return;
+
+    Map<PsiElement, String> ctorParamRenames = new LinkedHashMap<>();
+    List<UsageInfo> usagesToRename = new ArrayList<>();
+    ctorCandidate.getCtorParamsToFields().forEach((ctorParam, field) -> {
+      if (!ctorParam.getName().equals(field.getName())) {
+        UsageInfo[] usages = RenameUtil.findUsages(ctorParam, field.getName(), false, false, ctorParamRenames);
+        usagesToRename.addAll(Arrays.asList(usages));
+        ctorParamRenames.put(ctorParam, field.getName());
+      }
+    });
+
+    MultiMap<PsiElement, UsageInfo> renameUsagesByElement = RenameProcessor.classifyUsages(ctorParamRenames.keySet(), usagesToRename);
+    for (var entry : ctorParamRenames.entrySet()) {
+      PsiElement element = entry.getKey();
+      String newName = entry.getValue();
+      UsageInfo[] elementRenameUsages = renameUsagesByElement.get(entry.getKey()).toArray(UsageInfo.EMPTY_ARRAY);
+      RenamePsiElementProcessor renamePsiElementProcessor = RenamePsiElementProcessor.forElement(element);
+      renamePsiElementProcessor.renameElement(element, newName, elementRenameUsages, null);
+    }
+  }
+
   private void renameMembers(UsageInfo @NotNull [] usages) {
     List<UsageInfo> renameUsages = ContainerUtil.filter(usages, u -> !(u instanceof ConvertToRecordUsageInfo));
     MultiMap<PsiElement, UsageInfo> renameUsagesByElement = RenameProcessor.classifyUsages(myAllRenames.keySet(), renameUsages);

java/java-impl-inspections/src/com/intellij/codeInspection/classCanBeRecord/RecordBuilder.java

@@ -29,7 +29,7 @@ import java.util.Objects;
 import java.util.StringJoiner;
 import java.util.stream.Collectors;
 
-class RecordBuilder {
+final class RecordBuilder {
   private final StringBuilder myRecordText = new StringBuilder();
   private final PsiClass myOriginClass;
 

java/java-tests/testData/inspection/classCanBeRecord/afterCtorFieldAssignedToExpression.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 record Point2(double x, double y) {
     Point2(double x, double y) {

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_1.java

@@ -1,4 +1,3 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 record R(int myFirst) {
 }

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_10.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 record Point2(double x, double y) {
     Point2(double x, double y) {

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_11.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 record Point2(double x, double y) {
     Point2(double x, double y) {

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_2.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 record R(int myFirst) {
     R(int myFirst) {
         this.myFirst = myFirst;

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_2_another.java

@@ -0,0 +1,3 @@
+// "Convert to record class" "true-preview"
+record Person(String name, int age) {
+}

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_4_large.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 import org.jetbrains.annotations.NotNull;
 
 class SomeClass {

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_5_large_badOrder.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 import org.jetbrains.annotations.NotNull;
 
 class SomeClass {

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_6_large_veryBadOrder.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 import org.jetbrains.annotations.NotNull;
 
 class SomeClass {

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_7.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 // Test for IDEA-371419
 record Point2(double x, double y) {

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_8.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 // Test for IDEA-371419
 record Point2(double x, double y) {

java/java-tests/testData/inspection/classCanBeRecord/afterCtorParamsDifferFromFields_9.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 record Point2(double x, double y) {
     Point2(double x, double y) {

java/java-tests/testData/inspection/classCanBeRecord/afterSimple_5.java

@@ -0,0 +1,11 @@
+// "Convert to record class" "true"
+// no "true-preview" above because of IDEA-369873
+record R(int x, int y) {
+}
+
+class Main {
+  public static void main(String[] args) {
+    R r = new R(10, 20);
+    System.out.println("x: " + r.x() + ", y: " + r.y());
+  }
+}

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorFieldAssignedToExpression.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 class Point2<caret> {
   private final double x;

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_1.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 class <caret>R {
   final int myFirst;
 

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_10.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 class Point2<caret> {
   private final double x;

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_11.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 class Point2<caret> {
   private final double x;

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_2.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 class <caret>R {
   final int myFirst;
 

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_2_another.java

@@ -0,0 +1,10 @@
+// "Convert to record class" "true-preview"
+class Person<caret> {
+  final String name;
+  final int age;
+  
+  Person(String theName, int theAge) {
+    name = theName;
+    age = theAge;
+  }
+}

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_4_large.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 import org.jetbrains.annotations.NotNull;
 
 class SomeClass {

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_5_large_badOrder.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 import org.jetbrains.annotations.NotNull;
 
 class SomeClass {

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_6_large_veryBadOrder.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 import org.jetbrains.annotations.NotNull;
 
 class SomeClass {

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_7.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 // Test for IDEA-371419
 class Point2<caret> {

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_8.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 // Test for IDEA-371419
 class Point2<caret> {

java/java-tests/testData/inspection/classCanBeRecord/beforeCtorParamsDifferFromFields_9.java

@@ -1,5 +1,4 @@
-// "Convert to record class" "true"
-// no "true-preview" above because of IDEA-369873
+// "Convert to record class" "true-preview"
 
 class Point2<caret> {
   private final double x;

java/java-tests/testData/inspection/classCanBeRecord/beforeSimple_5.java

@@ -0,0 +1,26 @@
+// "Convert to record class" "true"
+// no "true-preview" above because of IDEA-369873
+class R<caret> {
+  final int x;
+  final int y;
+
+  R(int x, int y) {
+    this.x = x;
+    this.y = y;
+  }
+
+  int getX() {
+    return x;
+  }
+
+  int getY() {
+    return y;
+  }
+}
+
+class Main {
+  public static void main(String[] args) {
+    R r = new R(10, 20);
+    System.out.println("x: " + r.getX() + ", y: " + r.getY());
+  }
+}

java/java-tests/testData/inspection/classCanBeRecord/weakenAccessibility/afterCtorParamsDifferFromFields.java

@@ -0,0 +1,10 @@
+// "Convert to record class" "true-preview"
+
+// Test for IDEA-371419 and IDEA-371645
+record Point2(double x, double y) {
+    Point2(double x, double y) {
+        this.x = x;
+        this.y = y;
+        System.out.println("created");
+    }
+}

java/java-tests/testData/inspection/classCanBeRecord/weakenAccessibility/beforeCtorParamsDifferFromFields.java

@@ -0,0 +1,13 @@
+// "Convert to record class" "true-preview"
+
+// Test for IDEA-371419 and IDEA-371645
+class Point2<caret> {
+  private final double x;
+  private final double y;
+
+  Point2(double first, double second) {
+    this.x = first;
+    this.y = second;
+    System.out.println("created");
+  }
+}

java/java-tests/testSrc/com/intellij/java/codeInspection/classCanBeRecord/ClassCanBeRecordInspectionTest.java

@@ -25,6 +25,15 @@ public class ClassCanBeRecordInspectionTest extends LightQuickFixParameterizedTe
 
   @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);
+    }
+
+
     BaseRefactoringProcessor.ConflictsInTestsException.withIgnoredConflicts(super::runSingle);
   }
 }

java/java-tests/testSrc/com/intellij/java/codeInspection/classCanBeRecord/ClassCanBeRecordInspectionWeakenAccessibilityTest.java

@@ -29,6 +29,10 @@ public class ClassCanBeRecordInspectionWeakenAccessibilityTest extends LightQuic
 
   @Override
   public void runSingle() throws Throwable {
+    // Run and abort (because of conflicts), and then verify that no content was changed. See IDEA-371645.
+    assertThrows(BaseRefactoringProcessor.ConflictsInTestsException.class, () -> super.runSingle());
+    checkResultByFile(getTestName(false) + ".java", getBasePath() + "/before" + getTestName(false), false);
+    
     BaseRefactoringProcessor.ConflictsInTestsException.withIgnoredConflicts(super::runSingle);
   }
 }

platform/refactoring/src/com/intellij/refactoring/BaseRefactoringProcessor.java

@@ -180,8 +180,8 @@ public abstract class BaseRefactoringProcessor implements Runnable {
   protected abstract @NotNull @Command String getCommandName();
 
   /**
-   * Called as part of run {@link #run}
-   * 
+   * Called as part of {@link #run}.
+   * <p>
    * Must be called on EDT and outside a write action.
    */
   @RequiresEdt