IDEA-171317 ImplicitSubclassInspection moved to CI

java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplementedAtRuntimeCondition.java

@@ -21,7 +21,7 @@ import org.jetbrains.annotations.NotNull;
 
 /**
  * @author nik
- * @deprecated use {@link ImplicitSubclassProvider}
+ * @deprecated use {@link ImplicitSubclassProvider} (to remove in 2018.1)
  */
 @Deprecated
 public abstract class ImplementedAtRuntimeCondition {

java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplicitSubclassInspection.kt

@@ -0,0 +1,152 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.intellij.codeInspection.inheritance
+
+import com.intellij.codeInsight.daemon.QuickFixBundle
+import com.intellij.codeInspection.*
+import com.intellij.openapi.project.Project
+import com.intellij.psi.*
+import com.intellij.util.SmartList
+import org.jetbrains.uast.UClass
+import org.jetbrains.uast.UDeclaration
+
+class ImplicitSubclassInspection : AbstractBaseUastLocalInspectionTool() {
+
+  override fun checkClass(aClass: UClass, manager: InspectionManager, isOnTheFly: Boolean): Array<ProblemDescriptor>? {
+
+    val classIsFinal = aClass.isFinal || aClass.hasModifierProperty(PsiModifier.PRIVATE)
+
+    val problems = SmartList<ProblemDescriptor>()
+
+    val subclassers = ImplicitSubclassProvider.EP_NAME.extensions
+      .asSequence()
+      .filter { it.isApplicableTo(aClass) }
+
+    val methodsToOverride = aClass.methods.mapNotNull {
+      method ->
+      subclassers
+        .mapNotNull { it.findOverridingReason(method) }
+        .firstOrNull()?.let { description ->
+        method to description
+      }
+    }
+
+    val classLevelFix =
+      if (classIsFinal) {
+        val classReasonToBeSubclassed = subclassers.mapNotNull { it.findSubclassingReason(aClass) }.firstOrNull()
+
+        if (methodsToOverride.isNotEmpty() || classReasonToBeSubclassed != null) {
+          val classLevelFix = FixSubclassing(aClass, aClass.name ?: "class")
+          val classFixes = if (aClass.modifierList?.isWritable ?: false)
+            arrayOf<LocalQuickFix>(classLevelFix)
+          else emptyArray()
+
+          problemTargets(aClass).forEach {
+            problems.add(manager.createProblemDescriptor(
+              it, classReasonToBeSubclassed ?: InspectionsBundle.message("inspection.implicitsubclass.display.forClass", aClass.name), isOnTheFly,
+              classFixes,
+              ProblemHighlightType.GENERIC_ERROR_OR_WARNING)
+            )
+          }
+
+          classLevelFix
+        }
+        else null
+      }
+      else null
+
+    for ((method, description) in methodsToOverride) {
+      if (method.isFinal || method.isStatic || method.hasModifierProperty(PsiModifier.PRIVATE)) {
+
+        classLevelFix?.siblings?.add(method)
+
+        val methodFixes = if (method.modifierList.isWritable)
+          arrayOf<LocalQuickFix>(FixSubclassing(method, method.name))
+        else
+          emptyArray()
+
+        problemTargets(method).forEach {
+          problems.add(manager.createProblemDescriptor(
+            it, description, isOnTheFly,
+            methodFixes,
+            ProblemHighlightType.GENERIC_ERROR_OR_WARNING))
+        }
+      }
+    }
+
+    return problems.toTypedArray()
+  }
+
+  private fun problemTargets(declaration: UDeclaration): List<PsiElement> {
+
+    val modifiersElements = declaration.modifierList?.let { highlightableModifiersElements(it) } ?: emptyList<PsiElement>()
+
+    if (modifiersElements.isEmpty())
+      return (declaration as? PsiNameIdentifierOwner)?.nameIdentifier?.let { listOf(it) } ?: emptyList<PsiElement>()
+    else
+      return modifiersElements
+  }
+
+  private val highlightableModifiersSet = setOf(PsiModifier.FINAL, PsiModifier.PRIVATE, PsiModifier.STATIC)
+
+  private fun highlightableModifiersElements(memberModifierList: PsiModifierList): List<PsiElement> = memberModifierList.getChildren().filter {
+    it is PsiKeyword && highlightableModifiersSet.contains(it.getText())
+  }
+
+
+  private class FixSubclassing(val uDeclaration: UDeclaration, val hintName: String) : LocalQuickFixOnPsiElement(uDeclaration) {
+
+    val siblings = SmartList<UDeclaration>()
+
+    override fun getFamilyName(): String = QuickFixBundle.message("fix.modifiers.family")
+
+    override fun getText() = if (uDeclaration is UClass)
+      InspectionsBundle.message("inspection.implicitsubclass.make.class.extendable")
+    else
+      InspectionsBundle.message("inspection.implicitsubclass.extendable", hintName)
+
+
+    override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) {
+      makeExtendable(startElement as UDeclaration)
+      for (sibling in siblings) {
+        makeExtendable(sibling)
+      }
+    }
+
+
+    private fun makeExtendable(declaration: UDeclaration) {
+      val isClassMember = !(declaration is UClass)
+
+      declaration.modifierList?.apply {
+        setModifierProperty(PsiModifier.FINAL, false)
+        setModifierProperty(PsiModifier.PRIVATE, false)
+        if (isClassMember) {
+          setModifierProperty(PsiModifier.STATIC, false)
+        }
+      }
+
+      if (isClassMember) {
+        (declaration.uastParent as? UClass)?.modifierList?.apply {
+          setModifierProperty(PsiModifier.FINAL, false)
+          setModifierProperty(PsiModifier.PRIVATE, false)
+        }
+      }
+    }
+
+
+  }
+
+}

java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplicitSubclassProvider.java

@@ -39,11 +39,10 @@ public abstract class ImplicitSubclassProvider {
    * <b>Note:<b/> this check is expected to be cheap. If it requires long computations then it is better just to return true.
    *
    * @param psiClass a class to check for possible subclass
-   * @return {@code false} if this ImplicitSubclassProvider implementation have no possibility
-   * to create an subclass for the psiClass and all further checks could be skipped;
-   * {@code true} if this ImplicitSubclassProvider implementation could provide a subclass,
+   * @return {@code false} if definitely no subclass will be created for the psiClass and all further checks could be skipped;
+   * {@code true} if a subclass for the psiClass will probably be created,
    * and then you should check {@link #findOverridingReason(PsiMethod)} and {@link #findOverridingReason(PsiMethod)}
-   * methods to find out are there concrete reasons for this class to be subclassed.
+   * methods to find out if there are concrete reasons for the class to be subclassed.
    */
   public abstract boolean isApplicableTo(@NotNull PsiClass psiClass);
 
@@ -52,7 +51,7 @@ public abstract class ImplicitSubclassProvider {
    * <b>Note:<b/> this method could be computationally costly because in some cases it could require deep annotations checks
    * not only for class but also for all it's methods.
    * default implementations doesn't check methods, so implementors should override this methods if target framework
-   * makes decision about overriding basing methods annotations
+   * makes decision about overriding basing on methods annotations
    *
    * @param psiClass a class to check for possible subclass
    * @return true if class will be subclassed, false - otherwise

platform/platform-resources-en/src/messages/InspectionsBundle.properties

@@ -825,4 +825,10 @@ inspection.reflection.invocation.argument.not.assignable=Argument is not assigna
 inspection.reflection.invocation.item.not.assignable=Array item is not assignable to ''{0}''
 inspection.reflection.invocation.array.not.assignable=Array {0,choice,1#item has|1<items have} incompatible {0,choice,1#type|1<types}
 
+inspection.implicitsubclass.display.forClass=Class ''{0}'' is eligible for implicit subclassing and should not be final
+inspection.implicitsubclass.display.forMethod=Method ''{0}'' should be overridable
+inspection.implicitsubclass.make.class.extendable=Make class open for extension
+inspection.implicitsubclass.extendable=Make ''{0}'' overridable
+inspection.implicitsubclass.display.name=Class could be implicitly overridden by framework
+
 inspection.reflection.visibility.name=Reflective access across modules issues

resources-en/src/inspectionDescriptions/ImplicitSubclassInspection.html

@@ -0,0 +1,6 @@
+<html>
+<body>
+Reports if class is eligible for being subclassed at runtime by some framework (like Spring or Hibernate),
+but something in code prevents this class from being subclassed or method from being overridden.
+</body>
+</html>

resources/src/META-INF/IdeaPlugin.xml

@@ -812,6 +812,12 @@
                      groupKey="group.names.inheritance.issues" enabledByDefault="false" level="WARNING"
                      implementationClass="com.intellij.codeInspection.inheritance.SuperClassHasFrequentlyUsedInheritorsInspection"
                      displayName="Class may extend a commonly used base class instead of implementing interface"/>
+    <localInspection language="UAST" shortName="ImplicitSubclassInspection"
+                     bundle="messages.InspectionsBundle" key="inspection.implicitsubclass.display.name"
+                     groupBundle="messages.InspectionsBundle" groupKey="group.names.inheritance.issues"
+                     groupPath="Uast"
+                     enabledByDefault="true" level="WARNING"
+                     implementationClass="com.intellij.codeInspection.inheritance.ImplicitSubclassInspection"/>
     <localInspection groupPath="Java" language="JAVA" shortName="BlockMarkerComments"
                      groupBundle="messages.InspectionsBundle"
                      groupKey="group.names.code.style.issues" enabledByDefault="false" level="WARNING"