From c8ac55d0aba033aed8529dd34ec5de7e5be03b83 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Tue, 28 Jul 2020 12:38:25 +0200 Subject: [PATCH] safe delete: suggest to delete non-member referenced from method or field to be deleted (IDEA-232557) GitOrigin-RevId: 39813ba3faf73af0ce07cbba09cc726bb0389579 --- .../inCallers/JavaMemberNode.java | 10 +- .../safeDelete/JavaSafeDeleteProcessor.java | 6 +- .../SafeDeleteJavaCalleeChooser.java | 114 ++++++++++++------ .../SafeDeleteMemberCalleeUsageInfo.java | 15 +-- .../after/Bundle.properties | 0 .../after/ClassWithInnerStaticImport.java | 5 + .../before/Bundle.properties | 1 + .../before/ClassWithInnerStaticImport.java | 9 ++ .../java/refactoring/SafeDeleteTest.java | 4 + 9 files changed, 114 insertions(+), 50 deletions(-) create mode 100644 java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/after/Bundle.properties create mode 100644 java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/after/ClassWithInnerStaticImport.java create mode 100644 java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/before/Bundle.properties create mode 100644 java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/before/ClassWithInnerStaticImport.java diff --git a/java/java-impl/src/com/intellij/refactoring/changeSignature/inCallers/JavaMemberNode.java b/java/java-impl/src/com/intellij/refactoring/changeSignature/inCallers/JavaMemberNode.java index 16a157f3bf9c..054583952f92 100644 --- a/java/java-impl/src/com/intellij/refactoring/changeSignature/inCallers/JavaMemberNode.java +++ b/java/java-impl/src/com/intellij/refactoring/changeSignature/inCallers/JavaMemberNode.java @@ -37,15 +37,19 @@ public abstract class JavaMemberNode extends MemberNodeBase @Override protected void customizeRendererText(ColoredTreeCellRenderer renderer) { + customizeRendererText(renderer, getMember(), isEnabled()); + } + + public static void customizeRendererText(ColoredTreeCellRenderer renderer, M member, boolean enabled) { final StringBuilder buffer = new StringBuilder(128); - final PsiClass containingClass = getMember().getContainingClass(); + final PsiClass containingClass = member.getContainingClass(); if (containingClass != null) { buffer.append(ClassPresentationUtil.getNameForClass(containingClass, false)); buffer.append('.'); } - buffer.append(formatMember(getMember())); + buffer.append(formatMember(member)); - final SimpleTextAttributes attributes = isEnabled() ? + final SimpleTextAttributes attributes = enabled ? new SimpleTextAttributes(SimpleTextAttributes.STYLE_PLAIN, UIUtil.getTreeForeground()) : SimpleTextAttributes.EXCLUDED_ATTRIBUTES; renderer.append(buffer.toString(), attributes); diff --git a/java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java b/java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java index 394be6863442..2cc7fe9e0140 100644 --- a/java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java @@ -415,7 +415,7 @@ public class JavaSafeDeleteProcessor extends SafeDeleteProcessorDelegateBase { else { final PsiMember member = calleesSafeToDelete.get(0).getCallerMember(); final ArrayList list = new ArrayList<>(); - AbstractJavaMemberCallerChooser chooser = new SafeDeleteJavaCalleeChooser(member, project, list) { + SafeDeleteJavaCalleeChooser chooser = new SafeDeleteJavaCalleeChooser(member, project, list) { @Override protected ArrayList getTopLevelItems() { return calleesSafeToDelete; @@ -702,9 +702,9 @@ public class JavaSafeDeleteProcessor extends SafeDeleteProcessorDelegateBase { } private static void appendCallees(@NotNull PsiMember method, @NotNull List usages) { - final List calleesSafeToDelete = SafeDeleteJavaCalleeChooser.computeCalleesSafeToDelete(method); + final List calleesSafeToDelete = SafeDeleteJavaCalleeChooser.computeCalleesSafeToDelete(method); if (calleesSafeToDelete != null) { - for (PsiMember callee : calleesSafeToDelete) { + for (PsiElement callee : calleesSafeToDelete) { usages.add(new SafeDeleteMemberCalleeUsageInfo(callee, method)); } } diff --git a/java/java-impl/src/com/intellij/refactoring/safeDelete/SafeDeleteJavaCalleeChooser.java b/java/java-impl/src/com/intellij/refactoring/safeDelete/SafeDeleteJavaCalleeChooser.java index cd53f7d9f18a..a0ec660103e2 100644 --- a/java/java-impl/src/com/intellij/refactoring/safeDelete/SafeDeleteJavaCalleeChooser.java +++ b/java/java-impl/src/com/intellij/refactoring/safeDelete/SafeDeleteJavaCalleeChooser.java @@ -15,17 +15,19 @@ */ package com.intellij.refactoring.safeDelete; +import com.intellij.ide.highlighter.JavaFileType; import com.intellij.java.refactoring.JavaRefactoringBundle; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Condition; import com.intellij.psi.*; import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.refactoring.changeSignature.CallerChooserBase; import com.intellij.refactoring.changeSignature.MemberNodeBase; -import com.intellij.refactoring.changeSignature.inCallers.AbstractJavaMemberCallerChooser; import com.intellij.refactoring.changeSignature.inCallers.JavaMemberNode; import com.intellij.refactoring.safeDelete.usageInfo.SafeDeleteMemberCalleeUsageInfo; import com.intellij.refactoring.safeDelete.usageInfo.SafeDeleteReferenceJavaDeleteUsageInfo; +import com.intellij.ui.ColoredTreeCellRenderer; import com.intellij.usageView.UsageInfo; import com.intellij.util.CommonProcessors; import com.intellij.util.containers.ContainerUtil; @@ -35,13 +37,13 @@ import org.jetbrains.annotations.Nullable; import java.util.*; import java.util.stream.Collectors; -abstract class SafeDeleteJavaCalleeChooser extends AbstractJavaMemberCallerChooser { +abstract class SafeDeleteJavaCalleeChooser extends CallerChooserBase { private final Project myProject; SafeDeleteJavaCalleeChooser(PsiMember member, Project project, ArrayList result) { - super(member, project, JavaRefactoringBundle.message("safe.delete.select.members.to.propagate.dialog.title"), null, members -> result.addAll(ContainerUtil.map(members, m -> { + super(member, project, JavaRefactoringBundle.message("safe.delete.select.members.to.propagate.dialog.title"), null, "dummy." + JavaFileType.INSTANCE.getDefaultExtension(), members -> result.addAll(ContainerUtil.map(members, m -> { return new SafeDeleteReferenceJavaDeleteUsageInfo(m, m, true); }))); myProject = project; @@ -49,19 +51,23 @@ abstract class SafeDeleteJavaCalleeChooser extends AbstractJavaMemberCallerChoos protected abstract ArrayList getTopLevelItems(); - @NotNull @Override - protected String getMemberTypePresentableText() { - return "member"; + protected String getEmptyCallerText() { + return "Caller text with highlighted callee would be shown here"; } @Override - protected PsiMember[] findDeepestSuperMethods(PsiMember method) { + protected String getEmptyCalleeText() { + return "Callee text would be shown here"; + } + + @Override + protected PsiElement[] findDeepestSuperMethods(PsiElement method) { return method instanceof PsiMethod ? ((PsiMethod)method).findDeepestSuperMethods() : PsiMember.EMPTY_ARRAY; } @Nullable - static List computeCalleesSafeToDelete(final PsiMember psiMember) { + static List computeCalleesSafeToDelete(final PsiMember psiMember) { final PsiElement body; if (psiMember instanceof PsiMethod) { body = ((PsiMethod)psiMember).getBody(); @@ -72,21 +78,42 @@ abstract class SafeDeleteJavaCalleeChooser extends AbstractJavaMemberCallerChoos if (body != null) { final PsiClass containingClass = psiMember.getContainingClass(); if (containingClass != null) { - final Set membersToCheck = new HashSet<>(); + final Set membersToCheck = new HashSet<>(); body.accept(new JavaRecursiveElementWalkingVisitor() { @Override public void visitReferenceExpression(PsiReferenceExpression expression) { super.visitReferenceExpression(expression); PsiElement resolved = expression.resolve(); if (resolved instanceof PsiMethod || resolved instanceof PsiField) { - ContainerUtil.addAllNotNull(membersToCheck, (PsiMember) resolved); + ContainerUtil.addAllNotNull(membersToCheck, resolved); + } + } + + @Override + public void visitLiteralExpression(PsiLiteralExpression expression) { + super.visitLiteralExpression(expression); + PsiReference @NotNull [] references = expression.getReferences(); + for (PsiReference reference : references) { + if (reference instanceof PsiPolyVariantReference) { + PsiElement[] nonMembers = Arrays.stream(((PsiPolyVariantReference)reference).multiResolve(false)) + .map(result -> result.getElement()) + .filter(e -> !(e instanceof PsiMember)) + .toArray(PsiElement[]::new); + ContainerUtil.addAllNotNull(membersToCheck, nonMembers); + } + else { + PsiElement resolve = reference.resolve(); + if (resolve != null && !(resolve instanceof PsiMember)) { + membersToCheck.add(resolve); + } + } } } }); return membersToCheck .stream() - .filter(m -> containingClass.equals(m.getContainingClass()) && !psiMember.equals(m)) + .filter(m -> !(m instanceof PsiMember) || containingClass.equals(((PsiMember)m).getContainingClass()) && !psiMember.equals(m)) .filter(m -> !(m instanceof PsiMethod) || ((PsiMethod)m).findDeepestSuperMethods().length == 0) .filter(m -> usedOnlyIn(m, psiMember)) .collect(Collectors.toList()); @@ -96,8 +123,8 @@ abstract class SafeDeleteJavaCalleeChooser extends AbstractJavaMemberCallerChoos } @Override - protected JavaMemberNode createTreeNodeFor(PsiMember nodeMethod, - HashSet callees, + protected MemberNodeBase createTreeNodeFor(PsiElement nodeMethod, + HashSet callees, Runnable cancelCallback) { final SafeDeleteJavaMemberNode node = new SafeDeleteJavaMemberNode(nodeMethod, callees, cancelCallback, nodeMethod != null ? nodeMethod.getProject() : myProject); if (getTopMember().equals(nodeMethod)) { @@ -108,36 +135,49 @@ abstract class SafeDeleteJavaCalleeChooser extends AbstractJavaMemberCallerChoos } @Override - protected MemberNodeBase getCalleeNode(MemberNodeBase node) { + protected MemberNodeBase getCalleeNode(MemberNodeBase node) { return node; } @Override - protected MemberNodeBase getCallerNode(MemberNodeBase node) { - return (MemberNodeBase)node.getParent(); + protected MemberNodeBase getCallerNode(MemberNodeBase node) { + return (MemberNodeBase)node.getParent(); } - private class SafeDeleteJavaMemberNode extends JavaMemberNode { + private class SafeDeleteJavaMemberNode extends MemberNodeBase { - SafeDeleteJavaMemberNode(PsiMember currentMember, - HashSet callees, - Runnable cancelCallback, - Project project) { + SafeDeleteJavaMemberNode(PsiElement currentMember, + HashSet callees, + Runnable cancelCallback, + Project project) { super(currentMember, callees, project, cancelCallback); } @Override - protected MemberNodeBase createNode(PsiMember caller, HashSet callees) { + protected void customizeRendererText(ColoredTreeCellRenderer renderer) { + PsiElement member = getMember(); + if (member instanceof PsiMember) { + JavaMemberNode.customizeRendererText(renderer, ((PsiMember)member), isEnabled()); + } + else { + renderer.append(member.getText()); //todo + } + } + + @Override + protected MemberNodeBase createNode(PsiElement caller, HashSet callees) { return new SafeDeleteJavaMemberNode(caller, callees, myCancelCallback, myProject); } @Override - protected List computeCallers() { - if (getTopMember().equals(getMember())) { - return ContainerUtil.map(getTopLevelItems(), info -> info.getCalledMember()); + protected List computeCallers() { + PsiElement member = getMember(); + if (getTopMember().equals(member)) { + return ContainerUtil.map(getTopLevelItems(), info -> info.getCalledElement()); } - final List callees = computeCalleesSafeToDelete(getMember()); + if (!(member instanceof PsiMember)) return Collections.emptyList(); + final List callees = computeCalleesSafeToDelete((PsiMember)member); if (callees != null) { callees.remove(getTopMember()); return callees; @@ -148,20 +188,20 @@ abstract class SafeDeleteJavaCalleeChooser extends AbstractJavaMemberCallerChoos } @Override - protected Condition getFilter() { + protected Condition getFilter() { return member -> !getMember().equals(member); } } - private static boolean usedOnlyIn(@NotNull PsiMember explored, @NotNull PsiMember place) { - return ReferencesSearch.search(explored).forEach( - new CommonProcessors.CollectProcessor() { - @Override - public boolean process(PsiReference reference) { - final PsiElement element = reference.getElement(); - return PsiTreeUtil.isAncestor(place, element, true) || - PsiTreeUtil.isAncestor(explored, element, true); - } - }); + private static boolean usedOnlyIn(@NotNull PsiElement explored, @NotNull PsiMember place) { + CommonProcessors.FindProcessor findProcessor = new CommonProcessors.FindProcessor() { + @Override + protected boolean accept(PsiReference reference) { + final PsiElement element = reference.getElement(); + return !PsiTreeUtil.isAncestor(place, element, true) && + !PsiTreeUtil.isAncestor(explored, element, true); + } + }; + return ReferencesSearch.search(explored).forEach(findProcessor); } } \ No newline at end of file diff --git a/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeleteMemberCalleeUsageInfo.java b/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeleteMemberCalleeUsageInfo.java index e0c722fd3355..30d05e3e819e 100644 --- a/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeleteMemberCalleeUsageInfo.java +++ b/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeleteMemberCalleeUsageInfo.java @@ -15,30 +15,31 @@ */ package com.intellij.refactoring.safeDelete.usageInfo; +import com.intellij.psi.PsiElement; import com.intellij.psi.PsiMember; import com.intellij.util.IncorrectOperationException; public class SafeDeleteMemberCalleeUsageInfo extends SafeDeleteUsageInfo implements SafeDeleteCustomUsageInfo { - private final PsiMember myCalledMember; + private final PsiElement myCalledElement; private final PsiMember myCallerMember; - public SafeDeleteMemberCalleeUsageInfo(PsiMember calledMember, PsiMember callerMember) { - super(calledMember, calledMember); - myCalledMember = calledMember; + public SafeDeleteMemberCalleeUsageInfo(PsiElement calledElement, PsiMember callerMember) { + super(calledElement, calledElement); + myCalledElement = calledElement; myCallerMember = callerMember; } @Override public void performRefactoring() throws IncorrectOperationException { - final PsiMember callee = myCalledMember; + final PsiElement callee = myCalledElement; if (callee != null && callee.isValid()) { callee.delete(); } } - public PsiMember getCalledMember() { - return myCalledMember; + public PsiElement getCalledElement() { + return myCalledElement; } public PsiMember getCallerMember() { diff --git a/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/after/Bundle.properties b/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/after/Bundle.properties new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/after/ClassWithInnerStaticImport.java b/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/after/ClassWithInnerStaticImport.java new file mode 100644 index 000000000000..6411d8db8aee --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/after/ClassWithInnerStaticImport.java @@ -0,0 +1,5 @@ +public class Foo { + +} + +class Bundle {} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/before/Bundle.properties b/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/before/Bundle.properties new file mode 100644 index 000000000000..90ef8de50b91 --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/before/Bundle.properties @@ -0,0 +1 @@ +a.b.c=used in bar only \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/before/ClassWithInnerStaticImport.java b/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/before/ClassWithInnerStaticImport.java new file mode 100644 index 000000000000..f64c7fec5a45 --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/deleteMethodWithPropertyUsage/before/ClassWithInnerStaticImport.java @@ -0,0 +1,9 @@ +public class Foo { + void bar() { + foo("a.b.c") + } + + void foo(@org.jetbrains.annotations.PropertyKey(resourceBundle = "Bundle") String key) { } +} + +class Bundle {} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/SafeDeleteTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/SafeDeleteTest.java index a516700134d4..37c5871b398c 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/SafeDeleteTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/SafeDeleteTest.java @@ -123,6 +123,10 @@ public class SafeDeleteTest extends MultiFileTestCase { doSingleFileTest(); } + public void testDeleteMethodWithPropertyUsage() { + doTest("Foo"); + } + public void testParameterInHierarchy() { doTest("C2"); }