SealClassAction: rename, short circuited inheritors search, reject local inheritors

GitOrigin-RevId: 282a1fc9cbb4ccacca545d01bfc036007d7fdfe8

java/java-impl/src/META-INF/JavaPlugin.xml

@@ -1876,7 +1876,7 @@
       <category>Java/Declaration</category>
     </intentionAction>
     <intentionAction>
-      <className>com.intellij.codeInsight.intention.impl.MakeSealedAction</className>
+      <className>com.intellij.codeInsight.intention.impl.SealClassAction</className>
       <category>Java/Declaration</category>
     </intentionAction>
     <intentionAction>

java/java-impl/src/com/intellij/codeInsight/intention/impl/SealClassAction.java

@@ -3,6 +3,7 @@ package com.intellij.codeInsight.intention.impl;
 
 import com.intellij.codeInsight.FileModificationService;
 import com.intellij.codeInsight.daemon.impl.analysis.HighlightingFeature;
+import com.intellij.codeInsight.daemon.impl.analysis.JavaModuleGraphUtil;
 import com.intellij.codeInsight.intention.BaseElementAtCaretIntentionAction;
 import com.intellij.codeInspection.util.IntentionName;
 import com.intellij.java.JavaBundle;
@@ -15,18 +16,22 @@ import com.intellij.psi.*;
 import com.intellij.psi.search.searches.ClassInheritorsSearch;
 import com.intellij.psi.search.searches.FunctionalExpressionSearch;
 import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.psi.util.PsiUtil;
 import com.intellij.refactoring.util.CommonRefactoringUtil;
 import com.intellij.util.IncorrectOperationException;
 import com.intellij.util.SequentialModalProgressTask;
 import com.intellij.util.SequentialTask;
+import com.intellij.util.containers.ContainerUtil;
 import one.util.streamex.StreamEx;
 import org.jetbrains.annotations.NotNull;
 
-import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Objects;
-import java.util.stream.Stream;
 
-public class MakeSealedAction extends BaseElementAtCaretIntentionAction {
+import static com.intellij.util.ObjectUtils.tryCast;
+
+public class SealClassAction extends BaseElementAtCaretIntentionAction {
   @Override
   @NotNull
   public String getFamilyName() {
@@ -35,7 +40,7 @@ public class MakeSealedAction extends BaseElementAtCaretIntentionAction {
 
   @Override
   public @IntentionName @NotNull String getText() {
-    return JavaBundle.message("intention.name.make.sealed");
+    return getFamilyName();
   }
 
   @Override
@@ -52,10 +57,12 @@ public class MakeSealedAction extends BaseElementAtCaretIntentionAction {
     if (lBrace == null) return false;
     if (offset >= lBrace.getTextRange().getStartOffset()) return false;
     if (aClass.hasModifierProperty(PsiModifier.SEALED)) return false;
-    if (aClass.getImplementsList() == null) return false;
     if (aClass.getPermitsList() != null) return false;
     if (aClass.getModifierList() == null) return false;
     if (aClass.hasModifierProperty(PsiModifier.FINAL)) return false;
+    if (aClass.isEnum()) return false;
+    if (PsiUtil.isLocalOrAnonymousClass(aClass)) return false;
+    if (!(aClass.getContainingFile() instanceof PsiJavaFile)) return false;
     return !aClass.hasAnnotation(CommonClassNames.JAVA_LANG_FUNCTIONAL_INTERFACE);
   }
 
@@ -64,36 +71,73 @@ public class MakeSealedAction extends BaseElementAtCaretIntentionAction {
     PsiClass aClass = PsiTreeUtil.getParentOfType(element, PsiClass.class);
     if (aClass == null) return;
     if (!isAvailable(aClass, editor)) return;
-    FileModificationService.getInstance().prepareFileForWrite(aClass.getContainingFile());
+    PsiJavaFile parentFile = (PsiJavaFile)aClass.getContainingFile();
+    FileModificationService.getInstance().prepareFileForWrite(parentFile);
     if (aClass.isInterface()) {
       if (FunctionalExpressionSearch.search(aClass).findFirst() != null) {
-        String message = JavaBundle.message("intention.error.make.sealed.class.is.used.in.functional.expression");
-        CommonRefactoringUtil.showErrorHint(project, editor, message, getErrorTitle(), null);
+        showError(project, editor, "intention.error.make.sealed.class.is.used.in.functional.expression");
         return;
       }
     }
 
-    PsiClass[] inheritors = ClassInheritorsSearch.search(aClass, false).toArray(PsiClass.EMPTY_ARRAY);
-    String[] names = Stream.of(inheritors)
-      .map(aClass1 -> aClass1.getQualifiedName())
-      .toArray(String[]::new);
-    String[] nonNullNames = StreamEx.of(names).nonNull().toArray(String[]::new);
-    if (nonNullNames.length != names.length) {
-      String message = JavaBundle.message("intention.error.make.sealed.class.has.anonymous.inheritors");
-      CommonRefactoringUtil.showErrorHint(project, editor, message, getErrorTitle(), null);
-      return;
+    PsiJavaModule module = JavaModuleGraphUtil.findDescriptorByElement(aClass);
+
+    List<PsiClass> inheritors = new ArrayList<>();
+    for (PsiClass inheritor : ClassInheritorsSearch.search(aClass, false)) {
+      if (PsiUtil.isLocalOrAnonymousClass(inheritor)) {
+        showError(project, editor, "intention.error.make.sealed.class.has.anonymous.or.local.inheritors");
+        return;
+      }
+
+      if (module == null) {
+        PsiJavaFile file = tryCast(inheritor.getContainingFile(), PsiJavaFile.class);
+        if (file == null) {
+          showError(project, editor, "intention.error.make.sealed.class.inheritors.not.in.java.file");
+          return;
+        }
+        if (!parentFile.getPackageName().equals(file.getPackageName())) {
+          showError(project, editor, "intention.error.make.sealed.class.different.packages");
+          return;
+        }
+      }
+      else {
+        if (JavaModuleGraphUtil.findDescriptorByElement(inheritor) != module) {
+          showError(project, editor, "intention.error.make.sealed.class.different.modules");
+          return;
+        }
+      }
+
+      inheritors.add(inheritor);
     }
-    setParentModifier(aClass);
-    if (nonNullNames.length != 0) {
-      if (shouldCreatePermitsList(inheritors, aClass.getContainingFile())) {
-        addPermitsClause(project, aClass, nonNullNames);
+    List<String> names = ContainerUtil.map(inheritors, PsiClass::getQualifiedName);
+    @PsiModifier.ModifierConstant String modifier;
+    if (!names.isEmpty()) {
+      modifier = PsiModifier.SEALED;
+      if (shouldCreatePermitsList(inheritors, parentFile)) {
+        addPermitsClause(project, aClass, names);
       }
       setInheritorsModifiers(project, inheritors);
     }
+    else {
+      if (aClass.isInterface()) {
+        modifier = PsiModifier.SEALED;
+      }
+      else {
+        modifier = PsiModifier.FINAL;
+      }
+    }
+    ApplicationManager.getApplication().runWriteAction(() -> {
+      PsiModifierList modifierList = Objects.requireNonNull(aClass.getModifierList());
+      modifierList.setModifierProperty(modifier, true);
+    });
   }
 
-  public boolean shouldCreatePermitsList(PsiClass[] inheritors, PsiFile parentFile) {
-    return !Arrays.stream(inheritors).allMatch(psiClass -> psiClass.getContainingFile() == parentFile);
+  public void showError(@NotNull Project project, Editor editor, String message) {
+    CommonRefactoringUtil.showErrorHint(project, editor, JavaBundle.message(message), getErrorTitle(), null);
+  }
+
+  public boolean shouldCreatePermitsList(List<PsiClass> inheritors, PsiFile parentFile) {
+    return !inheritors.stream().allMatch(psiClass -> psiClass.getContainingFile() == parentFile);
   }
 
   public void setParentModifier(PsiClass aClass) {
@@ -106,12 +150,13 @@ public class MakeSealedAction extends BaseElementAtCaretIntentionAction {
     });
   }
 
-  public void setInheritorsModifiers(@NotNull Project project, PsiClass[] inheritors) {
+  public void setInheritorsModifiers(@NotNull Project project, List<PsiClass> inheritors) {
     String title = JavaBundle.message("intention.error.make.sealed.class.task.title.set.inheritors.modifiers");
     SequentialModalProgressTask task = new SequentialModalProgressTask(project, title, true);
     task.setTask(new SequentialTask() {
+      private final FileModificationService myFileModificationService = FileModificationService.getInstance();
       private int current = 0;
-      private final int size = inheritors.length;
+      private final int size = inheritors.size();
 
       @Override
       public boolean isDone() {
@@ -121,7 +166,7 @@ public class MakeSealedAction extends BaseElementAtCaretIntentionAction {
       @Override
       public boolean iteration() {
         task.getIndicator().setFraction(((double)current) / size);
-        PsiClass inheritor = inheritors[current];
+        PsiClass inheritor = inheritors.get(current);
         current++;
         PsiModifierList modifierList = inheritor.getModifierList();
         assert modifierList != null; // ensured by absence of anonymous classes
@@ -130,6 +175,7 @@ public class MakeSealedAction extends BaseElementAtCaretIntentionAction {
             modifierList.hasModifierProperty(PsiModifier.FINAL)) {
           return isDone();
         }
+        myFileModificationService.prepareFileForWrite(inheritor.getContainingFile());
         ApplicationManager.getApplication().runWriteAction(() -> {
           modifierList.setModifierProperty(PsiModifier.NON_SEALED, true);
         });
@@ -139,7 +185,7 @@ public class MakeSealedAction extends BaseElementAtCaretIntentionAction {
     ProgressManager.getInstance().run(task);
   }
 
-  public static void addPermitsClause(@NotNull Project project, PsiClass aClass, String[] nonNullNames) {
+  public static void addPermitsClause(@NotNull Project project, PsiClass aClass, List<String> nonNullNames) {
     String permitsClause = StreamEx.of(nonNullNames).sorted().joining(",", "permits ", "");
     PsiReferenceList permitsList = createPermitsClause(project, permitsClause);
     PsiReferenceList implementsList = Objects.requireNonNull(aClass.getImplementsList());

java/java-impl/src/intentionDescriptions/MakeSealedAction/description.html

@@ -1,6 +0,0 @@
-<html>
-<body>
-This intention makes the class sealed, lists all direct inheritors in permits clause if inheritors are not in the same file and
-adds 'non-sealed' modifier if none of 'sealed'/'non-sealed' modifiers were not specified for inheritors.
-</body>
-</html>

java/java-impl/src/intentionDescriptions/SealClassAction/description.html

@@ -0,0 +1,7 @@
+<html>
+<body>
+This intention makes the class sealed if it has inheritors or final otherwise,
+lists all direct inheritors in permits clause if inheritors are not in the same file and
+adds 'non-sealed' modifier if none of 'sealed'/'non-sealed'/'final' modifiers were specified for inheritors.
+</body>
+</html>

java/java-psi-impl/src/com/intellij/psi/impl/source/PsiModifierListImpl.java

@@ -235,6 +235,18 @@ public class PsiModifierListImpl extends JavaStubPsiElement<PsiModifierListStub>
         if (type == null) return;
       }
 
+      if (type == JavaTokenType.SEALED_KEYWORD || type == JavaTokenType.FINAL_KEYWORD || type == JavaTokenType.NON_SEALED_KEYWORD) {
+        if (type != JavaTokenType.SEALED_KEYWORD) {
+          setModifierProperty(SEALED, false);
+        }
+        if (type != JavaTokenType.NON_SEALED_KEYWORD) {
+          setModifierProperty(NON_SEALED, false);
+        }
+        if (type != JavaTokenType.FINAL_KEYWORD) {
+          setModifierProperty(FINAL, false);
+        }
+      }
+
       if (parent instanceof PsiField && grandParent instanceof PsiClass && ((PsiClass)grandParent).isInterface()) {
         if (type == JavaTokenType.PUBLIC_KEYWORD || type == JavaTokenType.STATIC_KEYWORD || type == JavaTokenType.FINAL_KEYWORD) return;
       }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/makeClassSealed/afterNoInheritors.java

@@ -1,3 +0,0 @@
-// "Make sealed" "true"
-
-public sealed class Main { }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/afterInheritorsWithModifiers.java

@@ -1,4 +1,4 @@
-// "Make sealed" "true"
+// "Seal class" "true"
 
 public sealed class Main { }
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/afterNoInheritors.java

@@ -0,0 +1,3 @@
+// "Seal class" "true"
+
+public final class Main { }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/afterNonDirect.java

@@ -1,4 +1,4 @@
-// "Make sealed" "true"
+// "Seal class" "true"
 
 public sealed class Main { }
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/afterNonSealed.java

@@ -1,4 +1,4 @@
-// "Make sealed" "true"
+// "Seal class" "true"
 
 public sealed class Main { }
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/beforeFinal.java

@@ -1,4 +1,4 @@
-// "Make sealed" "false"
+// "Seal class" "false"
 
 public final class Ma<caret>in { }
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/beforeInheritorsWithModifiers.java

@@ -1,4 +1,4 @@
-// "Make sealed" "true"
+// "Seal class" "true"
 
 public class Ma<caret>in { }
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/beforeNoInheritors.java

@@ -1,3 +1,3 @@
-// "Make sealed" "true"
+// "Seal class" "true"
 
 public class Ma<caret>in { }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/beforeNonDirect.java

@@ -1,4 +1,4 @@
-// "Make sealed" "true"
+// "Seal class" "true"
 
 public class Ma<caret>in { }
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/beforeNonSealed.java

@@ -1,4 +1,4 @@
-// "Make sealed" "true"
+// "Seal class" "true"
 
 public non-sealed class Ma<caret>in { }
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/beforeSealed.java

@@ -1,4 +1,4 @@
-// "Make sealed" "false"
+// "Seal class" "false"
 
 public sealed class Ma<caret>in { }
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/failing/DifferentPackages.java

@@ -0,0 +1,5 @@
+packages p;
+
+class Pa<caret>rent {
+
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/failing/LocalClass.java

@@ -0,0 +1,12 @@
+interface Par<caret>ent {
+}
+
+class Inheritor implements Parent {
+}
+
+class A {
+  void foo() {
+    class Local implements Parent {
+    }
+  }
+}

java/java-tests/testSrc/com/intellij/java/codeInsight/intention/SealClassActionFailingTest.java

@@ -1,16 +1,16 @@
 // Copyright 2000-2020 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.java.codeInsight.intention;
 
-import com.intellij.codeInsight.intention.impl.MakeSealedAction;
+import com.intellij.JavaTestUtil;
+import com.intellij.codeInsight.intention.impl.SealClassAction;
 import com.intellij.java.JavaBundle;
 import com.intellij.openapi.application.ApplicationManager;
 import com.intellij.refactoring.util.CommonRefactoringUtil;
-import com.intellij.testFramework.LightJavaCodeInsightTestCase;
 import com.intellij.testFramework.LightProjectDescriptor;
 import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase;
 import org.jetbrains.annotations.NotNull;
 
-public class MakeSealedActionFailingTest extends LightJavaCodeInsightTestCase {
+public class SealClassActionFailingTest extends LightJavaCodeInsightFixtureTestCase {
   @Override
   protected @NotNull LightProjectDescriptor getProjectDescriptor() {
     return LightJavaCodeInsightFixtureTestCase.JAVA_15;
@@ -21,12 +21,21 @@ public class MakeSealedActionFailingTest extends LightJavaCodeInsightTestCase {
   }
 
   public void testAnonymousClass() {
-    checkErrorMessage(JavaBundle.message("intention.error.make.sealed.class.has.anonymous.inheritors"));
+    checkErrorMessage(JavaBundle.message("intention.error.make.sealed.class.has.anonymous.or.local.inheritors"));
+  }
+
+  public void testLocalClass() {
+    checkErrorMessage(JavaBundle.message("intention.error.make.sealed.class.has.anonymous.or.local.inheritors"));
+  }
+
+  public void testDifferentPackages() {
+    myFixture.addFileToProject("foo.java", "package other;\n class Other extends Parent {}");
+    checkErrorMessage(JavaBundle.message("intention.error.make.sealed.class.different.packages"));
   }
 
   private void checkErrorMessage(@NotNull String message) {
-    configureByFile("/codeInsight/daemonCodeAnalyzer/quickFix/makeClassSealed/failing/" + getTestName(false) + ".java");
-    MakeSealedAction action = new MakeSealedAction();
+    myFixture.configureByFile(getTestName(false) + ".java");
+    SealClassAction action = new SealClassAction();
     assertTrue(action.isAvailable(getProject(), getEditor(), getFile()));
     try {
       ApplicationManager.getApplication().runWriteAction(() -> action.invoke(getProject(), getEditor(), getFile()));
@@ -36,4 +45,9 @@ public class MakeSealedActionFailingTest extends LightJavaCodeInsightTestCase {
     }
     fail("Test must fail with error message");
   }
+
+  @Override
+  protected String getBasePath() {
+    return JavaTestUtil.getRelativeJavaTestDataPath() + "/codeInsight/daemonCodeAnalyzer/quickFix/sealClass/failing/";
+  }
 }

java/java-tests/testSrc/com/intellij/java/codeInsight/intention/SealClassActionMultiFileTest.java

@@ -1,7 +1,7 @@
 // Copyright 2000-2020 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.java.codeInsight.intention;
 
-import com.intellij.codeInsight.intention.impl.MakeSealedAction;
+import com.intellij.codeInsight.intention.impl.SealClassAction;
 import com.intellij.openapi.command.WriteCommandAction;
 import com.intellij.psi.PsiClass;
 import com.intellij.psi.PsiFile;
@@ -9,7 +9,7 @@ import com.intellij.testFramework.LightProjectDescriptor;
 import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase;
 import org.jetbrains.annotations.NotNull;
 
-public class MakeSealedActionMultiFileTest extends LightJavaCodeInsightFixtureTestCase {
+public class SealClassActionMultiFileTest extends LightJavaCodeInsightFixtureTestCase {
   @Override
   protected @NotNull LightProjectDescriptor getProjectDescriptor() {
     return LightJavaCodeInsightFixtureTestCase.JAVA_15;
@@ -21,7 +21,7 @@ public class MakeSealedActionMultiFileTest extends LightJavaCodeInsightFixtureTe
     PsiClass direct2 = myFixture.addClass("class Direct2 extends Main {}");
     PsiClass indirect = myFixture.addClass("class Indirect extends Direct1 {}");
 
-    MakeSealedAction action = new MakeSealedAction();
+    SealClassAction action = new SealClassAction();
     assertTrue(action.isAvailable(getProject(), getEditor(), getFile()));
     WriteCommandAction.runWriteCommandAction(getProject(), () -> {
       action.invoke(getProject(), getEditor(), getFile());

java/java-tests/testSrc/com/intellij/java/codeInsight/intention/SealClassActionTest.java

@@ -6,7 +6,7 @@ import com.intellij.testFramework.LightProjectDescriptor;
 import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase;
 import org.jetbrains.annotations.NotNull;
 
-public class MakeSealedActionTest extends LightIntentionActionTestCase {
+public class SealClassActionTest extends LightIntentionActionTestCase {
   @Override
   protected @NotNull LightProjectDescriptor getProjectDescriptor() {
     return LightJavaCodeInsightFixtureTestCase.JAVA_15;
@@ -14,6 +14,6 @@ public class MakeSealedActionTest extends LightIntentionActionTestCase {
 
   @Override
   protected String getBasePath() {
-    return "/codeInsight/daemonCodeAnalyzer/quickFix/makeClassSealed";
+    return "/codeInsight/daemonCodeAnalyzer/quickFix/sealClass";
   }
 }

java/openapi/resources/messages/JavaBundle.properties

@@ -1261,9 +1261,12 @@ slice.usage.message.tracking.container.contents=(Tracking container ''{0}{1}'' c
 slice.usage.message.location=in {0}
 intention.name.move.into.if.branches=Move up into 'if' statement branches
 intention.name.collapse.into.loop=Collapse into loop
-intention.family.name.make.sealed=Make class sealed
+intention.family.name.make.sealed=Seal class
 intention.name.make.sealed=Make sealed
 intention.error.make.sealed.class.is.used.in.functional.expression=Class is used in functional expression
 intention.error.make.sealed.class.hint.title=Make Sealed
-intention.error.make.sealed.class.has.anonymous.inheritors=Some of the inheritors are anonymous
+intention.error.make.sealed.class.has.anonymous.or.local.inheritors=Some of the inheritors are anonymous or local
+intention.error.make.sealed.class.different.packages=Module is unnamed and some of the inheritors are in the different package
+intention.error.make.sealed.class.inheritors.not.in.java.file=Some of the inheritors are not in java files
+intention.error.make.sealed.class.different.modules=Some of the inheritors are in different modules
 intention.error.make.sealed.class.task.title.set.inheritors.modifiers=Setting inheritors modifiers