[java-inspections] AnnotateMethodFix: mod-command

Now, works for in-code annotations only (but may work in batch). Was used for MissingOverrideAnnotationInspection and NullableStuffInspectionBase. The later uses old code now, as external annotations could be desired there.

Part of IDEA-322693 Migrate as much as possible Java intentions and quick-fixes to ModCommand API

GitOrigin-RevId: f580467c5e64782a629bbe7a48b7f95686f6b83c

java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties

@@ -400,7 +400,7 @@ replace.with.lambda=Replace with lambda
 replace.with.boolean.equals=Replace with Boolean.equals
 report.suspicious.but.possibly.correct.method.calls=&Report suspicious but possibly correct method calls
 report.when.interface.is.not.annotated.with.functional.interface=Report when interface is not annotated with @FunctionalInterface
-searching.for.overriding.methods=Searching for overriding methods
+searching.for.overriding.methods=Searching for Overriding Methods
 statement.lambda.can.be.replaced.with.expression.lambda=Statement lambda can be replaced with expression lambda
 static.inheritrance.fix.replace.progress=Replacing usages of {0}
 static.member.guarded.by.instance.0.loc=Static member guarded by instance "{0}" #loc

java/java-analysis-impl/src/com/intellij/codeInspection/AnnotateMethodFix.java

@@ -2,48 +2,56 @@
 package com.intellij.codeInspection;
 
 import com.intellij.codeInsight.AnnotationUtil;
-import com.intellij.codeInsight.FileModificationService;
-import com.intellij.codeInsight.intention.AddAnnotationPsiFix;
-import com.intellij.codeInsight.intention.preview.IntentionPreviewInfo;
-import com.intellij.codeInspection.nullable.NullableStuffInspectionBase;
 import com.intellij.java.analysis.JavaAnalysisBundle;
-import com.intellij.openapi.application.ReadAction;
-import com.intellij.openapi.command.undo.UndoUtil;
+import com.intellij.modcommand.*;
 import com.intellij.openapi.diagnostic.Logger;
-import com.intellij.openapi.progress.ProgressManager;
 import com.intellij.openapi.project.Project;
+import com.intellij.psi.PsiAnnotation;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiMethod;
+import com.intellij.psi.PsiModifierList;
 import com.intellij.psi.search.searches.OverridingMethodsSearch;
 import com.intellij.psi.util.ClassUtil;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.util.ArrayUtilRt;
+import com.intellij.util.containers.ContainerUtil;
 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;
 
-public class AnnotateMethodFix implements LocalQuickFix {
+/**
+ * Annotate method in code only, not externally
+ */
+public class AnnotateMethodFix extends ModCommandQuickFix {
   private static final Logger LOG = Logger.getInstance(AnnotateMethodFix.class);
 
   protected final String myAnnotation;
+  private final boolean myAnnotateOverriddenMethods;
+  private final boolean myAnnotateSelf;
   private final String[] myAnnotationsToRemove;
 
+  @SuppressWarnings("unused") // used in third-party plugins
   public AnnotateMethodFix(@NotNull String fqn, String @NotNull ... annotationsToRemove) {
+    this(fqn, false, true, annotationsToRemove);
+  }
+  
+  public AnnotateMethodFix(@NotNull String fqn, boolean annotateOverriddenMethods, boolean annotateSelf, String @NotNull ... annotationsToRemove) {
     myAnnotation = fqn;
+    myAnnotateOverriddenMethods = annotateOverriddenMethods;
+    myAnnotateSelf = annotateSelf;
     myAnnotationsToRemove = annotationsToRemove.length == 0 ? ArrayUtilRt.EMPTY_STRING_ARRAY : annotationsToRemove;
-    LOG.assertTrue(annotateSelf() || annotateOverriddenMethods(), "annotate method quick fix should not do nothing");
+    LOG.assertTrue(myAnnotateSelf || myAnnotateOverriddenMethods, "annotate method quick fix should not do nothing");
   }
 
   @Override
   @NotNull
   public String getName() {
-    if (annotateSelf()) {
-      if (annotateOverriddenMethods()) {
+    if (myAnnotateSelf) {
+      if (myAnnotateOverriddenMethods) {
         return JavaAnalysisBundle.message("inspection.annotate.overridden.method.and.self.quickfix.name",
                                           ClassUtil.extractClassName(myAnnotation));
       }
@@ -56,8 +64,8 @@ public class AnnotateMethodFix implements LocalQuickFix {
   @Override
   @NotNull
   public String getFamilyName() {
-    if (annotateSelf()) {
-      if (annotateOverriddenMethods()) {
+    if (myAnnotateSelf) {
+      if (myAnnotateOverriddenMethods) {
         return JavaAnalysisBundle.message("inspection.annotate.overridden.method.and.self.quickfix.family.name");
       }
       return JavaAnalysisBundle.message("inspection.annotate.method.quickfix.family.name");
@@ -66,68 +74,43 @@ public class AnnotateMethodFix implements LocalQuickFix {
   }
 
   @Override
-  public boolean startInWriteAction() {
-    return false;
-  }
-
-  @Override
-  public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
+  public @NotNull ModCommand perform(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
     final PsiElement psiElement = descriptor.getPsiElement();
 
     PsiMethod method = PsiTreeUtil.getParentOfType(psiElement, PsiMethod.class);
-    if (method == null) return;
+    if (method == null) return ModCommand.nop();
     final List<PsiMethod> toAnnotate = new ArrayList<>();
-    if (annotateSelf()) {
+    if (myAnnotateSelf) {
       toAnnotate.add(method);
     }
 
-    if (annotateOverriddenMethods() && !processModifiableInheritorsUnderProgress(method, psiMethod -> {
-      if (AnnotationUtil.isAnnotatingApplicable(psiMethod, myAnnotation) &&
-          !AnnotationUtil.isAnnotated(psiMethod, myAnnotation, CHECK_EXTERNAL | CHECK_TYPE)) {
-        toAnnotate.add(psiMethod);
+    if (myAnnotateOverriddenMethods) {
+      for (PsiMethod inheritor : OverridingMethodsSearch.search(method)) {
+        if (AnnotationUtil.isAnnotatingApplicable(inheritor, myAnnotation) &&
+            !AnnotationUtil.isAnnotated(inheritor, myAnnotation, CHECK_EXTERNAL | CHECK_TYPE)) {
+          toAnnotate.add(inheritor);
+        }
       }
-    })) {
-      return;
     }
 
-    FileModificationService.getInstance().preparePsiElementsForWrite(toAnnotate);
-    for (PsiMethod psiMethod : toAnnotate) {
-      annotateMethod(psiMethod);
-    }
-    UndoUtil.markPsiFileForUndo(method.getContainingFile());
-  }
-
-  @Override
-  public @NotNull IntentionPreviewInfo generatePreview(@NotNull Project project, @NotNull ProblemDescriptor previewDescriptor) {
-    if (!annotateSelf()) return IntentionPreviewInfo.EMPTY;
-    PsiMethod method = PsiTreeUtil.getParentOfType(previewDescriptor.getPsiElement(), PsiMethod.class);
-    if (method == null) return IntentionPreviewInfo.EMPTY;
-    annotateMethod(method);
-    return IntentionPreviewInfo.DIFF;
-  }
-
-  protected boolean annotateOverriddenMethods() {
-    return false;
-  }
-
-  protected boolean annotateSelf() {
-    return true;
+    return ModCommand.psiUpdate(ActionContext.from(descriptor), updater -> {
+      for (PsiMethod psiMethod : ContainerUtil.map(toAnnotate, updater::getWritable)) {
+        annotateMethod(psiMethod);
+      }
+    });
   }
 
   private void annotateMethod(@NotNull PsiMethod method) {
-    AddAnnotationPsiFix fix = new AddAnnotationPsiFix(myAnnotation, method, 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);
+    PsiModifierList list = method.getModifierList();
+    if (myAnnotationsToRemove.length > 0) {
+      for (PsiAnnotation annotation : list.getAnnotations()) {
+        for (String fqn : myAnnotationsToRemove) {
+          if (annotation.hasQualifiedName(fqn)) {
+            annotation.delete();
           }
-        });
+        }
       }
-    }, JavaAnalysisBundle.message("searching.for.overriding.methods"), true, method.getProject());
+    }
+    list.addAnnotation(myAnnotation);
   }
 }

java/java-analysis-impl/src/com/intellij/codeInspection/RemoveAnnotationQuickFix.java

@@ -79,12 +79,12 @@ public class RemoveAnnotationQuickFix implements LocalQuickFix {
       Consumer<PsiModifierListOwner> inheritorProcessor = owner -> {
         registerAnnotation(AnnotationUtil.findAnnotation(owner, qualifiedName), owner, physical, externalOwners);
       };
-      if (listOwner instanceof PsiMethod &&
-          !AnnotateMethodFix.processModifiableInheritorsUnderProgress((PsiMethod)listOwner, inheritorProcessor)) {
+      if (listOwner instanceof PsiMethod method &&
+          !AnnotateOverriddenMethodParameterFix.processModifiableInheritorsUnderProgress(method, inheritorProcessor)) {
         return;
       }
-      if (listOwner instanceof PsiParameter &&
-               !AnnotateOverriddenMethodParameterFix.processParameterInheritorsUnderProgress((PsiParameter)listOwner, inheritorProcessor)) {
+      if (listOwner instanceof PsiParameter parameter &&
+          !AnnotateOverriddenMethodParameterFix.processParameterInheritorsUnderProgress(parameter, inheritorProcessor)) {
         return;
       }
     }

java/java-analysis-impl/src/com/intellij/codeInspection/nullable/AnnotateOverriddenMethodParameterFix.java

@@ -3,15 +3,17 @@ package com.intellij.codeInspection.nullable;
 
 import com.intellij.codeInsight.*;
 import com.intellij.codeInsight.intention.AddAnnotationPsiFix;
-import com.intellij.codeInspection.AnnotateMethodFix;
 import com.intellij.codeInspection.LocalQuickFix;
 import com.intellij.codeInspection.ProblemDescriptor;
 import com.intellij.java.analysis.JavaAnalysisBundle;
+import com.intellij.openapi.application.ReadAction;
+import com.intellij.openapi.progress.ProgressManager;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.PsiFile;
 import com.intellij.psi.PsiInvalidElementAccessException;
 import com.intellij.psi.PsiMethod;
 import com.intellij.psi.PsiParameter;
+import com.intellij.psi.search.searches.OverridingMethodsSearch;
 import com.intellij.psi.util.ClassUtil;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.util.ArrayUtil;
@@ -89,7 +91,7 @@ public class AnnotateOverriddenMethodParameterFix implements LocalQuickFix {
     PsiParameter[] parameters = method.getParameterList().getParameters();
     int index = ArrayUtilRt.find(parameters, parameter);
 
-    return AnnotateMethodFix.processModifiableInheritorsUnderProgress(method, psiMethod -> {
+    return processModifiableInheritorsUnderProgress(method, psiMethod -> {
       PsiParameter[] psiParameters = psiMethod.getParameterList().getParameters();
       if (index < psiParameters.length) {
         consumer.accept(psiParameters[index]);
@@ -102,4 +104,16 @@ public class AnnotateOverriddenMethodParameterFix implements LocalQuickFix {
   public String getFamilyName() {
     return JavaAnalysisBundle.message("annotate.overridden.methods.parameters.family.name");
   }
+
+  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);
+          }
+        });
+      }
+    }, JavaAnalysisBundle.message("searching.for.overriding.methods"), true, method.getProject());
+  }
 }

java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java

@@ -6,12 +6,14 @@ import com.intellij.codeInsight.daemon.impl.analysis.JavaGenericsUtil;
 import com.intellij.codeInsight.daemon.impl.quickfix.MoveAnnotationOnStaticMemberQualifyingTypeFix;
 import com.intellij.codeInsight.intention.AddAnnotationPsiFix;
 import com.intellij.codeInsight.intention.AddTypeAnnotationFix;
+import com.intellij.codeInsight.intention.preview.IntentionPreviewInfo;
 import com.intellij.codeInspection.*;
 import com.intellij.codeInspection.dataFlow.DfaPsiUtil;
 import com.intellij.codeInspection.dataFlow.java.inst.MethodCallInstruction;
 import com.intellij.codeInspection.util.InspectionMessage;
 import com.intellij.java.analysis.JavaAnalysisBundle;
 import com.intellij.modcommand.ModCommandAction;
+import com.intellij.openapi.command.undo.UndoUtil;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.roots.GeneratedSourcesFilter;
@@ -42,7 +44,9 @@ import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.PropertyKey;
 
 import java.util.*;
+import java.util.function.Consumer;
 
+import static com.intellij.codeInsight.AnnotationUtil.CHECK_EXTERNAL;
 import static com.intellij.codeInsight.AnnotationUtil.CHECK_HIERARCHY;
 import static com.intellij.codeInsight.AnnotationUtil.CHECK_TYPE;
 import static com.intellij.patterns.PsiJavaPatterns.psiElement;
@@ -1072,9 +1076,54 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection
     return null;
   }
 
-  private static class MyAnnotateMethodFix extends AnnotateMethodFix {
-    MyAnnotateMethodFix(String defaultNotNull, String[] annotationsToRemove) {
-      super(defaultNotNull, annotationsToRemove);
+  private static class MyAnnotateMethodFix implements LocalQuickFix {
+    protected final String myAnnotation;
+    private final String[] myAnnotationsToRemove;
+
+    MyAnnotateMethodFix(@NotNull String fqn, String @NotNull ... annotationsToRemove) {
+      myAnnotation = fqn;
+      myAnnotationsToRemove = annotationsToRemove.length == 0 ? ArrayUtilRt.EMPTY_STRING_ARRAY : annotationsToRemove;
+    }
+
+    @Override
+    @NotNull
+    public String getFamilyName() {
+      return JavaAnalysisBundle.message("inspection.annotate.overridden.method.quickfix.family.name");
+    }
+
+    @Override
+    public boolean startInWriteAction() {
+      return false;
+    }
+
+    @Override
+    public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
+      final PsiElement psiElement = descriptor.getPsiElement();
+
+      PsiMethod method = PsiTreeUtil.getParentOfType(psiElement, PsiMethod.class);
+      if (method == null) return;
+      final List<PsiMethod> toAnnotate = new ArrayList<>();
+
+      if (!AnnotateOverriddenMethodParameterFix.processModifiableInheritorsUnderProgress(method, (Consumer<? super PsiMethod>)psiMethod -> {
+        if (AnnotationUtil.isAnnotatingApplicable(psiMethod, myAnnotation) &&
+            !AnnotationUtil.isAnnotated(psiMethod, myAnnotation, CHECK_EXTERNAL | CHECK_TYPE)) {
+          toAnnotate.add(psiMethod);
+        }
+      })) {
+        return;
+      }
+
+      FileModificationService.getInstance().preparePsiElementsForWrite(toAnnotate);
+      for (PsiMethod psiMethod : toAnnotate) {
+        AddAnnotationPsiFix fix = new AddAnnotationPsiFix(myAnnotation, psiMethod, myAnnotationsToRemove);
+        fix.invoke(psiMethod.getProject(), psiMethod.getContainingFile(), psiMethod, psiMethod);
+      }
+      UndoUtil.markPsiFileForUndo(method.getContainingFile());
+    }
+
+    @Override
+    public @NotNull IntentionPreviewInfo generatePreview(@NotNull Project project, @NotNull ProblemDescriptor previewDescriptor) {
+      return IntentionPreviewInfo.EMPTY;
     }
 
     @Override
@@ -1082,15 +1131,5 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection
       return JavaAnalysisBundle.message("inspection.annotate.overridden.method.nullable.quickfix.name",
                                         ClassUtil.extractClassName(myAnnotation));
     }
-
-    @Override
-    protected boolean annotateOverriddenMethods() {
-      return true;
-    }
-
-    @Override
-    protected boolean annotateSelf() {
-      return false;
-    }
   }
 }

java/java-impl/src/com/siyeh/ig/inheritance/MissingOverrideAnnotationInspection.java

@@ -2,7 +2,6 @@
 package com.siyeh.ig.inheritance;
 
 import com.intellij.codeInsight.ExternalAnnotationsManager;
-import com.intellij.codeInsight.intention.AddAnnotationPsiFix;
 import com.intellij.codeInspection.AnnotateMethodFix;
 import com.intellij.codeInspection.CleanupLocalInspectionTool;
 import com.intellij.codeInspection.LocalQuickFix;
@@ -62,15 +61,14 @@ public class MissingOverrideAnnotationInspection extends BaseInspection implemen
 
   @Override
   protected LocalQuickFix buildFix(Object... infos) {
-    final PsiMethod method = (PsiMethod)infos[0];
-    final boolean annotateMethod = (boolean)infos[1];
-    final boolean annotateHierarchy = (boolean)infos[2];
-    return createAnnotateFix(method, annotateMethod, annotateHierarchy);
+    final boolean annotateMethod = (boolean)infos[0];
+    final boolean annotateHierarchy = (boolean)infos[1];
+    return createAnnotateFix(annotateMethod, annotateHierarchy);
   }
 
   @Override
   protected @NotNull String buildErrorString(Object... infos) {
-    final boolean annotateMethod = (boolean)infos[1];
+    final boolean annotateMethod = (boolean)infos[0];
     return InspectionGadgetsBundle.message(annotateMethod
                                            ? "missing.override.annotation.problem.descriptor"
                                            : "missing.override.annotation.in.overriding.problem.descriptor");
@@ -114,9 +112,9 @@ public class MissingOverrideAnnotationInspection extends BaseInspection implemen
         }
 
         final boolean annotateMethod = isMissingOverride(method);
-        final boolean annotateHierarchy = warnInSuper && isOnTheFly() && isMissingOverrideInOverriders(method);
+        final boolean annotateHierarchy = warnInSuper && isMissingOverrideInOverriders(method);
         if (annotateMethod || annotateHierarchy) {
-          registerMethodError(method, method, annotateMethod, annotateHierarchy);
+          registerMethodError(method, annotateMethod, annotateHierarchy);
         }
       }
 
@@ -175,7 +173,7 @@ public class MissingOverrideAnnotationInspection extends BaseInspection implemen
         return !annotations.isEmpty();
       }
 
-      private boolean isJdk6Override(PsiMethod method, PsiClass methodClass) {
+      private static boolean isJdk6Override(PsiMethod method, PsiClass methodClass) {
         final PsiMethod[] superMethods = method.findSuperMethods();
         boolean hasSupers = false;
         for (PsiMethod superMethod : superMethods) {
@@ -194,7 +192,7 @@ public class MissingOverrideAnnotationInspection extends BaseInspection implemen
         return hasSupers && !methodClass.isInterface();
       }
 
-      private boolean isJdk5Override(PsiMethod method, PsiClass methodClass) {
+      private static boolean isJdk5Override(PsiMethod method, PsiClass methodClass) {
         final PsiMethod[] superMethods = method.findSuperMethods();
         for (PsiMethod superMethod : superMethods) {
           final PsiClass superClass = superMethod.getContainingClass();
@@ -223,21 +221,8 @@ public class MissingOverrideAnnotationInspection extends BaseInspection implemen
   }
 
   @NotNull
-  private static LocalQuickFix createAnnotateFix(@NotNull PsiMethod method, boolean annotateMethod, boolean annotateHierarchy) {
-    if (!annotateHierarchy) {
-      return new AddAnnotationPsiFix(CommonClassNames.JAVA_LANG_OVERRIDE, method);
-    }
-    return new AnnotateMethodFix(CommonClassNames.JAVA_LANG_OVERRIDE) {
-      @Override
-      protected boolean annotateSelf() {
-        return annotateMethod;
-      }
-
-      @Override
-      protected boolean annotateOverriddenMethods() {
-        return true;
-      }
-    };
+  private static LocalQuickFix createAnnotateFix(boolean annotateMethod, boolean annotateHierarchy) {
+    return new AnnotateMethodFix(CommonClassNames.JAVA_LANG_OVERRIDE, annotateHierarchy, annotateMethod);
   }
 
   @Nullable

java/java-tests/testData/ig/com/siyeh/igtest/inheritance/missing_override_annotation/afterOverride.java

@@ -1,4 +1,4 @@
-// "Annotate method 'test()' as '@Override'" "true"
+// "Annotate method with '@Override'" "true"
 class Super {
   void test() {}
 }

java/java-tests/testData/ig/com/siyeh/igtest/inheritance/missing_override_annotation/beforeOverride.java

@@ -1,4 +1,4 @@
-// "Annotate method 'test()' as '@Override'" "true"
+// "Annotate method with '@Override'" "true"
 class Super {
   void test() {}
 }