[devkit] LightServiceMustBeFinal: suggest removing the 'Service' annotation on abstract classes and interfaces

IJ-CR-102194 GitOrigin-RevId: d7bf3203b32d9da35df621ed7cdc5c4c68c775c8
2026-09-27 10:03:11 +07:00 · 2023-04-13 00:38:41 +00:00
parent 1c3d684b64
commit 278a5b80a8
18 changed files with 174 additions and 18 deletions
@@ -21,6 +21,8 @@ abstract class JvmElementActionsFactory {

  open fun createAddAnnotationActions(target: JvmModifiersOwner, request: AnnotationRequest): List<IntentionAction> = emptyList()

+  open fun createRemoveAnnotationActions(target: JvmModifiersOwner, request: AnnotationRequest): List<IntentionAction> = emptyList()
+
  open fun createChangeAnnotationAttributeActions(annotation: JvmAnnotation,
                                                  attributeIndex: Int,
                                                  request: AnnotationAttributeRequest): List<IntentionAction> = emptyList()
@@ -35,6 +35,12 @@ fun createAddAnnotationActions(target: JvmModifiersOwner, request: AnnotationReq
  }
 }

+fun createRemoveAnnotationActions(target: JvmModifiersOwner, request: AnnotationRequest): List<IntentionAction> {
+  return createActions {
+    it.createRemoveAnnotationActions(target, request)
+  }
+}
+
 fun createChangeAnnotationAttributeActions(annotation: JvmAnnotation,
                                           attributeIndex: Int,
                                           request: AnnotationAttributeRequest): List<IntentionAction> {
@@ -171,6 +171,9 @@ remove.modifier.fix.family=Make not {0}
 remove.override.fix.family=Remove override
 remove.override.fix.text=Remove override annotation from method declaration

+remove.annotation.fix.family=Remove annotation
+remove.annotation.fix.text=Remove ''@{0}'' annotation
+
 change.inheritors.visibility.warning.text=Do you want to change inheritors' visibility to visibility of the base method?
 change.inheritors.visibility.warning.title=Change Inheritors
 move.class.in.extend.list.family=Move Class in Extend list
@@ -7,13 +7,17 @@ import com.intellij.codeInsight.intention.AddAnnotationPsiFix
 import com.intellij.codeInsight.intention.FileModifier
 import com.intellij.codeInsight.intention.IntentionAction
 import com.intellij.codeInsight.intention.preview.IntentionPreviewInfo
+import com.intellij.codeInspection.util.IntentionFamilyName
+import com.intellij.codeInspection.util.IntentionName
 import com.intellij.lang.java.JavaLanguage
 import com.intellij.lang.java.actions.*
 import com.intellij.lang.jvm.*
 import com.intellij.lang.jvm.actions.*
 import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.util.text.StringUtilRt
 import com.intellij.psi.*
+import com.intellij.psi.codeStyle.JavaCodeStyleManager
 import com.intellij.psi.util.PsiTreeUtil
 import com.intellij.psi.util.PsiUtil
 import com.intellij.refactoring.suggested.createSmartPointer
@@ -28,14 +32,17 @@ class JavaElementActionsFactory : JvmElementActionsFactory() {
    return listOf(ChangeModifierFix(declaration, request))
  }

-  private class RemoveAnnotationFix(private val fqn: String, element: PsiModifierListOwner) : IntentionAction {
+  private open class RemoveAnnotationFix(private val fqn: String,
+                                         element: PsiModifierListOwner,
+                                         @IntentionName private val text: String,
+                                         @IntentionFamilyName private val familyName: String) : IntentionAction {
    val pointer = element.createSmartPointer()

    override fun startInWriteAction(): Boolean = true

-    override fun getText(): String = QuickFixBundle.message("remove.override.fix.text")
+    override fun getText(): String = text

-    override fun getFamilyName(): String = QuickFixBundle.message("remove.override.fix.family")
+    override fun getFamilyName(): String = familyName

    override fun isAvailable(project: Project, editor: Editor?, file: PsiFile?): Boolean = pointer.element != null

@@ -50,16 +57,27 @@ class JavaElementActionsFactory : JvmElementActionsFactory() {

    private fun PsiModifierListOwner.deleteAnnotation() {
      getAnnotation(fqn)?.delete()
+      val file = this.containingFile as? PsiJavaFile ?: return
+      JavaCodeStyleManager.getInstance(project).removeRedundantImports(file)
    }
  }

+  private class RemoveOverrideAnnotationFix(element: PsiModifierListOwner) :
+    RemoveAnnotationFix(
+      CommonClassNames.JAVA_LANG_OVERRIDE,
+      element,
+      QuickFixBundle.message("remove.override.fix.text"),
+      QuickFixBundle.message("remove.override.fix.family")
+    )
+
  override fun createChangeOverrideActions(target: JvmModifiersOwner, shouldBePresent: Boolean): List<IntentionAction> {
    val psiElement = target.asSafely<PsiModifierListOwner>() ?: return emptyList()
    if (psiElement.language != JavaLanguage.INSTANCE) return emptyList()
    return if (shouldBePresent) {
      createAddAnnotationActions(target, annotationRequest(CommonClassNames.JAVA_LANG_OVERRIDE))
-    } else {
-      listOf(RemoveAnnotationFix(CommonClassNames.JAVA_LANG_OVERRIDE, psiElement))
+    }
+    else {
+      listOf(RemoveOverrideAnnotationFix(psiElement))
    }
  }

@@ -85,6 +103,15 @@ class JavaElementActionsFactory : JvmElementActionsFactory() {
    return listOf(CreateAnnotationAction(declaration, request))
  }

+  override fun createRemoveAnnotationActions(target: JvmModifiersOwner, request: AnnotationRequest): List<IntentionAction> {
+    val declaration = target as? PsiModifierListOwner ?: return emptyList()
+    if (declaration.language != JavaLanguage.INSTANCE) return emptyList()
+    val shortName = StringUtilRt.getShortName(request.qualifiedName)
+    val text = QuickFixBundle.message("remove.annotation.fix.text", shortName)
+    val familyName = QuickFixBundle.message("remove.annotation.fix.family")
+    return listOf(RemoveAnnotationFix(request.qualifiedName, target, text, familyName))
+  }
+
  override fun createChangeAnnotationAttributeActions(annotation: JvmAnnotation,
                                                      attributeIndex: Int,
                                                      request: AnnotationAttributeRequest): List<IntentionAction> {
@@ -1,19 +1,34 @@
 <html>
 <body>
 Reports classes annotated with the <code>@com.intellij.openapi.components.Service</code> annotation that are not final.
-<p>Suggests making a class final.</p>
+<p>
+  Suggests making a class final if it is concrete.</p>
 <p><b>Example:</b></p>
 <pre><code>
  // MyService.kt
  @Service(Service.Level.APP)
  open class MyService
 </code></pre>
-<p>After the quick fix is applied:</p>
+<p>After the quick-fix is applied:</p>
 <pre><code>
  // MyService.kt
  @Service(Service.Level.APP)
  class MyService
 </code></pre>
+<p>
+  Suggests removing the <code>@Service</code> annotation if it is an abstract class or interface.
+</p>
+<p><b>Example:</b></p>
+<pre><code>
+  // MyService.java
+  @Service(Service.Level.APP)
+  abstract class MyService {}
+</code></pre>
+<p>After the quick-fix is applied:</p>
+<pre><code>
+  // MyService.java
+  abstract class MyService {}
+</code></pre>
 <p><small>New in 2023.2</small>
 </body>
 </html>
@@ -618,6 +618,14 @@ inspection.message.obsolete.api.used=Obsolete API is used
 inspection.light.service.must.be.final.display.name=Light service must be final
 inspection.light.service.must.be.final.message=Light service must be final
 inspection.light.service.must.not.be.open.message=Light service must not be open
+inspection.light.service.must.be.concrete.class.message=\
+  Light service must be a concrete class and cannot be abstract or an interface.\n\
+  The IntelliJ Platform relies on the concrete implementation class to create and \
+  manage the service instance. Without a concrete implementation, the platform \
+  would not be able to create an instance of the service, and the service would \
+  not be available for use by the plugin.\n\
+  To solve this problem, you should define a concrete implementation class for the \
+  service and annotate it with 'com.intellij.openapi.components.Service'.

 inspection.mismatched.light.service.level.and.ctor.display.name=Mismatch between light service level and its constructor
 inspection.mismatched.light.service.level.and.ctor.project.level.required=Light service with a constructor that takes a single parameter of type 'com.intellij.openapi.project.Project' must specify '@Service(Service.Level.PROJECT)'
@@ -3,8 +3,13 @@ package org.jetbrains.idea.devkit.inspections

 import com.intellij.codeInspection.IntentionWrapper
 import com.intellij.codeInspection.ProblemHighlightType
-import com.intellij.lang.jvm.*
+import com.intellij.lang.jvm.DefaultJvmElementVisitor
+import com.intellij.lang.jvm.JvmClass
+import com.intellij.lang.jvm.JvmElementVisitor
+import com.intellij.lang.jvm.JvmModifier
+import com.intellij.lang.jvm.actions.annotationRequest
 import com.intellij.lang.jvm.actions.createModifierActions
+import com.intellij.lang.jvm.actions.createRemoveAnnotationActions
 import com.intellij.lang.jvm.actions.modifierRequest
 import com.intellij.openapi.components.Service
 import com.intellij.openapi.project.Project
@@ -16,14 +21,23 @@ internal class LightServiceMustBeFinalInspection : DevKitJvmInspection() {
  override fun buildVisitor(project: Project, sink: HighlightSink, isOnTheFly: Boolean): JvmElementVisitor<Boolean> {
    return object : DefaultJvmElementVisitor<Boolean> {
      override fun visitClass(clazz: JvmClass): Boolean {
-        if (clazz !is PsiClass) return true
-        if (clazz.classKind != JvmClassKind.CLASS || clazz.hasModifier(JvmModifier.FINAL)) return true
-        val file = clazz.sourceElement?.containingFile ?: return true
-        val hasServiceAnnotation = clazz.hasAnnotation(Service::class.java.canonicalName)
-        if (hasServiceAnnotation) {
-          val actions = createModifierActions(clazz, modifierRequest(JvmModifier.FINAL, true))
+        val sourceElement = clazz.sourceElement
+        if (sourceElement !is PsiClass) return true
+        if (sourceElement.isAnnotationType || sourceElement.isEnum || sourceElement.hasModifier(JvmModifier.FINAL)) return true
+        val file = sourceElement.containingFile ?: return true
+        val serviceAnnotation = sourceElement.getAnnotation(Service::class.java.canonicalName) ?: return true
+        val elementToReport = serviceAnnotation.nameReferenceElement ?: return true
+        if (sourceElement.isInterface || sourceElement.hasModifier(JvmModifier.ABSTRACT)) {
+          val actions = createRemoveAnnotationActions(sourceElement, annotationRequest(Service::class.java.canonicalName))
          val fixes = IntentionWrapper.wrapToQuickFixes(actions.toTypedArray(), file)
-          val message = when (clazz.language.id) {
+          val message = DevKitBundle.message("inspection.light.service.must.be.concrete.class.message")
+          val holder = (sink as HighlightSinkImpl).holder
+          holder.registerProblem(elementToReport, message, ProblemHighlightType.GENERIC_ERROR, *fixes)
+        }
+        else {
+          val actions = createModifierActions(sourceElement, modifierRequest(JvmModifier.FINAL, true))
+          val fixes = IntentionWrapper.wrapToQuickFixes(actions.toTypedArray(), file)
+          val message = when (sourceElement.language.id) {
            "kotlin" -> DevKitBundle.message("inspection.light.service.must.not.be.open.message")
            else -> DevKitBundle.message("inspection.light.service.must.be.final.message")
          }
@@ -0,0 +1,7 @@
+import com.intellij.openapi.components.Service;
+
+@<error descr="Light service must be a concrete class and cannot be abstract or an interface.
+The IntelliJ Platform relies on the concrete implementation class to create and manage the service instance. Without a concrete implementation, the platform would not be able to create an instance of the service, and the service would not be available for use by the plugin.
+To solve this problem, you should define a concrete implementation class for the service and annotate it with 'com.intellij.openapi.components.Service'.">Service<caret></error>
+abstract class MyService {
+}
@@ -0,0 +1,2 @@
+abstract class MyService {
+}
@@ -0,0 +1,7 @@
+import com.intellij.openapi.components.Service;
+
+@<error descr="Light service must be a concrete class and cannot be abstract or an interface.
+The IntelliJ Platform relies on the concrete implementation class to create and manage the service instance. Without a concrete implementation, the platform would not be able to create an instance of the service, and the service would not be available for use by the plugin.
+To solve this problem, you should define a concrete implementation class for the service and annotate it with 'com.intellij.openapi.components.Service'.">Service<caret></error>
+interface MyService {
+}
@@ -0,0 +1,2 @@
+interface MyService {
+}
@@ -2,6 +2,7 @@
 package org.jetbrains.idea.devkit.inspections

 import com.intellij.codeInsight.daemon.QuickFixBundle
+import com.intellij.openapi.components.Service
 import com.intellij.testFramework.TestDataPath
 import org.jetbrains.idea.devkit.DevkitJavaTestsUtil
 import org.jetbrains.idea.devkit.inspections.quickfix.LightServiceMustBeFinalInspectionTestBase
@@ -17,4 +18,8 @@ internal class LightServiceMustBeFinalInspectionTest : LightServiceMustBeFinalIn
  fun testMakeFinal() { doTest(MAKE_FINAL_FIX_NAME) }

  fun testMakeFinalMultiLineModifierList() { doTest(MAKE_FINAL_FIX_NAME) }
+
+  fun testAbstractClass() { doTest(QuickFixBundle.message("remove.annotation.fix.text", Service::class.java.simpleName)) }
+
+  fun testInterface() { doTest(QuickFixBundle.message("remove.annotation.fix.text", Service::class.java.simpleName)) }
 }
@@ -0,0 +1,6 @@
+import com.intellij.openapi.components.Service
+
+<error descr="Light service must be a concrete class and cannot be abstract or an interface.
+The IntelliJ Platform relies on the concrete implementation class to create and manage the service instance. Without a concrete implementation, the platform would not be able to create an instance of the service, and the service would not be available for use by the plugin.
+To solve this problem, you should define a concrete implementation class for the service and annotate it with 'com.intellij.openapi.components.Service'.">@Service<caret></error>
+abstract class MyService
@@ -0,0 +1 @@
+abstract class MyService
@@ -0,0 +1,6 @@
+import com.intellij.openapi.components.Service
+
+<error descr="Light service must be a concrete class and cannot be abstract or an interface.
+The IntelliJ Platform relies on the concrete implementation class to create and manage the service instance. Without a concrete implementation, the platform would not be able to create an instance of the service, and the service would not be available for use by the plugin.
+To solve this problem, you should define a concrete implementation class for the service and annotate it with 'com.intellij.openapi.components.Service'.">@Service<caret></error>
+interface MyService
@@ -0,0 +1 @@
+interface MyService
@@ -2,6 +2,7 @@
 package org.jetbrains.idea.devkit.kotlin.inspections

 import com.intellij.codeInsight.daemon.QuickFixBundle
+import com.intellij.openapi.components.Service
 import com.intellij.testFramework.TestDataPath
 import org.jetbrains.idea.devkit.inspections.quickfix.LightServiceMustBeFinalInspectionTestBase
 import org.jetbrains.idea.devkit.kotlin.DevkitKtTestsUtil
@@ -15,7 +16,9 @@ internal class KtLightServiceMustBeFinalInspectionTest : LightServiceMustBeFinal

  override fun getFileExtension() = "kt"

-  fun testMakeNotOpen() {
-    doTest(fixName)
-  }
+  fun testMakeNotOpen() { doTest(fixName) }
+
+  fun testAbstractClass() { doTest(QuickFixBundle.message("remove.annotation.fix.text", Service::class.java.simpleName)) }
+
+  fun testInterface() { doTest(QuickFixBundle.message("remove.annotation.fix.text", Service::class.java.simpleName)) }
 }
@@ -41,6 +41,7 @@ import org.jetbrains.kotlin.idea.quickfix.createFromUsage.callableBuilder.TypeIn
 import org.jetbrains.kotlin.idea.resolve.ResolutionFacade
 import org.jetbrains.kotlin.idea.util.CommentSaver
 import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
+import org.jetbrains.kotlin.idea.util.findAnnotation
 import org.jetbrains.kotlin.idea.util.resolveToKotlinType
 import org.jetbrains.kotlin.incremental.components.NoLookupLocation
 import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
@@ -171,6 +172,13 @@ class KotlinElementActionsFactory : JvmElementActionsFactory() {
        return createChangeModifierActions(kModifierOwner, KtTokens.OVERRIDE_KEYWORD, shouldBePresent)
    }

+    override fun createRemoveAnnotationActions(target: JvmModifiersOwner, request: AnnotationRequest): List<IntentionAction> {
+        val declaration = target.safeAs<KtLightElement<*, *>>()?.kotlinOrigin.safeAs<KtModifierListOwner>()?.takeIf {
+            it.language == KotlinLanguage.INSTANCE
+        } ?: return emptyList()
+        return listOf(RemoveAnnotationAction(declaration, request))
+    }
+
    override fun createChangeModifierActions(target: JvmModifiersOwner, request: ChangeModifierRequest): List<IntentionAction> {
        val kModifierOwner = target.toKtElement<KtModifierListOwner>() ?: return emptyList()

@@ -492,6 +500,39 @@ class KotlinElementActionsFactory : JvmElementActionsFactory() {
        }
    }

+    private class RemoveAnnotationAction(target: KtModifierListOwner, val request: AnnotationRequest) : IntentionAction {
+
+        private val pointer = target.createSmartPointer()
+
+        override fun startInWriteAction(): Boolean = true
+
+        override fun getText(): String {
+            val shortName = StringUtilRt.getShortName(request.qualifiedName)
+            return QuickFixBundle.message("remove.annotation.fix.text", shortName)
+        }
+
+        override fun getFamilyName(): String = QuickFixBundle.message("remove.annotation.fix.family")
+
+        override fun isAvailable(project: Project, editor: Editor, file: PsiFile): Boolean = pointer.element != null
+
+        override fun generatePreview(project: Project, editor: Editor, file: PsiFile): IntentionPreviewInfo {
+            PsiTreeUtil.findSameElementInCopy(pointer.element, file)?.removeAnnotation()
+            return IntentionPreviewInfo.DIFF
+        }
+
+        override fun invoke(project: Project, editor: Editor?, file: PsiFile?) {
+            pointer.element?.removeAnnotation()
+        }
+
+        private fun KtModifierListOwner.removeAnnotation() {
+            val annotationName = FqName(request.qualifiedName)
+            val annotation = this.findAnnotation(annotationName)
+            annotation?.delete() ?: return
+            val importList = (this.containingFile as? KtFile)?.importList
+            importList?.imports?.find { it.importedFqName == annotationName }?.delete()
+        }
+    }
+
    override fun createChangeParametersActions(target: JvmMethod, request: ChangeParametersRequest): List<IntentionAction> {
        return when (val kotlinOrigin = (target as? KtLightElement<*, *>)?.kotlinOrigin) {
            is KtNamedFunction -> listOfNotNull(ChangeMethodParameters.create(kotlinOrigin, request))