[java-inspections] Experimental ModCommand-based DeletePrivateMethodFix for unused inspection

GitOrigin-RevId: efd9306505048695f1aa1de1851383e170b0c1ca
This commit is contained in:
Tagir Valeev
2024-03-12 10:57:50 +01:00
committed by intellij-monorepo-bot
parent b9d88ce85d
commit 193b2cb477
14 changed files with 323 additions and 122 deletions

View File

@@ -405,6 +405,12 @@ public abstract class QuickFixFactory {
@NotNull
public abstract IntentionAction createSafeDeleteFix(@NotNull PsiElement element);
/**
* @param method method to delete
* @return a fix to remove private method, possibly along with called methods unused elsewhere
*/
public abstract @NotNull ModCommandAction createDeletePrivateMethodFix(@NotNull PsiMethod method);
public abstract @NotNull List<@NotNull LocalQuickFix> registerOrderEntryFixes(@NotNull PsiReference reference,
@NotNull List<? super IntentionAction> registrar);

View File

@@ -176,10 +176,11 @@ class PostHighlightingVisitor extends JavaElementVisitor {
return custom != null ? custom.apply(currentProfile).getInspectionProfile() : currentProfile;
}
private String message;
@NlsContexts.DetailedDescription private String message;
private final List<IntentionAction> quickFixes = new ArrayList<>();
private final List<IntentionAction> quickFixOptions = new ArrayList<>();
@Override
public void visitLocalVariable(@NotNull PsiLocalVariable variable) {
if (myUnusedSymbolInspection.LOCAL_VARIABLE) {
processLocalVariable(variable);
@@ -330,7 +331,7 @@ class PostHighlightingVisitor extends JavaElementVisitor {
quickFixes.add(QuickFixFactory.getInstance().createAddToImplicitlyWrittenFieldsFix(project, annoName)));
}
}
else if (!UnusedSymbolUtil.isFieldUsed(myProject, myFile, field, ProgressManager.getGlobalProgressIndicator(), myGlobalUsageHelper)) {
else if (!UnusedSymbolUtil.isFieldUsed(myProject, myFile, field, ProgressIndicatorProvider.getGlobalProgressIndicator(), myGlobalUsageHelper)) {
if (UnusedSymbolUtil.isImplicitWrite(myProject, field)) {
message = getNotUsedForReadingMessage(field);
quickFixes.add(QuickFixFactory.getInstance().createSafeDeleteFix(field));
@@ -481,9 +482,13 @@ class PostHighlightingVisitor extends JavaElementVisitor {
int options = PsiFormatUtilBase.SHOW_TYPE | PsiFormatUtilBase.SHOW_FQ_CLASS_NAMES;
String symbolName = HighlightMessageUtil.getSymbolName(method, PsiSubstitutor.EMPTY, options);
message = JavaErrorBundle.message(key, symbolName);
quickFixes.add(QuickFixFactory.getInstance().createSafeDeleteFix(method));
QuickFixFactory factory = QuickFixFactory.getInstance();
quickFixes.add(factory.createSafeDeleteFix(method));
if (ApplicationManager.getApplication().isHeadlessEnvironment() && method.hasModifierProperty(PsiModifier.PRIVATE)) {
quickFixes.add(factory.createDeletePrivateMethodFix(method).asIntention());
}
SpecialAnnotationsUtilBase.processUnknownAnnotations(method, annoName ->
quickFixes.add(QuickFixFactory.getInstance().createAddToDependencyInjectionAnnotationsFix(project, annoName)));
quickFixes.add(factory.createAddToDependencyInjectionAnnotationsFix(project, annoName)));
}
private void processClass(@NotNull Project project, @NotNull PsiClass aClass) {

View File

@@ -4,6 +4,7 @@ package com.intellij.refactoring.safeDelete;
import com.intellij.codeInsight.AnnotationUtil;
import com.intellij.codeInsight.ExternalAnnotationsManager;
import com.intellij.codeInsight.daemon.impl.quickfix.RemoveUnusedVariableUtil;
import com.intellij.codeInsight.daemon.impl.quickfix.SafeDeleteFix;
import com.intellij.codeInsight.generation.GetterSetterPrototypeProvider;
import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil;
import com.intellij.find.findUsages.PsiElement2UsageTargetAdapter;
@@ -679,13 +680,11 @@ public class JavaSafeDeleteProcessor extends SafeDeleteProcessorDelegateBase {
}
private static void appendCallees(@NotNull PsiMember method, @NotNull List<? super UsageInfo> usages) {
List<PsiElement> calleesSafeToDelete = SafeDeleteJavaCalleeChooser.computeReferencedCodeSafeToDelete(method);
if (calleesSafeToDelete != null) {
List<PsiElement> calleesSafeToDelete = SafeDeleteFix.computeReferencedCodeSafeToDelete(method);
for (PsiElement callee : calleesSafeToDelete) {
usages.add(new SafeDeleteMemberCalleeUsageInfo(callee, method));
}
}
}
private static void findFunctionalExpressions(List<? super UsageInfo> usages, PsiMethod... methods) {
for (PsiMethod method : methods) {

View File

@@ -1,18 +1,17 @@
// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package com.intellij.refactoring.safeDelete;
import com.intellij.codeInspection.deadCode.UnusedDeclarationInspectionBase;
import com.intellij.codeInsight.daemon.impl.quickfix.SafeDeleteFix;
import com.intellij.ide.highlighter.JavaFileType;
import com.intellij.java.JavaBundle;
import com.intellij.java.refactoring.JavaRefactoringBundle;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Condition;
import com.intellij.openapi.util.NlsContexts;
import com.intellij.psi.*;
import com.intellij.psi.search.GlobalSearchScope;
import com.intellij.psi.search.PsiSearchHelper;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.ElementDescriptionUtil;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiMember;
import com.intellij.psi.PsiMethod;
import com.intellij.refactoring.changeSignature.CallerChooserBase;
import com.intellij.refactoring.changeSignature.MemberNodeBase;
import com.intellij.refactoring.changeSignature.inCallers.JavaMemberNode;
@@ -21,14 +20,13 @@ import com.intellij.refactoring.safeDelete.usageInfo.SafeDeleteReferenceJavaDele
import com.intellij.ui.ColoredTreeCellRenderer;
import com.intellij.usageView.UsageInfo;
import com.intellij.usageView.UsageViewShortNameLocation;
import com.intellij.util.CommonProcessors;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.*;
import java.util.stream.Collectors;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
abstract class SafeDeleteJavaCalleeChooser extends CallerChooserBase<PsiElement> {
SafeDeleteJavaCalleeChooser(@NotNull PsiMember member,
@@ -56,72 +54,6 @@ abstract class SafeDeleteJavaCalleeChooser extends CallerChooserBase<PsiElement>
return method instanceof PsiMethod ? ((PsiMethod)method).findDeepestSuperMethods() : PsiMember.EMPTY_ARRAY;
}
@Nullable
static List<PsiElement> computeReferencedCodeSafeToDelete(final PsiMember psiMember) {
final PsiElement body;
if (psiMember instanceof PsiMethod) {
body = ((PsiMethod)psiMember).getBody();
}
else if (psiMember instanceof PsiField) {
body = ((PsiField)psiMember).getInitializer();
}
else if (psiMember instanceof PsiClass) {
body = psiMember;
}
else {
body = null;
}
if (body != null) {
final PsiClass containingClass = psiMember.getContainingClass();
final Set<PsiElement> elementsToCheck = new HashSet<>();
body.accept(new JavaRecursiveElementWalkingVisitor() {
@Override
public void visitReferenceExpression(@NotNull PsiReferenceExpression expression) {
super.visitReferenceExpression(expression);
PsiElement resolved = expression.resolve();
if (resolved instanceof PsiMethod || resolved instanceof PsiField) {
ContainerUtil.addAllNotNull(elementsToCheck, resolved);
}
}
@Override
public void visitLiteralExpression(@NotNull 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);
if (nonMembers.length < 10) {
ContainerUtil.addAllNotNull(elementsToCheck, nonMembers);
}
}
else {
PsiElement resolve = reference.resolve();
if (resolve != null && !(resolve instanceof PsiMember)) {
elementsToCheck.add(resolve);
}
}
}
}
});
PsiFile containingFile = body.getContainingFile();
return elementsToCheck
.stream()
.filter(m -> m != containingFile)
.filter(m -> !PsiTreeUtil.isAncestor(psiMember, m, true))
.filter(m -> !(m instanceof PsiMember) || containingClass != null && containingClass.equals(((PsiMember)m).getContainingClass()) && !psiMember.equals(m))
.filter(m -> !(m instanceof PsiMethod) || ((PsiMethod)m).findDeepestSuperMethods().length == 0)
.filter(m -> m.isPhysical())
.filter(m -> usedOnlyIn(m, psiMember))
.collect(Collectors.toList());
}
return null;
}
@Override
protected MemberNodeBase<PsiElement> createTreeNodeFor(PsiElement nodeMethod,
HashSet<PsiElement> callees,
@@ -182,46 +114,14 @@ abstract class SafeDeleteJavaCalleeChooser extends CallerChooserBase<PsiElement>
}
if (!(member instanceof PsiMember)) return Collections.emptyList();
final List<PsiElement> callees = computeReferencedCodeSafeToDelete((PsiMember)member);
if (callees != null) {
final List<PsiElement> callees = SafeDeleteFix.computeReferencedCodeSafeToDelete((PsiMember)member);
callees.remove(getTopMember());
return callees;
}
else {
return Collections.emptyList();
}
}
@Override
protected Condition<PsiElement> getFilter() {
return member -> !getMember().equals(member);
}
}
private static boolean usedOnlyIn(@NotNull PsiElement explored, @NotNull PsiMember place) {
if (explored instanceof PsiNamedElement) {
final String name = ((PsiNamedElement)explored).getName();
if (name != null &&
PsiSearchHelper.getInstance(explored.getProject())
.isCheapEnoughToSearch(name, GlobalSearchScope.projectScope(explored.getProject()), null, null) == PsiSearchHelper.SearchCostResult.TOO_MANY_OCCURRENCES) {
return false;
}
}
if (explored instanceof PsiClassOwner) {
for (PsiClass aClass : ((PsiClassOwner)explored).getClasses()) {
if (!usedOnlyIn(aClass, place)) return false;
}
return true;
}
if (UnusedDeclarationInspectionBase.isDeclaredAsEntryPoint(explored)) return false;
CommonProcessors.FindProcessor<PsiReference> 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);
}
}

View File

@@ -0,0 +1,87 @@
// Copyright 2000-2024 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.java.JavaBundle;
import com.intellij.modcommand.ActionContext;
import com.intellij.modcommand.ModCommand;
import com.intellij.modcommand.Presentation;
import com.intellij.modcommand.PsiBasedModCommandAction;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiMethod;
import com.intellij.psi.PsiModifier;
import com.intellij.util.ThreeState;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.NotNull;
import java.util.*;
public final class DeletePrivateMethodFix extends PsiBasedModCommandAction<PsiMethod> {
private final ThreeState myDeleteCalled;
public DeletePrivateMethodFix() {
super(PsiMethod.class);
myDeleteCalled = ThreeState.UNSURE;
}
public DeletePrivateMethodFix(@NotNull PsiMethod method) {
this(method, ThreeState.UNSURE);
}
private DeletePrivateMethodFix(@NotNull PsiMethod method, ThreeState deleteCalled) {
super(method);
myDeleteCalled = deleteCalled;
}
@Override
protected @NotNull Presentation getPresentation(@NotNull ActionContext context, @NotNull PsiMethod method) {
return switch (myDeleteCalled) {
case UNSURE -> Presentation.of(JavaBundle.message("intention.name.delete.method", method.getName()));
case YES -> Presentation.of(JavaBundle.message("intention.name.delete.method.with.callees"));
case NO -> Presentation.of(JavaBundle.message("intention.name.delete.method.only"));
};
}
@Override
protected @NotNull ModCommand perform(@NotNull ActionContext context, @NotNull PsiMethod method) {
if (!method.hasModifierProperty(PsiModifier.PRIVATE)) return ModCommand.nop();
List<PsiElement> elements;
if (myDeleteCalled == ThreeState.NO) {
elements = List.of();
}
else {
Set<PsiElement> finalElements = new HashSet<>();
Deque<PsiMethod> toProcess = new ArrayDeque<>();
toProcess.add(method);
finalElements.add(method);
while (!toProcess.isEmpty()) {
PsiMethod next = toProcess.poll();
List<PsiMethod> newMethods = ContainerUtil.filterIsInstance(SafeDeleteFix.computeReferencedCodeSafeToDelete(
next, t -> t instanceof PsiMethod m && m.hasModifierProperty(PsiModifier.PRIVATE)), PsiMethod.class);
for (PsiMethod newMethod : newMethods) {
if (finalElements.add(newMethod)) {
toProcess.add(newMethod);
}
}
}
elements = List.copyOf(finalElements);
}
if (elements.isEmpty()) {
return ModCommand.psiUpdate(method, m -> m.delete());
}
if (myDeleteCalled == ThreeState.UNSURE) {
return ModCommand.chooseAction(JavaBundle.message("intention.name.delete.method.title", method.getName()),
new DeletePrivateMethodFix(method, ThreeState.YES),
new DeletePrivateMethodFix(method, ThreeState.NO));
}
return ModCommand.psiUpdate(method, (m, updater) -> {
List<PsiElement> writable = ContainerUtil.map(elements, updater::getWritable);
writable.forEach(PsiElement::delete);
m.delete();
});
}
@Override
public @NotNull String getFamilyName() {
return JavaBundle.message("intention.family.name.delete.private.method");
}
}

View File

@@ -6,17 +6,28 @@ import com.intellij.codeInsight.daemon.QuickFixBundle;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightMessageUtil;
import com.intellij.codeInsight.intention.preview.IntentionPreviewInfo;
import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement;
import com.intellij.codeInspection.deadCode.UnusedDeclarationInspectionBase;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Predicates;
import com.intellij.psi.*;
import com.intellij.psi.search.GlobalSearchScope;
import com.intellij.psi.search.PsiSearchHelper;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.util.PsiFormatUtilBase;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.refactoring.safeDelete.SafeDeleteHandler;
import com.intellij.refactoring.safeDelete.SafeDeleteProcessor;
import com.intellij.util.CommonProcessors;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.*;
import java.util.function.Predicate;
import java.util.stream.Collectors;
public class SafeDeleteFix extends LocalQuickFixAndIntentionActionOnPsiElement {
public SafeDeleteFix(@NotNull PsiElement element) {
super(element);
@@ -82,4 +93,109 @@ public class SafeDeleteFix extends LocalQuickFixAndIntentionActionOnPsiElement {
public boolean startInWriteAction() {
return false;
}
@NotNull
public static List<PsiElement> computeReferencedCodeSafeToDelete(@Nullable PsiMember psiMember) {
return computeReferencedCodeSafeToDelete(psiMember, Predicates.alwaysTrue());
}
@NotNull
public static List<PsiElement> computeReferencedCodeSafeToDelete(
@Nullable PsiMember psiMember,
@NotNull Predicate<? super PsiElement> additionalFilter) {
final PsiElement body;
if (psiMember instanceof PsiMethod method) {
body = method.getBody();
}
else if (psiMember instanceof PsiField field) {
body = field.getInitializer();
}
else if (psiMember instanceof PsiClass) {
body = psiMember;
}
else {
body = null;
}
if (body == null) {
// Do not use List.of(), as call sites like to delete from the resulting list
return Collections.emptyList();
}
final PsiClass containingClass = psiMember.getContainingClass();
final Set<PsiElement> elementsToCheck = new HashSet<>();
body.accept(new JavaRecursiveElementWalkingVisitor() {
@Override
public void visitReferenceExpression(@NotNull PsiReferenceExpression expression) {
super.visitReferenceExpression(expression);
PsiElement resolved = expression.resolve();
if (resolved instanceof PsiMethod || resolved instanceof PsiField) {
ContainerUtil.addAllNotNull(elementsToCheck, resolved);
}
}
@Override
public void visitLiteralExpression(@NotNull PsiLiteralExpression expression) {
super.visitLiteralExpression(expression);
PsiReference @NotNull [] references = expression.getReferences();
for (PsiReference reference : references) {
if (reference instanceof PsiPolyVariantReference ref) {
PsiElement[] nonMembers = Arrays.stream(ref.multiResolve(false))
.map(result -> result.getElement())
.filter(e -> !(e instanceof PsiMember))
.toArray(PsiElement[]::new);
if (nonMembers.length < 10) {
ContainerUtil.addAllNotNull(elementsToCheck, nonMembers);
}
}
else {
PsiElement resolve = reference.resolve();
if (resolve != null && !(resolve instanceof PsiMember)) {
elementsToCheck.add(resolve);
}
}
}
}
});
PsiFile containingFile = body.getContainingFile();
return elementsToCheck
.stream()
.filter(additionalFilter)
.filter(m -> m != containingFile)
.filter(m -> !PsiTreeUtil.isAncestor(psiMember, m, true))
.filter(m -> !(m instanceof PsiMember member) ||
containingClass != null && containingClass.equals(member.getContainingClass()) && !psiMember.equals(m))
.filter(m -> !(m instanceof PsiMethod method) ||
method.findDeepestSuperMethods().length == 0)
.filter(m -> m.isPhysical())
.filter(m -> usedOnlyIn(m, psiMember))
.collect(Collectors.toList());
}
private static boolean usedOnlyIn(@NotNull PsiElement explored, @NotNull PsiMember place) {
if (explored instanceof PsiNamedElement namedElement) {
final String name = namedElement.getName();
if (name != null &&
PsiSearchHelper.getInstance(explored.getProject())
.isCheapEnoughToSearch(name, GlobalSearchScope.projectScope(explored.getProject()), null, null) ==
PsiSearchHelper.SearchCostResult.TOO_MANY_OCCURRENCES) {
return false;
}
}
if (explored instanceof PsiClassOwner classOwner) {
for (PsiClass aClass : classOwner.getClasses()) {
if (!usedOnlyIn(aClass, place)) return false;
}
return true;
}
if (UnusedDeclarationInspectionBase.isDeclaredAsEntryPoint(explored)) return false;
CommonProcessors.FindProcessor<PsiReference> 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);
}
}

View File

@@ -798,6 +798,11 @@ public final class QuickFixFactoryImpl extends QuickFixFactory {
return new SafeDeleteFix(element);
}
@Override
public @NotNull ModCommandAction createDeletePrivateMethodFix(@NotNull PsiMethod method) {
return new DeletePrivateMethodFix(method);
}
@Override
public @NotNull List<@NotNull LocalQuickFix> registerOrderEntryFixes(@NotNull PsiReference reference, @NotNull List<? super IntentionAction> registrar) {
return OrderEntryFix.registerFixes(reference, registrar);

View File

@@ -0,0 +1,16 @@
// "Delete method 'test()'|->... along with other private methods used only there" "true"
class X {
private void test2() {
}
public void test3() {
test4();
}
private void test4() {
}
}

View File

@@ -0,0 +1,3 @@
// "Delete method 'test()'" "true"
class X {
}

View File

@@ -0,0 +1,30 @@
// "Delete method 'test()'|->... along with other private methods used only there" "true"
class X {
private void t<caret>est() {
test1();
test2();
test3();
test4();
}
private void test1() {
test2();
test5();
}
private void test2() {
}
public void test3() {
test4();
}
private void test4() {
}
private void test5() {
}
}

View File

@@ -0,0 +1,6 @@
// "Delete method 'test()'" "true"
class X {
private void t<caret>est() {
}
}

View File

@@ -0,0 +1,19 @@
// Copyright 2000-2024 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;
import com.intellij.codeInspection.deadCode.UnusedDeclarationInspection;
public class DeletePrivateMethodFixTest extends LightQuickFixParameterizedTestCase {
@Override
protected void setUp() throws Exception {
super.setUp();
enableInspectionTool(new UnusedDeclarationInspection(true));
}
@Override
protected String getBasePath() {
return "/codeInsight/daemonCodeAnalyzer/quickFix/deletePrivateMethod";
}
}

View File

@@ -1929,3 +1929,8 @@ invalid.compilation.notification.action.rebuild.error.title=Can''t Rebuild Modul
inspection.mapping.before.count.family.name=Mapping call before count()
inspection.mapping.before.count.message=The ''{0}()'' call does not change the final count and might be optimized out.
unknown.library=Unknown Library
intention.name.delete.method=Delete method ''{0}()''
intention.name.delete.method.title=Delete Method ''{0}()''
intention.name.delete.method.with.callees=... along with other private methods used only there
intention.name.delete.method.only=... and nothing else
intention.family.name.delete.private.method=Delete private method

View File

@@ -18,6 +18,7 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Collection;
import java.util.Objects;
import java.util.function.Supplier;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -141,7 +142,10 @@ public final class ActionHint {
if(myShouldPresent) {
if(result == null) {
fail(exceptionHeader(lastStep) + " not found\nAvailable actions: " +
actions.stream().map(IntentionAction::getText).collect(Collectors.joining(", ", "[", "]\n")) +
commonActions.stream().map(ca -> {
return ca instanceof ModCommandAction mca && context != null ? Objects.requireNonNull(mca.getPresentation(context)).name() :
ca.asIntention().getText();
}).collect(Collectors.joining(", ", "[", "]\n")) +
infoSupplier.get());
}
else if(myHighlightType != null) {