From 21617c81371b0f1573a0ccb54aa70dd205982687 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 15 Mar 2024 16:05:56 +0100 Subject: [PATCH] [java-refactoring] Rename conflicting local class IDEA-332489 Inlining refactoring operation leads to naming conflicts GitOrigin-RevId: 4e239def02419e7a0a30097d3b3ec8cdb69911fd --- .../inline/InlineMethodProcessor.java | 2 +- .../inline/InlineObjectProcessor.java | 6 +- .../intellij/refactoring/util/InlineUtil.java | 71 +++++++++++++------ .../inlineMethod/RenameLocalClass.java | 15 ++++ .../inlineMethod/RenameLocalClass.java.after | 13 ++++ .../RenameLocalClassDoubleConflict.java | 19 +++++ .../RenameLocalClassDoubleConflict.java.after | 16 +++++ .../refactoring/inline/InlineMethodTest.java | 4 ++ 8 files changed, 122 insertions(+), 24 deletions(-) create mode 100644 java/java-tests/testData/refactoring/inlineMethod/RenameLocalClass.java create mode 100644 java/java-tests/testData/refactoring/inlineMethod/RenameLocalClass.java.after create mode 100644 java/java-tests/testData/refactoring/inlineMethod/RenameLocalClassDoubleConflict.java create mode 100644 java/java-tests/testData/refactoring/inlineMethod/RenameLocalClassDoubleConflict.java.after diff --git a/java/java-impl-refactorings/src/com/intellij/refactoring/inline/InlineMethodProcessor.java b/java/java-impl-refactorings/src/com/intellij/refactoring/inline/InlineMethodProcessor.java index 6ed644ddd077..a9505fd491b9 100644 --- a/java/java-impl-refactorings/src/com/intellij/refactoring/inline/InlineMethodProcessor.java +++ b/java/java-impl-refactorings/src/com/intellij/refactoring/inline/InlineMethodProcessor.java @@ -655,7 +655,7 @@ public class InlineMethodProcessor extends BaseRefactoringProcessor { BlockData blockData = prepareBlock(ref, helper); ChangeContextUtil.encodeContextInfo(blockData.block, false); helper.substituteTypes(blockData.parmVars); - InlineUtil.solveVariableNameConflicts(blockData.block, ref, myMethodCopy.getBody()); + InlineUtil.solveLocalNameConflicts(blockData.block, ref, myMethodCopy.getBody()); helper.initializeParameters(blockData.parmVars); addThisInitializer(methodCall, blockData.thisVar); diff --git a/java/java-impl-refactorings/src/com/intellij/refactoring/inline/InlineObjectProcessor.java b/java/java-impl-refactorings/src/com/intellij/refactoring/inline/InlineObjectProcessor.java index 77a342c6029f..8e5e01ddc837 100644 --- a/java/java-impl-refactorings/src/com/intellij/refactoring/inline/InlineObjectProcessor.java +++ b/java/java-impl-refactorings/src/com/intellij/refactoring/inline/InlineObjectProcessor.java @@ -101,7 +101,7 @@ public final class InlineObjectProcessor extends BaseRefactoringProcessor { InlineTransformer ctorTransformer = InlineTransformer.getSuitableTransformer(myMethod).apply(myReference); ctorTransformer.transformBody(ctorCopy, myReference, PsiTypes.voidType()); PsiCodeBlock ctorBody = Objects.requireNonNull(ctorCopy.getBody()); - InlineUtil.solveVariableNameConflicts(ctorBody, target, ctorBody); + InlineUtil.solveLocalNameConflicts(ctorBody, target, ctorBody); updateFieldRefs(ctorCopy, aClass); ctorParameters = addRange(target, ctorBody, ctorParameters); @@ -110,7 +110,7 @@ public final class InlineObjectProcessor extends BaseRefactoringProcessor { InlineTransformer nextTransformer = InlineTransformer.getSuitableTransformer(myNextMethod).apply(myNextCall.getMethodExpression()); PsiLocalVariable result = nextTransformer.transformBody(nextCopy, myNextCall.getMethodExpression(), myNextCall.getType()); PsiCodeBlock nextBody = Objects.requireNonNull(nextCopy.getBody()); - InlineUtil.solveVariableNameConflicts(nextBody, target, nextBody); + InlineUtil.solveLocalNameConflicts(nextBody, target, nextBody); updateFieldRefs(nextCopy, aClass); if (result != null) { PsiLocalVariable[] resultAndParameters = ArrayUtil.prepend(result, nextParameters); @@ -122,7 +122,7 @@ public final class InlineObjectProcessor extends BaseRefactoringProcessor { nextParameters = addRange(target, nextBody, nextParameters); } - InlineUtil.solveVariableNameConflicts(target, myReference.getElement(), target); + InlineUtil.solveLocalNameConflicts(target, myReference.getElement(), target); ctorHelper.initializeParameters(ctorParameters); nextHelper.initializeParameters(nextParameters); diff --git a/java/java-impl-refactorings/src/com/intellij/refactoring/util/InlineUtil.java b/java/java-impl-refactorings/src/com/intellij/refactoring/util/InlineUtil.java index 23f8e5a8237b..a73b6d453c57 100644 --- a/java/java-impl-refactorings/src/com/intellij/refactoring/util/InlineUtil.java +++ b/java/java-impl-refactorings/src/com/intellij/refactoring/util/InlineUtil.java @@ -30,11 +30,13 @@ import com.intellij.util.CommonJavaRefactoringUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MultiMap; +import com.intellij.util.text.NameUtilCore; import com.siyeh.ig.psiutils.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.*; +import java.util.function.BiFunction; import static com.intellij.util.ObjectUtils.tryCast; @@ -78,7 +80,7 @@ public final class InlineUtil implements CommonJavaInlineUtil { } } } - solveVariableNameConflicts(initializer, ref, initializer); + solveLocalNameConflicts(initializer, ref, initializer); ChangeContextUtil.encodeContextInfo(initializer, false); PsiExpression expr = (PsiExpression)replaceDiamondWithInferredTypesIfNeeded(initializer, ref); @@ -299,33 +301,62 @@ public final class InlineUtil implements CommonJavaInlineUtil { return ref != initializer ? ref.replace(initializer) : initializer; } - public static void solveVariableNameConflicts(final PsiElement scope, - final PsiElement placeToInsert, - final PsiElement renameScope) throws IncorrectOperationException { - if (scope instanceof PsiVariable var) { - String name = var.getName(); - String oldName = name; - final JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(scope.getProject()); - while (true) { - String newName = codeStyleManager.suggestUniqueVariableName(name, placeToInsert, true); - if (newName.equals(name)) break; - name = newName; - newName = codeStyleManager.suggestUniqueVariableName(name, var, true); - if (newName.equals(name)) break; - name = newName; - } - if (!name.equals(oldName)) { - RefactoringUtil.renameVariableReferences(var, name, new LocalSearchScope(renameScope), true); - var.getNameIdentifier().replace(JavaPsiFacade.getElementFactory(scope.getProject()).createIdentifier(name)); + public static void solveLocalNameConflicts(final PsiElement scope, + final PsiElement placeToInsert, + final PsiElement renameScope) { + if (scope instanceof PsiVariable || scope instanceof PsiClass) { + PsiNameIdentifierOwner named = (PsiNameIdentifierOwner)scope; + String name = named.getName(); + PsiElement identifier = named.getNameIdentifier(); + if (name != null && identifier != null) { + String oldName = name; + Project project = scope.getProject(); + final JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(project); + BiFunction suggester = + scope instanceof PsiVariable ? + (place, curName) -> codeStyleManager.suggestUniqueVariableName(curName, place, true) : + (place, curName) -> suggestClassName(place, curName); + while (true) { + String newName = suggester.apply(placeToInsert, name); + if (newName.equals(name)) break; + name = newName; + newName = suggester.apply(named, name); + if (newName.equals(name)) break; + name = newName; + } + if (!name.equals(oldName)) { + for (PsiReference reference : ReferencesSearch.search(named, new LocalSearchScope(renameScope), true)) { + reference.handleElementRename(name); + } + PsiElementFactory factory = JavaPsiFacade.getElementFactory(scope.getProject()); + if (named instanceof PsiClass cls) { + for (PsiMethod constructor : cls.getConstructors()) { + if (!(constructor instanceof SyntheticElement) && constructor.getName().equals(oldName)) { + Objects.requireNonNull(constructor.getNameIdentifier()).replace(factory.createIdentifier(name)); + } + } + } + Objects.requireNonNull(named.getNameIdentifier()).replace(factory.createIdentifier(name)); + } } } PsiElement[] children = scope.getChildren(); for (PsiElement child : children) { - solveVariableNameConflicts(child, placeToInsert, renameScope); + solveLocalNameConflicts(child, placeToInsert, renameScope); } } + private static @NotNull String suggestClassName(@NotNull PsiElement place, @NotNull String name) { + PsiResolveHelper helper = PsiResolveHelper.getInstance(place.getProject()); + return NameUtilCore.uniqName( + name, + n -> helper.resolveReferencedClass(n, place) != null || + place instanceof PsiClass && place.getParent() instanceof PsiDeclarationStatement decl && + decl.getParent() instanceof PsiCodeBlock block && + SyntaxTraverser.psiTraverser(block).filter(PsiClass.class).find(cls -> n.equals(cls.getName())) != null); + } + public static boolean isChainingConstructor(PsiMethod constructor) { return CommonJavaRefactoringUtil.getChainedConstructor(constructor) != null; } diff --git a/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClass.java b/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClass.java new file mode 100644 index 000000000000..4bde77d44c15 --- /dev/null +++ b/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClass.java @@ -0,0 +1,15 @@ +class TestCase{ + public void main() { + class T { + public T() {} + } + /*]*/foo();/*[*/ + } + + public void foo() { + class T { + T t; + public T() {} + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClass.java.after b/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClass.java.after new file mode 100644 index 000000000000..c3046082d0fd --- /dev/null +++ b/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClass.java.after @@ -0,0 +1,13 @@ +class TestCase{ + public void main() { + class T { + public T() {} + } + /*]*//*[*/ + class T1 { + T1 t; + public T1() {} + } + } + +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClassDoubleConflict.java b/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClassDoubleConflict.java new file mode 100644 index 000000000000..dcd920424b3d --- /dev/null +++ b/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClassDoubleConflict.java @@ -0,0 +1,19 @@ +class TestCase{ + public void main() { + class T1 { + public T1() {} + } + foo(); + } + + public void foo() { + class T1 { + T1 t; + public T1() {} + } + class T2 { + T2 t; + public T2() {} + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClassDoubleConflict.java.after b/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClassDoubleConflict.java.after new file mode 100644 index 000000000000..eb7c8e9c80f6 --- /dev/null +++ b/java/java-tests/testData/refactoring/inlineMethod/RenameLocalClassDoubleConflict.java.after @@ -0,0 +1,16 @@ +class TestCase{ + public void main() { + class T1 { + public T1() {} + } + class T3 { + T3 t; + public T3() {} + } + class T2 { + T2 t; + public T2() {} + } + } + +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/inline/InlineMethodTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/inline/InlineMethodTest.java index 2433da198be3..f81726a37e05 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/inline/InlineMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/inline/InlineMethodTest.java @@ -579,6 +579,10 @@ public class InlineMethodTest extends LightRefactoringTestCase { public void testSplitIfAndCollapseBack() { doTest(); } public void testThisVariableName() { doTest(); } + + public void testRenameLocalClass() { doTest(); } + + public void testRenameLocalClassDoubleConflict() { doTest(); } @Override protected Sdk getProjectJDK() {