From c0931ffe1f1cdd99b2e006902c11f06bf18bffa9 Mon Sep 17 00:00:00 2001 From: Nicolay Mitropolsky Date: Wed, 19 Apr 2017 20:04:24 +0300 Subject: [PATCH] IDEA-171317 `ImplicitSubclassInspection` moved to CI --- .../ImplementedAtRuntimeCondition.java | 2 +- .../inheritance/ImplicitSubclassInspection.kt | 152 ++++++++++++++++++ .../inheritance/ImplicitSubclassProvider.java | 9 +- .../src/messages/InspectionsBundle.properties | 6 + .../ImplicitSubclassInspection.html | 6 + resources/src/META-INF/IdeaPlugin.xml | 6 + 6 files changed, 175 insertions(+), 6 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplicitSubclassInspection.kt create mode 100644 resources-en/src/inspectionDescriptions/ImplicitSubclassInspection.html diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplementedAtRuntimeCondition.java b/java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplementedAtRuntimeCondition.java index bacd115c54e0..1e79658b3bd9 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplementedAtRuntimeCondition.java +++ b/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 { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplicitSubclassInspection.kt b/java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplicitSubclassInspection.kt new file mode 100644 index 000000000000..8880ae4cf8a5 --- /dev/null +++ b/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? { + + val classIsFinal = aClass.isFinal || aClass.hasModifierProperty(PsiModifier.PRIVATE) + + val problems = SmartList() + + 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(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(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 { + + val modifiersElements = declaration.modifierList?.let { highlightableModifiersElements(it) } ?: emptyList() + + if (modifiersElements.isEmpty()) + return (declaration as? PsiNameIdentifierOwner)?.nameIdentifier?.let { listOf(it) } ?: emptyList() + else + return modifiersElements + } + + private val highlightableModifiersSet = setOf(PsiModifier.FINAL, PsiModifier.PRIVATE, PsiModifier.STATIC) + + private fun highlightableModifiersElements(memberModifierList: PsiModifierList): List = memberModifierList.getChildren().filter { + it is PsiKeyword && highlightableModifiersSet.contains(it.getText()) + } + + + private class FixSubclassing(val uDeclaration: UDeclaration, val hintName: String) : LocalQuickFixOnPsiElement(uDeclaration) { + + val siblings = SmartList() + + 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) + } + } + } + + + } + +} \ No newline at end of file diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplicitSubclassProvider.java b/java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplicitSubclassProvider.java index c41e868b4c87..b8d384596a2c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplicitSubclassProvider.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/inheritance/ImplicitSubclassProvider.java @@ -39,11 +39,10 @@ public abstract class ImplicitSubclassProvider { * Note: 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 { * Note: 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 diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index eac595468de6..4710fd0d8657 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/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 + +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. + + \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 467a053359bc..b11ac5702d15 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/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"/> +