IDEA-165845 Intention "Remove annotation" does not remove @Nonnull/@Nullable annotation from methods overriddes

GitOrigin-RevId: bce0718604b643df12bbc4e4e3e379091a6293aa
This commit is contained in:
peter
2019-10-16 19:46:14 +02:00
committed by intellij-monorepo-bot
parent 8abae98db4
commit 851124baf5
9 changed files with 160 additions and 45 deletions

View File

@@ -21,6 +21,7 @@ import org.jetbrains.annotations.NotNull;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Consumer;
import static com.intellij.codeInsight.AnnotationUtil.CHECK_EXTERNAL;
import static com.intellij.codeInsight.AnnotationUtil.CHECK_TYPE;
@@ -79,19 +80,12 @@ public class AnnotateMethodFix implements LocalQuickFix {
toAnnotate.add(method);
}
if (annotateOverriddenMethods() && !ProgressManager.getInstance().runProcessWithProgressSynchronously(() -> {
PsiMethod[] methods = OverridingMethodsSearch.search(method).toArray(PsiMethod.EMPTY_ARRAY);
for (PsiMethod psiMethod : methods) {
ReadAction.run(() -> {
if (psiMethod.isPhysical() &&
AnnotationUtil.isAnnotatingApplicable(psiMethod, myAnnotation) &&
!AnnotationUtil.isAnnotated(psiMethod, myAnnotation, CHECK_EXTERNAL | CHECK_TYPE) &&
!NullableStuffInspectionBase.shouldSkipOverriderAsGenerated(psiMethod)) {
toAnnotate.add(psiMethod);
}
});
if (annotateOverriddenMethods() && !processModifiableInheritorsUnderProgress(method, psiMethod -> {
if (AnnotationUtil.isAnnotatingApplicable(psiMethod, myAnnotation) &&
!AnnotationUtil.isAnnotated(psiMethod, myAnnotation, CHECK_EXTERNAL | CHECK_TYPE)) {
toAnnotate.add(psiMethod);
}
}, "Searching for Overriding Methods", true, project)) {
})) {
return;
}
@@ -114,4 +108,16 @@ public class AnnotateMethodFix implements LocalQuickFix {
AddAnnotationPsiFix fix = new AddAnnotationPsiFix(myAnnotation, method, PsiNameValuePair.EMPTY_ARRAY, myAnnotationsToRemove);
fix.invoke(method.getProject(), method.getContainingFile(), method, method);
}
public static boolean processModifiableInheritorsUnderProgress(@NotNull PsiMethod method, @NotNull Consumer<? super PsiMethod> consumer) {
return ProgressManager.getInstance().runProcessWithProgressSynchronously(() -> {
for (PsiMethod psiMethod : OverridingMethodsSearch.search(method)) {
ReadAction.run(() -> {
if (psiMethod.isPhysical() && !NullableStuffInspectionBase.shouldSkipOverriderAsGenerated(psiMethod)) {
consumer.accept(psiMethod);
}
});
}
}, "Searching for Overriding Methods", true, method.getProject());
}
}

View File

@@ -1,18 +1,22 @@
// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.codeInspection;
import com.intellij.codeInsight.AnnotationUtil;
import com.intellij.codeInsight.CodeInsightBundle;
import com.intellij.codeInsight.ExternalAnnotationsManager;
import com.intellij.codeInsight.FileModificationService;
import com.intellij.codeInspection.nullable.AnnotateOverriddenMethodParameterFix;
import com.intellij.openapi.application.WriteAction;
import com.intellij.openapi.project.Project;
import com.intellij.psi.PsiAnnotation;
import com.intellij.psi.PsiModifierListOwner;
import com.intellij.psi.SmartPointerManager;
import com.intellij.psi.SmartPsiElementPointer;
import com.intellij.psi.*;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Consumer;
/**
* @author yole
*/
@@ -27,6 +31,10 @@ public class RemoveAnnotationQuickFix implements LocalQuickFix {
myListOwner = listOwner == null ? null : pm.createSmartPsiElementPointer(listOwner);
}
protected boolean shouldRemoveInheritors() {
return false;
}
@Override
@NotNull
public String getFamilyName() {
@@ -42,16 +50,51 @@ public class RemoveAnnotationQuickFix implements LocalQuickFix {
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
PsiAnnotation annotation = myAnnotation.getElement();
if (annotation == null) return;
if (annotation.isPhysical()) {
if (!FileModificationService.getInstance().preparePsiElementForWrite(annotation)) return;
WriteAction.run(() -> annotation.delete());
PsiModifierListOwner listOwner = myListOwner == null ? null : myListOwner.getElement();
String qualifiedName = annotation.getQualifiedName();
List<PsiAnnotation> physical = new ArrayList<>();
List<PsiModifierListOwner> externalOwners = new ArrayList<>();
registerAnnotation(annotation, listOwner, physical, externalOwners);
if (shouldRemoveInheritors() && qualifiedName != null) {
Consumer<PsiModifierListOwner> inheritorProcessor = owner -> {
registerAnnotation(AnnotationUtil.findAnnotation(owner, qualifiedName), owner, physical, externalOwners);
};
if (listOwner instanceof PsiMethod &&
!AnnotateMethodFix.processModifiableInheritorsUnderProgress((PsiMethod)listOwner, inheritorProcessor)) {
return;
}
if (listOwner instanceof PsiParameter &&
!AnnotateOverriddenMethodParameterFix.processParameterInheritorsUnderProgress((PsiParameter)listOwner, inheritorProcessor)) {
return;
}
}
else {
PsiModifierListOwner listOwner = myListOwner.getElement();
String qualifiedName = annotation.getQualifiedName();
if (listOwner != null && qualifiedName != null) {
ExternalAnnotationsManager.getInstance(project).deannotate(listOwner, qualifiedName);
if (!FileModificationService.getInstance().preparePsiElementsForWrite(physical)) {
return;
}
WriteAction.run(() -> physical.forEach(PsiAnnotation::delete));
if (qualifiedName != null) {
for (PsiModifierListOwner owner : externalOwners) {
ExternalAnnotationsManager.getInstance(project).deannotate(owner, qualifiedName);
}
}
}
private static void registerAnnotation(@Nullable PsiAnnotation annotation,
@Nullable PsiModifierListOwner listOwner,
@NotNull List<PsiAnnotation> physical,
@NotNull List<PsiModifierListOwner> externalOwners) {
if (annotation == null) return;
if (annotation.isPhysical()) {
physical.add(annotation);
} else {
ContainerUtil.addIfNotNull(externalOwners, listOwner);
}
}
}

View File

@@ -18,12 +18,12 @@ package com.intellij.codeInspection.nullable;
import com.intellij.codeInsight.AnnotationUtil;
import com.intellij.codeInsight.FileModificationService;
import com.intellij.codeInsight.intention.AddAnnotationPsiFix;
import com.intellij.codeInspection.AnnotateMethodFix;
import com.intellij.codeInspection.InspectionsBundle;
import com.intellij.codeInspection.LocalQuickFix;
import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.search.searches.OverridingMethodsSearch;
import com.intellij.psi.util.ClassUtil;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.ArrayUtilRt;
@@ -32,11 +32,12 @@ import org.jetbrains.annotations.NotNull;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Consumer;
/**
* @author cdr
*/
class AnnotateOverriddenMethodParameterFix implements LocalQuickFix {
public class AnnotateOverriddenMethodParameterFix implements LocalQuickFix {
private final String myAnnotation;
private final String[] myAnnosToRemove;
@@ -58,27 +59,15 @@ class AnnotateOverriddenMethodParameterFix implements LocalQuickFix {
@Override
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
final PsiElement psiElement = descriptor.getPsiElement();
PsiParameter parameter = PsiTreeUtil.getParentOfType(psiElement, PsiParameter.class, false);
if (parameter == null) return;
PsiMethod method = PsiTreeUtil.getParentOfType(parameter, PsiMethod.class);
if (method == null) return;
PsiParameter[] parameters = method.getParameterList().getParameters();
int index = ArrayUtilRt.find(parameters, parameter);
List<PsiParameter> toAnnotate = new ArrayList<>();
PsiMethod[] methods = OverridingMethodsSearch.search(method).toArray(PsiMethod.EMPTY_ARRAY);
for (PsiMethod psiMethod : methods) {
if (NullableStuffInspectionBase.shouldSkipOverriderAsGenerated(psiMethod)) continue;
PsiParameter[] psiParameters = psiMethod.getParameterList().getParameters();
if (index >= psiParameters.length) continue;
if (AddAnnotationPsiFix.isAvailable(psiParameters[index], myAnnotation)) {
toAnnotate.add(psiParameters[index]);
PsiParameter parameter = PsiTreeUtil.getParentOfType(descriptor.getPsiElement(), PsiParameter.class, false);
if (parameter == null || !processParameterInheritorsUnderProgress(parameter, param -> {
if (AddAnnotationPsiFix.isAvailable(param, myAnnotation)) {
toAnnotate.add(param);
}
})) {
return;
}
FileModificationService.getInstance().preparePsiElementsForWrite(toAnnotate);
@@ -103,6 +92,20 @@ class AnnotateOverriddenMethodParameterFix implements LocalQuickFix {
}
}
public static boolean processParameterInheritorsUnderProgress(@NotNull PsiParameter parameter, @NotNull Consumer<? super PsiParameter> consumer) {
PsiMethod method = PsiTreeUtil.getParentOfType(parameter, PsiMethod.class);
if (method == null) return false;
PsiParameter[] parameters = method.getParameterList().getParameters();
int index = ArrayUtilRt.find(parameters, parameter);
return AnnotateMethodFix.processModifiableInheritorsUnderProgress(method, psiMethod -> {
PsiParameter[] psiParameters = psiMethod.getParameterList().getParameters();
if (index < psiParameters.length) {
consumer.accept(psiParameters[index]);
}
});
}
@Override
@NotNull
public String getFamilyName() {

View File

@@ -620,7 +620,12 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection
@Nullable PsiModifierListOwner listOwner) {
holder.registerProblem(!annotation.isPhysical() && listOwner != null ? listOwner.getNavigationElement() : annotation,
InspectionsBundle.message("inspection.nullable.problems.primitive.type.annotation"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new RemoveAnnotationQuickFix(annotation, listOwner));
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new RemoveAnnotationQuickFix(annotation, listOwner) {
@Override
protected boolean shouldRemoveInheritors() {
return true;
}
});
}
@Override

View File

@@ -0,0 +1,14 @@
import org.jetbrains.annotations.*;
interface Foo {
<warning descr="Primitive type members cannot be annotated">@Nul<caret>lable</warning>
long getTime();
}
class Bar implements Foo {
<warning descr="Primitive type members cannot be annotated">@Nullable</warning>
@Override
public long getTime() {
return 0;
}
}

View File

@@ -0,0 +1,12 @@
import org.jetbrains.annotations.*;
interface Foo {
long getTime();
}
class Bar implements Foo {
@Override
public long getTime() {
return 0;
}
}

View File

@@ -0,0 +1,9 @@
import org.jetbrains.annotations.*;
interface Foo {
void getTime(<warning descr="Primitive type members cannot be annotated">@Nul<caret>lable</warning> int a);
}
class Bar implements Foo {
public void getTime(<warning descr="Primitive type members cannot be annotated">@Nullable</warning> int a) {}
}

View File

@@ -0,0 +1,9 @@
import org.jetbrains.annotations.*;
interface Foo {
void getTime(int a);
}
class Bar implements Foo {
public void getTime(int a) {}
}

View File

@@ -287,6 +287,20 @@ public class NullableStuffInspectionTest extends LightJavaCodeInsightFixtureTest
myFixture.checkResultByFile(getTestName(false) + "_after.java");
}
public void testRemoveMethodAnnotationRemovesOverriders() {
myInspection.REPORT_ANNOTATION_NOT_PROPAGATED_TO_OVERRIDERS = true;
doTest();
myFixture.launchAction(myFixture.findSingleIntention("Remove annotation"));
myFixture.checkResultByFile(getTestName(false) + "_after.java");
}
public void testRemoveParameterAnnotationRemovesOverriders() {
myInspection.REPORT_ANNOTATION_NOT_PROPAGATED_TO_OVERRIDERS = true;
doTest();
myFixture.launchAction(myFixture.findSingleIntention("Remove annotation"));
myFixture.checkResultByFile(getTestName(false) + "_after.java");
}
public void testNullPassedToNullableParameter() {
doTest();
}