From e6c99824c6af95a43d7e02564a8c9d2df8e876f9 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 28 Feb 2024 17:27:52 +0100 Subject: [PATCH 01/31] Add ASM Method utilities --- .../verifiers/resolution/Methods.kt | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt new file mode 100644 index 000000000..ad875466c --- /dev/null +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt @@ -0,0 +1,63 @@ +package com.jetbrains.pluginverifier.verifiers.resolution + +import com.jetbrains.plugin.structure.classes.resolvers.ResolutionResult +import com.jetbrains.plugin.structure.classes.resolvers.Resolver +import org.objectweb.asm.tree.ClassNode +import org.slf4j.Logger +import org.slf4j.LoggerFactory + +private val LOG: Logger = LoggerFactory.getLogger(Method::class.java) + +fun Method.isOverriding(anotherMethod: Method): Boolean = + nonStaticAndNonFinal(anotherMethod) + && sameName(this, anotherMethod) + && sameParametersAndReturnType(this, anotherMethod) + && sameOrBroaderVisibility(this, anotherMethod) + +private fun nonStaticAndNonFinal(anotherMethod: Method) = !anotherMethod.isStatic && !anotherMethod.isFinal + +private fun sameName(method: Method, anotherMethod: Method) = method.name == anotherMethod.name + +private fun sameParametersAndReturnType(method: Method, anotherMethod: Method) = + method.descriptor == anotherMethod.descriptor + +private fun sameOrBroaderVisibility(method: Method, anotherMethod: Method): Boolean { + return method.visibilityRating >= anotherMethod.visibilityRating +} + +private val Method.visibilityRating: Int + get() = when { + isPublic -> 3 + isProtected -> 2 + isPackagePrivate -> 1 + isPrivate -> 0 + else -> -1 + } + +fun Method.search(resolver: Resolver, matchHandler: (ClassFile, Method) -> Unit) { + var superClassFqn = containingClassFile.superName ?: return + while (true) { + val superClass = when(val resolvedClass = resolver.resolveClass(superClassFqn)) { + is ResolutionResult.Found -> ClassFileAsm(resolvedClass.value, resolvedClass.fileOrigin) + else -> return + } + findInSuperClass(superClass)?.let {overriddenMethod -> + matchHandler(superClass, overriddenMethod) + } + superClassFqn = superClass.superName ?: return + } +} + +private fun Method.findInSuperClass(superClass: ClassFileAsm): Method? { + val superClassMethodCandidates = superClass.methods.filter { superMethod -> + isOverriding(superMethod) + }.toList() + return when { + superClassMethodCandidates.isEmpty() -> null + superClassMethodCandidates.size == 1 -> superClassMethodCandidates.first() + else -> { + LOG.warn("Too many candidates for discovering overridden method in superclass for ${this.name}") + null + } + } +} From 8e7305f99f58286582a027eae4331ccaa65293dd Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 28 Feb 2024 17:28:02 +0100 Subject: [PATCH 02/31] Initial implementation --- .../verifiers/resolution/Methods.kt | 16 +++- .../OverrideOnlyMethodUsageProcessor.kt | 80 ++++++++++++++++++- 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt index ad875466c..64345674a 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt @@ -14,6 +14,8 @@ fun Method.isOverriding(anotherMethod: Method): Boolean = && sameParametersAndReturnType(this, anotherMethod) && sameOrBroaderVisibility(this, anotherMethod) +data class MethodInClass(val method: Method, val klass: ClassFile) + private fun nonStaticAndNonFinal(anotherMethod: Method) = !anotherMethod.isStatic && !anotherMethod.isFinal private fun sameName(method: Method, anotherMethod: Method) = method.name == anotherMethod.name @@ -34,14 +36,22 @@ private val Method.visibilityRating: Int else -> -1 } -fun Method.search(resolver: Resolver, matchHandler: (ClassFile, Method) -> Unit) { +fun Method.searchParentOverrides(resolver: Resolver): List { + return mutableListOf().apply { + searchParentOverrides(resolver) { klass: ClassFile, method: Method -> + add(MethodInClass(method, klass)) + } + } +} + +fun Method.searchParentOverrides(resolver: Resolver, matchHandler: (ClassFile, Method) -> Unit) { var superClassFqn = containingClassFile.superName ?: return while (true) { - val superClass = when(val resolvedClass = resolver.resolveClass(superClassFqn)) { + val superClass = when (val resolvedClass = resolver.resolveClass(superClassFqn)) { is ResolutionResult.Found -> ClassFileAsm(resolvedClass.value, resolvedClass.fileOrigin) else -> return } - findInSuperClass(superClass)?.let {overriddenMethod -> + findInSuperClass(superClass)?.let { overriddenMethod -> matchHandler(superClass, overriddenMethod) } superClassFqn = superClass.superName ?: return diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt index 4fe486b0d..d6878fb45 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt @@ -6,10 +6,20 @@ package com.jetbrains.pluginverifier.usages.overrideOnly import com.jetbrains.pluginverifier.results.reference.MethodReference import com.jetbrains.pluginverifier.usages.ApiUsageProcessor +import com.jetbrains.pluginverifier.usages.util.findEffectiveMemberAnnotation import com.jetbrains.pluginverifier.verifiers.VerificationContext import com.jetbrains.pluginverifier.verifiers.findAnnotation import com.jetbrains.pluginverifier.verifiers.resolution.Method +import com.jetbrains.pluginverifier.verifiers.resolution.searchParentOverrides +import org.objectweb.asm.Opcodes import org.objectweb.asm.tree.AbstractInsnNode +import org.objectweb.asm.tree.FieldInsnNode +import org.objectweb.asm.tree.MethodInsnNode +import org.objectweb.asm.tree.VarInsnNode +import org.slf4j.Logger +import org.slf4j.LoggerFactory + +private val LOG: Logger = LoggerFactory.getLogger(OverrideOnlyMethodUsageProcessor::class.java) class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: OverrideOnlyRegistrar) : ApiUsageProcessor { @@ -20,23 +30,87 @@ class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: Overri callerMethod: Method, context: VerificationContext ) { - if (resolvedMethod.isOverrideOnlyMethod() && !isCallOfSuperConstructor(callerMethod, resolvedMethod)) { + if (resolvedMethod.isOverrideOnlyMethod(context) && !isAllowedOverrideOnlyUsage(callerMethod, resolvedMethod, instructionNode)) { overrideOnlyRegistrar.registerOverrideOnlyMethodUsage( OverrideOnlyMethodUsage(methodReference, resolvedMethod.location, callerMethod.location) ) } } + private fun isAllowedOverrideOnlyUsage(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { + return isCallOfSuperConstructor(callerMethod, resolvedMethod) + || isAnActionUpdateSuperCall(callerMethod, resolvedMethod, instructionNode) + || isAnActionUpdateDelegateCall(callerMethod, resolvedMethod, instructionNode) + } + private fun isCallOfSuperConstructor(callerMethod: Method, resolvedMethod: Method) = resolvedMethod.isConstructor && callerMethod.isConstructor && callerMethod.containingClassFile.superName == resolvedMethod.containingClassFile.name - private fun Method.isOverrideOnlyMethod(): Boolean = + private fun isCallOfSuperMethod(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { + return callerMethod.name == resolvedMethod.name + && callerMethod.descriptor == resolvedMethod.descriptor + && instructionNode.opcode == Opcodes.INVOKESPECIAL + } + + private fun Method.isOverrideOnlyMethod(context: VerificationContext): Boolean = annotations.findAnnotation(overrideOnlyAnnotationName) != null || containingClassFile.annotations.findAnnotation(overrideOnlyAnnotationName) != null + || isAnnotationPresent(overrideOnlyAnnotationName, context) + + private fun Method.isAnnotationPresent(annotationFqn: String, verificationContext: VerificationContext): Boolean { + if (findEffectiveMemberAnnotation(annotationFqn, verificationContext.classResolver) != null) { + return true + } + + val overriddenMethod = searchParentOverrides(verificationContext.classResolver).firstOrNull { (overriddenMethod, c) -> + overriddenMethod.findEffectiveMemberAnnotation(annotationFqn, verificationContext.classResolver) != null + } + return if (overriddenMethod == null) { + LOG.trace("No overridden method for $name is annotated by [$annotationFqn]") + false + } else { + LOG.debug("Method '${overriddenMethod.method.name}' in [${overriddenMethod.klass.name}] is annotated by [$annotationFqn]") + true + } + } + private companion object { const val overrideOnlyAnnotationName = "org/jetbrains/annotations/ApiStatus\$OverrideOnly" } -} \ No newline at end of file + + private fun isAnActionUpdateSuperCall(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { + return (resolvedMethod.name == "update" + && resolvedMethod.descriptor == "(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V" + && isCallOfSuperMethod(callerMethod, resolvedMethod, instructionNode)) + } + + private fun isAnActionUpdateDelegateCall(callerMethod: Method, resolvedMethod: Method, instruction: AbstractInsnNode): Boolean { + val isCallingAnActionUpdateMethod = resolvedMethod.name == "update" + && resolvedMethod.descriptor == "(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V" + + // MethodIns + // VarIns + // FieldIns + var ins = instruction + val callAnActionUpdateMethod = ins.narrow() ?: return false + ins = ins.previous + val loadAnActionEventUpdateMethodParameter = ins.narrow() ?: return false + ins = ins.previous + val getAnActionDelegateField = ins.narrow() ?: return false + + // field 'delegate': Lcom/intellij/openapi/actionSystem/AnAction; + // field delegate must be a subclass of `AnAction` + // callAnActionUpdateMethod must be: name = 'update', desc = '(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V' + + return false + } + + private inline fun AbstractInsnNode.narrow(): T? { + return if (this is T) this else null + } +} + + From 4ea357a4745f352fc7c25301071ab26fb2e96cf4 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 28 Feb 2024 20:06:29 +0100 Subject: [PATCH 03/31] Implement detection of super calls and delegate calls of AnAction#update --- .../verifiers/filter/ApiUsageFilter.kt | 14 ++++ .../verifiers/resolution/Methods.kt | 8 ++ .../AnActionUpdateMethodAllowedUsageFilter.kt | 80 +++++++++++++++++++ .../OverrideOnlyMethodUsageProcessor.kt | 59 ++++---------- 4 files changed, 119 insertions(+), 42 deletions(-) create mode 100644 intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/ApiUsageFilter.kt create mode 100644 intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/ApiUsageFilter.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/ApiUsageFilter.kt new file mode 100644 index 000000000..8b553b537 --- /dev/null +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/ApiUsageFilter.kt @@ -0,0 +1,14 @@ +package com.jetbrains.pluginverifier.verifiers.filter + +import com.jetbrains.pluginverifier.results.reference.MethodReference +import com.jetbrains.pluginverifier.verifiers.VerificationContext +import com.jetbrains.pluginverifier.verifiers.resolution.Method +import org.objectweb.asm.tree.AbstractInsnNode + +interface ApiUsageFilter { + fun allowMethodInvocation(methodReference: MethodReference, + resolvedMethod: Method, + instructionNode: AbstractInsnNode, + callerMethod: Method, + context: VerificationContext): Boolean = true +} \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt index 64345674a..611b74502 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt @@ -2,6 +2,8 @@ package com.jetbrains.pluginverifier.verifiers.resolution import com.jetbrains.plugin.structure.classes.resolvers.ResolutionResult import com.jetbrains.plugin.structure.classes.resolvers.Resolver +import org.objectweb.asm.Opcodes +import org.objectweb.asm.tree.AbstractInsnNode import org.objectweb.asm.tree.ClassNode import org.slf4j.Logger import org.slf4j.LoggerFactory @@ -71,3 +73,9 @@ private fun Method.findInSuperClass(superClass: ClassFileAsm): Method? { } } } + +fun isCallOfSuperMethod(callerMethod: Method, calledMethod: Method, instructionNode: AbstractInsnNode): Boolean { + return callerMethod.name == calledMethod.name + && callerMethod.descriptor == calledMethod.descriptor + && instructionNode.opcode == Opcodes.INVOKESPECIAL +} \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt new file mode 100644 index 000000000..e32558391 --- /dev/null +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt @@ -0,0 +1,80 @@ +package com.jetbrains.pluginverifier.usages.overrideOnly + +import com.jetbrains.plugin.structure.classes.resolvers.ResolutionResult +import com.jetbrains.pluginverifier.results.reference.MethodReference +import com.jetbrains.pluginverifier.verifiers.VerificationContext +import com.jetbrains.pluginverifier.verifiers.extractClassNameFromDescriptor +import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter +import com.jetbrains.pluginverifier.verifiers.isSubclassOrSelf +import com.jetbrains.pluginverifier.verifiers.resolution.Method +import com.jetbrains.pluginverifier.verifiers.resolution.isCallOfSuperMethod +import org.objectweb.asm.tree.* + +class AnActionUpdateMethodAllowedUsageFilter : ApiUsageFilter { + override fun allowMethodInvocation(methodReference: MethodReference, + resolvedMethod: Method, + instructionNode: AbstractInsnNode, + callerMethod: Method, + context: VerificationContext): Boolean { + return isAnActionUpdateSuperCall(callerMethod, resolvedMethod, instructionNode) + || isAnActionUpdateDelegateCall(resolvedMethod, instructionNode, context) + } + + private fun isAnActionUpdateSuperCall(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { + return (resolvedMethod.isAnActionStyleUpdateMethod() + && isCallOfSuperMethod(callerMethod, resolvedMethod, instructionNode)) + } + + private fun isAnActionUpdateDelegateCall(resolvedMethod: Method, + instruction: AbstractInsnNode, + context: VerificationContext): Boolean { + val resolver = context.classResolver + val isCallingAnActionUpdateMethod = resolvedMethod.isAnActionStyleUpdateMethod() + if (!isCallingAnActionUpdateMethod) { + return false + } + + var ins = instruction + val callAnActionUpdateMethod = ins.narrow() ?: return false + ins = ins.previous + @Suppress("UNUSED_VARIABLE") + val loadAnActionEventUpdateMethodParameter = ins.narrow() ?: return false + ins = ins.previous + val getAnActionDelegateField = ins.narrow() ?: return false + + // field 'delegate' examples: + // - Lcom/intellij/openapi/actionSystem/AnAction; + // - Lcom/example/plugin/DelegateAction; + val delegateBinaryClassName = getAnActionDelegateField.desc.extractClassNameFromDescriptor() ?: return false + val delegateClassNode = when (val classResolution = resolver.resolveClass(delegateBinaryClassName)) { + is ResolutionResult.Found -> classResolution.value + else -> return false + } + // field delegate must be a subclass of `AnAction` + val isSubclassOfAnAction = resolver.isSubclassOrSelf(delegateClassNode.name, "com/intellij/openapi/actionSystem/AnAction") + if (!isSubclassOfAnAction) { + return false + } + // callAnActionUpdateMethod must be: name = 'update', desc = '(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V' + if (!callAnActionUpdateMethod.isAnActionStyleUpdateMethod()) { + return false + } + return true + } + + private inline fun AbstractInsnNode.narrow(): T? { + return if (this is T) this else null + } + + private fun Method.isAnActionStyleUpdateMethod(): Boolean { + return (name to descriptor).isAnActionStyleUpdateMethod() + } + + private fun MethodInsnNode.isAnActionStyleUpdateMethod(): Boolean { + return (name to desc).isAnActionStyleUpdateMethod() + } + + private fun Pair.isAnActionStyleUpdateMethod(): Boolean { + return ("update" to "(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V") == this + } +} \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt index d6878fb45..41633ad1c 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt @@ -11,11 +11,7 @@ import com.jetbrains.pluginverifier.verifiers.VerificationContext import com.jetbrains.pluginverifier.verifiers.findAnnotation import com.jetbrains.pluginverifier.verifiers.resolution.Method import com.jetbrains.pluginverifier.verifiers.resolution.searchParentOverrides -import org.objectweb.asm.Opcodes import org.objectweb.asm.tree.AbstractInsnNode -import org.objectweb.asm.tree.FieldInsnNode -import org.objectweb.asm.tree.MethodInsnNode -import org.objectweb.asm.tree.VarInsnNode import org.slf4j.Logger import org.slf4j.LoggerFactory @@ -23,6 +19,8 @@ private val LOG: Logger = LoggerFactory.getLogger(OverrideOnlyMethodUsageProcess class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: OverrideOnlyRegistrar) : ApiUsageProcessor { + private val anActionUpdateMethodAllowedFilter = AnActionUpdateMethodAllowedUsageFilter() + override fun processMethodInvocation( methodReference: MethodReference, resolvedMethod: Method, @@ -30,17 +28,24 @@ class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: Overri callerMethod: Method, context: VerificationContext ) { - if (resolvedMethod.isOverrideOnlyMethod(context) && !isAllowedOverrideOnlyUsage(callerMethod, resolvedMethod, instructionNode)) { + + + if (resolvedMethod.isOverrideOnlyMethod(context) + && !isAllowedOverrideOnlyUsage(methodReference, resolvedMethod, instructionNode, callerMethod, context)) + { overrideOnlyRegistrar.registerOverrideOnlyMethodUsage( OverrideOnlyMethodUsage(methodReference, resolvedMethod.location, callerMethod.location) ) } } - private fun isAllowedOverrideOnlyUsage(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { + private fun isAllowedOverrideOnlyUsage(methodReference: MethodReference, + resolvedMethod: Method, + instructionNode: AbstractInsnNode, + callerMethod: Method, + context: VerificationContext): Boolean { return isCallOfSuperConstructor(callerMethod, resolvedMethod) - || isAnActionUpdateSuperCall(callerMethod, resolvedMethod, instructionNode) - || isAnActionUpdateDelegateCall(callerMethod, resolvedMethod, instructionNode) + || anActionUpdateMethodAllowedFilter.allowMethodInvocation(methodReference, resolvedMethod, instructionNode, callerMethod, context) } private fun isCallOfSuperConstructor(callerMethod: Method, resolvedMethod: Method) = @@ -48,12 +53,6 @@ class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: Overri && callerMethod.isConstructor && callerMethod.containingClassFile.superName == resolvedMethod.containingClassFile.name - private fun isCallOfSuperMethod(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { - return callerMethod.name == resolvedMethod.name - && callerMethod.descriptor == resolvedMethod.descriptor - && instructionNode.opcode == Opcodes.INVOKESPECIAL - } - private fun Method.isOverrideOnlyMethod(context: VerificationContext): Boolean = annotations.findAnnotation(overrideOnlyAnnotationName) != null || containingClassFile.annotations.findAnnotation(overrideOnlyAnnotationName) != null @@ -81,36 +80,12 @@ class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: Overri const val overrideOnlyAnnotationName = "org/jetbrains/annotations/ApiStatus\$OverrideOnly" } - private fun isAnActionUpdateSuperCall(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { - return (resolvedMethod.name == "update" - && resolvedMethod.descriptor == "(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V" - && isCallOfSuperMethod(callerMethod, resolvedMethod, instructionNode)) - } - private fun isAnActionUpdateDelegateCall(callerMethod: Method, resolvedMethod: Method, instruction: AbstractInsnNode): Boolean { - val isCallingAnActionUpdateMethod = resolvedMethod.name == "update" - && resolvedMethod.descriptor == "(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V" - - // MethodIns - // VarIns - // FieldIns - var ins = instruction - val callAnActionUpdateMethod = ins.narrow() ?: return false - ins = ins.previous - val loadAnActionEventUpdateMethodParameter = ins.narrow() ?: return false - ins = ins.previous - val getAnActionDelegateField = ins.narrow() ?: return false - - // field 'delegate': Lcom/intellij/openapi/actionSystem/AnAction; - // field delegate must be a subclass of `AnAction` - // callAnActionUpdateMethod must be: name = 'update', desc = '(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V' - - return false - } - private inline fun AbstractInsnNode.narrow(): T? { - return if (this is T) this else null - } + + + + } From c45c15dace73cec973ba8f55860802e64a960f1d Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Thu, 29 Feb 2024 13:04:18 +0100 Subject: [PATCH 04/31] Add test-case scenarios for actions invoking update() of other actions --- .../openapi/actionSystem/AnAction.java | 5 ++++ .../openapi/actionSystem/AnAction.java | 5 ++++ .../plugin/OverrideFinalMethodProblem.java | 6 +++++ ...UpdatingAnotherActionExplicitlyAction.java | 24 +++++++++++++++++++ .../ActionUpdatingItselfAction.java | 24 +++++++++++++++++++ .../mock/plugin/overrideOnly/BaseAction.java | 17 +++++++++++++ .../plugin/overrideOnly/DelegatingAction.java | 19 +++++++++++++++ .../plugin/overrideOnly/SimpleAction.java | 11 +++++++++ .../src/main/resources/META-INF/plugin.xml | 5 ++++ 9 files changed, 116 insertions(+) create mode 100644 intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionUpdatingAnotherActionExplicitlyAction.java create mode 100644 intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionUpdatingItselfAction.java create mode 100644 intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/BaseAction.java create mode 100644 intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingAction.java create mode 100644 intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/SimpleAction.java diff --git a/intellij-plugin-verifier/verifier-test/after-idea/src/main/java/com/intellij/openapi/actionSystem/AnAction.java b/intellij-plugin-verifier/verifier-test/after-idea/src/main/java/com/intellij/openapi/actionSystem/AnAction.java index 82b99ae5a..5b82825d9 100644 --- a/intellij-plugin-verifier/verifier-test/after-idea/src/main/java/com/intellij/openapi/actionSystem/AnAction.java +++ b/intellij-plugin-verifier/verifier-test/after-idea/src/main/java/com/intellij/openapi/actionSystem/AnAction.java @@ -1,5 +1,8 @@ package com.intellij.openapi.actionSystem; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + public abstract class AnAction { public final boolean isEnabledInModalContext() { @@ -12,4 +15,6 @@ public boolean displayTextInToolbar() { protected abstract void actionPerformed(AnActionEvent e); + @ApiStatus.OverrideOnly + public abstract void update(@NotNull AnActionEvent e); } diff --git a/intellij-plugin-verifier/verifier-test/before-idea/src/main/java/com/intellij/openapi/actionSystem/AnAction.java b/intellij-plugin-verifier/verifier-test/before-idea/src/main/java/com/intellij/openapi/actionSystem/AnAction.java index f0e2366ab..5e810df29 100644 --- a/intellij-plugin-verifier/verifier-test/before-idea/src/main/java/com/intellij/openapi/actionSystem/AnAction.java +++ b/intellij-plugin-verifier/verifier-test/before-idea/src/main/java/com/intellij/openapi/actionSystem/AnAction.java @@ -1,5 +1,8 @@ package com.intellij.openapi.actionSystem; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + public abstract class AnAction { public static void nonExistingMethod() { @@ -16,4 +19,6 @@ public boolean displayTextInToolbar() { protected abstract void actionPerformed(AnActionEvent e); + @ApiStatus.OverrideOnly + public abstract void update(@NotNull AnActionEvent e); } diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/OverrideFinalMethodProblem.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/OverrideFinalMethodProblem.java index fea168f56..992f10744 100644 --- a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/OverrideFinalMethodProblem.java +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/OverrideFinalMethodProblem.java @@ -2,6 +2,7 @@ import com.intellij.openapi.actionSystem.AnAction; import com.intellij.openapi.actionSystem.AnActionEvent; +import org.jetbrains.annotations.NotNull; public class OverrideFinalMethodProblem extends AnAction { /*expected(PROBLEM) @@ -19,4 +20,9 @@ protected void actionPerformed(AnActionEvent e) { } + // just a plain override, no issues here + @Override + public void update(@NotNull AnActionEvent e) { + + } } diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionUpdatingAnotherActionExplicitlyAction.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionUpdatingAnotherActionExplicitlyAction.java new file mode 100644 index 000000000..fac9cb524 --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionUpdatingAnotherActionExplicitlyAction.java @@ -0,0 +1,24 @@ +package mock.plugin.overrideOnly; + +import com.intellij.openapi.actionSystem.AnAction; +import com.intellij.openapi.actionSystem.AnActionEvent; +import org.jetbrains.annotations.NotNull; + +public class ActionUpdatingAnotherActionExplicitlyAction extends AnAction { + + /*expected(OVERRIDE_ONLY) + Invocation of override-only method mock.plugin.overrideOnly.SimpleAction.update(AnActionEvent) + + Override-only method mock.plugin.overrideOnly.SimpleAction.update(AnActionEvent) is invoked in mock.plugin.overrideOnly.ActionUpdatingAnotherActionExplicitlyAction.actionPerformed(AnActionEvent) : void. This method is marked with @org.jetbrains.annotations.ApiStatus.OverrideOnly annotation, which indicates that the method must be only overridden but not invoked by client code. See documentation of the @ApiStatus.OverrideOnly for more info. + */ + @Override + protected void actionPerformed(AnActionEvent e) { + // this should be prohibited as per IDEA-336988 + new SimpleAction().update(e); + } + + @Override + public void update(@NotNull AnActionEvent e) { + + } +} diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionUpdatingItselfAction.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionUpdatingItselfAction.java new file mode 100644 index 000000000..22805d5a8 --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionUpdatingItselfAction.java @@ -0,0 +1,24 @@ +package mock.plugin.overrideOnly; + +import com.intellij.openapi.actionSystem.AnAction; +import com.intellij.openapi.actionSystem.AnActionEvent; +import org.jetbrains.annotations.NotNull; + +public class ActionUpdatingItselfAction extends AnAction { + /*expected(OVERRIDE_ONLY) + Invocation of override-only method mock.plugin.overrideOnly.ActionUpdatingItselfAction.update(AnActionEvent) + + Override-only method mock.plugin.overrideOnly.ActionUpdatingItselfAction.update(AnActionEvent) is invoked in mock.plugin.overrideOnly.ActionUpdatingItselfAction.update(AnActionEvent) : void. This method is marked with @org.jetbrains.annotations.ApiStatus.OverrideOnly annotation, which indicates that the method must be only overridden but not invoked by client code. See documentation of the @ApiStatus.OverrideOnly for more info. + */ + @Override + public void update(@NotNull AnActionEvent e) { + ActionUpdatingItselfAction action = new ActionUpdatingItselfAction(); + // this should be prohibited as per IDEA-336988 + action.update(e); + } + + @Override + protected void actionPerformed(AnActionEvent e) { + + } +} diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/BaseAction.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/BaseAction.java new file mode 100644 index 000000000..9cbe572c3 --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/BaseAction.java @@ -0,0 +1,17 @@ +package mock.plugin.overrideOnly; + +import com.intellij.openapi.actionSystem.AnAction; +import com.intellij.openapi.actionSystem.AnActionEvent; +import org.jetbrains.annotations.NotNull; + +public class BaseAction extends AnAction { + @Override + protected void actionPerformed(AnActionEvent e) { + // no-op + } + + @Override + public void update(@NotNull AnActionEvent e) { + // no-op + } +} diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingAction.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingAction.java new file mode 100644 index 000000000..df43d5c52 --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingAction.java @@ -0,0 +1,19 @@ +package mock.plugin.overrideOnly; + +import com.intellij.openapi.actionSystem.AnAction; +import com.intellij.openapi.actionSystem.AnActionEvent; +import org.jetbrains.annotations.NotNull; + +public class DelegatingAction extends AnAction { + private final AnAction delegateAction = new SimpleAction(); + + @Override + protected void actionPerformed(AnActionEvent e) { + // no-op + } + + @Override + public void update(@NotNull AnActionEvent e) { + delegateAction.update(e); + } +} diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/SimpleAction.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/SimpleAction.java new file mode 100644 index 000000000..d77bfb3e3 --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/SimpleAction.java @@ -0,0 +1,11 @@ +package mock.plugin.overrideOnly; + +import com.intellij.openapi.actionSystem.AnActionEvent; +import org.jetbrains.annotations.NotNull; + +public class SimpleAction extends BaseAction { + @Override + public void update(@NotNull AnActionEvent e) { + super.update(e); + } +} diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/resources/META-INF/plugin.xml b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/resources/META-INF/plugin.xml index d5fdf3d79..0f858c440 100644 --- a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/resources/META-INF/plugin.xml +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/resources/META-INF/plugin.xml @@ -10,6 +10,11 @@ + + + + + Date: Thu, 29 Feb 2024 13:18:41 +0100 Subject: [PATCH 05/31] Polish flow --- .../AnActionUpdateMethodAllowedUsageFilter.kt | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt index e32558391..bce2a47e8 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt @@ -10,6 +10,9 @@ import com.jetbrains.pluginverifier.verifiers.resolution.Method import com.jetbrains.pluginverifier.verifiers.resolution.isCallOfSuperMethod import org.objectweb.asm.tree.* + +typealias BinaryClassName = String + class AnActionUpdateMethodAllowedUsageFilter : ApiUsageFilter { override fun allowMethodInvocation(methodReference: MethodReference, resolvedMethod: Method, @@ -25,10 +28,10 @@ class AnActionUpdateMethodAllowedUsageFilter : ApiUsageFilter { && isCallOfSuperMethod(callerMethod, resolvedMethod, instructionNode)) } + @Suppress("UNUSED_VARIABLE") private fun isAnActionUpdateDelegateCall(resolvedMethod: Method, instruction: AbstractInsnNode, - context: VerificationContext): Boolean { - val resolver = context.classResolver + context: VerificationContext): Boolean = with(context.classResolver) { val isCallingAnActionUpdateMethod = resolvedMethod.isAnActionStyleUpdateMethod() if (!isCallingAnActionUpdateMethod) { return false @@ -37,21 +40,17 @@ class AnActionUpdateMethodAllowedUsageFilter : ApiUsageFilter { var ins = instruction val callAnActionUpdateMethod = ins.narrow() ?: return false ins = ins.previous - @Suppress("UNUSED_VARIABLE") val loadAnActionEventUpdateMethodParameter = ins.narrow() ?: return false ins = ins.previous val getAnActionDelegateField = ins.narrow() ?: return false - // field 'delegate' examples: - // - Lcom/intellij/openapi/actionSystem/AnAction; - // - Lcom/example/plugin/DelegateAction; - val delegateBinaryClassName = getAnActionDelegateField.desc.extractClassNameFromDescriptor() ?: return false - val delegateClassNode = when (val classResolution = resolver.resolveClass(delegateBinaryClassName)) { + val delegateBinaryClassName = getAnActionDelegateField.fieldClass ?: return false + val delegateClassNode = when (val classResolution = resolveClass(delegateBinaryClassName)) { is ResolutionResult.Found -> classResolution.value else -> return false } // field delegate must be a subclass of `AnAction` - val isSubclassOfAnAction = resolver.isSubclassOrSelf(delegateClassNode.name, "com/intellij/openapi/actionSystem/AnAction") + val isSubclassOfAnAction = isSubclassOrSelf(delegateClassNode.name, "com/intellij/openapi/actionSystem/AnAction") if (!isSubclassOfAnAction) { return false } @@ -66,6 +65,9 @@ class AnActionUpdateMethodAllowedUsageFilter : ApiUsageFilter { return if (this is T) this else null } + private val FieldInsnNode.fieldClass: BinaryClassName? + get() = desc.extractClassNameFromDescriptor() + private fun Method.isAnActionStyleUpdateMethod(): Boolean { return (name to descriptor).isAnActionStyleUpdateMethod() } From 4022a4eadd538e5dc02f94dc41e8eb417f556694 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Tue, 12 Mar 2024 12:48:41 +0100 Subject: [PATCH 06/31] Create a generalized allowed method invocations for override only --- .../OverrideOnlyMethodAllowedUsageFilter.kt | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt new file mode 100644 index 000000000..d8f78f658 --- /dev/null +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt @@ -0,0 +1,78 @@ +package com.jetbrains.pluginverifier.usages.overrideOnly + +import com.jetbrains.plugin.structure.classes.resolvers.ResolutionResult +import com.jetbrains.pluginverifier.results.reference.MethodReference +import com.jetbrains.pluginverifier.verifiers.VerificationContext +import com.jetbrains.pluginverifier.verifiers.extractClassNameFromDescriptor +import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter +import com.jetbrains.pluginverifier.verifiers.isSubclassOrSelf +import com.jetbrains.pluginverifier.verifiers.resolution.Method +import com.jetbrains.pluginverifier.verifiers.resolution.isCallOfSuperMethod +import org.objectweb.asm.tree.* + +class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: MethodDescriptor, + private val superclassName: BinaryClassName) : ApiUsageFilter { + + override fun allowMethodInvocation(methodReference: MethodReference, + resolvedMethod: Method, + instructionNode: AbstractInsnNode, + callerMethod: Method, + context: VerificationContext): Boolean { + return isSuperCall(callerMethod, resolvedMethod, instructionNode) + || isDelegateCall(resolvedMethod, instructionNode, context) + } + + private fun isSuperCall(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { + return (resolvedMethod.matches(allowedMethodDescriptor) + && isCallOfSuperMethod(callerMethod, resolvedMethod, instructionNode)) + } + + @Suppress("UNUSED_VARIABLE") + private fun isDelegateCall(resolvedMethod: Method, + instruction: AbstractInsnNode, + context: VerificationContext): Boolean = with(context.classResolver) { + val isCallingAllowedMethod = resolvedMethod.matches(allowedMethodDescriptor) + if (!isCallingAllowedMethod) { + return false + } + + var ins = instruction + val callMethod = ins.narrow() ?: return false + ins = ins.previous + val loadMethodParameter = ins.narrow() ?: return false + ins = ins.previous + val getDelegateField = ins.narrow() ?: return false + + val delegateBinaryClassName = getDelegateField.fieldClass ?: return false + val delegateClassNode = when (val classResolution = resolveClass(delegateBinaryClassName)) { + is ResolutionResult.Found -> classResolution.value + else -> return false + } + // field delegate must be a subclass of allowed class + val isSubclass = isSubclassOrSelf(delegateClassNode.name, superclassName) + if (!isSubclass) { + return false + } + if (!callMethod.matches(allowedMethodDescriptor)) { + return false + } + return true + } + + private inline fun AbstractInsnNode.narrow(): T? { + return if (this is T) this else null + } + + private val FieldInsnNode.fieldClass: BinaryClassName? + get() = desc.extractClassNameFromDescriptor() + +} + +data class MethodDescriptor(private val methodName: String, private val descriptor: String) + +fun Method.matches(methodDescriptor: MethodDescriptor): Boolean = + MethodDescriptor(name, descriptor) == methodDescriptor + +fun MethodInsnNode.matches(methodDescriptor: MethodDescriptor): Boolean = + MethodDescriptor(name, desc) == methodDescriptor + From dd6cb26c8c974985ba59a8e72a15da242f105ad5 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Tue, 12 Mar 2024 12:53:28 +0100 Subject: [PATCH 07/31] Delegate to generalized mechanism --- .../AnActionUpdateMethodAllowedUsageFilter.kt | 79 ++----------------- 1 file changed, 5 insertions(+), 74 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt index bce2a47e8..dd1fb0c96 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt @@ -1,82 +1,13 @@ package com.jetbrains.pluginverifier.usages.overrideOnly -import com.jetbrains.plugin.structure.classes.resolvers.ResolutionResult -import com.jetbrains.pluginverifier.results.reference.MethodReference -import com.jetbrains.pluginverifier.verifiers.VerificationContext -import com.jetbrains.pluginverifier.verifiers.extractClassNameFromDescriptor import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter -import com.jetbrains.pluginverifier.verifiers.isSubclassOrSelf -import com.jetbrains.pluginverifier.verifiers.resolution.Method -import com.jetbrains.pluginverifier.verifiers.resolution.isCallOfSuperMethod -import org.objectweb.asm.tree.* typealias BinaryClassName = String -class AnActionUpdateMethodAllowedUsageFilter : ApiUsageFilter { - override fun allowMethodInvocation(methodReference: MethodReference, - resolvedMethod: Method, - instructionNode: AbstractInsnNode, - callerMethod: Method, - context: VerificationContext): Boolean { - return isAnActionUpdateSuperCall(callerMethod, resolvedMethod, instructionNode) - || isAnActionUpdateDelegateCall(resolvedMethod, instructionNode, context) - } +val anActionUpdateMethodDescriptor = MethodDescriptor("update", "(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V") +const val anActionClass: BinaryClassName = "com/intellij/openapi/actionSystem/AnAction" - private fun isAnActionUpdateSuperCall(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { - return (resolvedMethod.isAnActionStyleUpdateMethod() - && isCallOfSuperMethod(callerMethod, resolvedMethod, instructionNode)) - } - - @Suppress("UNUSED_VARIABLE") - private fun isAnActionUpdateDelegateCall(resolvedMethod: Method, - instruction: AbstractInsnNode, - context: VerificationContext): Boolean = with(context.classResolver) { - val isCallingAnActionUpdateMethod = resolvedMethod.isAnActionStyleUpdateMethod() - if (!isCallingAnActionUpdateMethod) { - return false - } - - var ins = instruction - val callAnActionUpdateMethod = ins.narrow() ?: return false - ins = ins.previous - val loadAnActionEventUpdateMethodParameter = ins.narrow() ?: return false - ins = ins.previous - val getAnActionDelegateField = ins.narrow() ?: return false - - val delegateBinaryClassName = getAnActionDelegateField.fieldClass ?: return false - val delegateClassNode = when (val classResolution = resolveClass(delegateBinaryClassName)) { - is ResolutionResult.Found -> classResolution.value - else -> return false - } - // field delegate must be a subclass of `AnAction` - val isSubclassOfAnAction = isSubclassOrSelf(delegateClassNode.name, "com/intellij/openapi/actionSystem/AnAction") - if (!isSubclassOfAnAction) { - return false - } - // callAnActionUpdateMethod must be: name = 'update', desc = '(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V' - if (!callAnActionUpdateMethod.isAnActionStyleUpdateMethod()) { - return false - } - return true - } - - private inline fun AbstractInsnNode.narrow(): T? { - return if (this is T) this else null - } - - private val FieldInsnNode.fieldClass: BinaryClassName? - get() = desc.extractClassNameFromDescriptor() - - private fun Method.isAnActionStyleUpdateMethod(): Boolean { - return (name to descriptor).isAnActionStyleUpdateMethod() - } - - private fun MethodInsnNode.isAnActionStyleUpdateMethod(): Boolean { - return (name to desc).isAnActionStyleUpdateMethod() - } - - private fun Pair.isAnActionStyleUpdateMethod(): Boolean { - return ("update" to "(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V") == this - } -} \ No newline at end of file +class AnActionUpdateMethodAllowedUsageFilter( + private val delegate: ApiUsageFilter + = OverrideOnlyMethodAllowedUsageFilter(anActionUpdateMethodDescriptor, anActionClass)) : ApiUsageFilter by delegate \ No newline at end of file From 2253a8f90f801e5de38440a0a66cef93a025f11a Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Tue, 12 Mar 2024 13:22:28 +0100 Subject: [PATCH 08/31] Remove unnecessary whitespace --- .../overrideOnly/OverrideOnlyMethodUsageProcessor.kt | 8 -------- 1 file changed, 8 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt index 41633ad1c..dd52e89f8 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt @@ -75,17 +75,9 @@ class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: Overri } } - private companion object { const val overrideOnlyAnnotationName = "org/jetbrains/annotations/ApiStatus\$OverrideOnly" } - - - - - - - } From 59b8343bc378bb3dc3db7c6f6d56b75e3c9bc227 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Tue, 12 Mar 2024 15:07:30 +0100 Subject: [PATCH 09/31] Add KDoc --- .../OverrideOnlyMethodUsageProcessor.kt | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt index dd52e89f8..425211980 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt @@ -39,13 +39,36 @@ class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: Overri } } - private fun isAllowedOverrideOnlyUsage(methodReference: MethodReference, - resolvedMethod: Method, - instructionNode: AbstractInsnNode, + /** + * Detects if this is an allowed method invocation. + * In other words, if this is an allowed scenario where the invocation of such method is allowed and will not + * be reported as a plugin problem. + * + * Example: + * ``` + * public void usages(OverrideOnlyMethodOwner owner1,) { + * owner1.overrideOnlyMethod(); + * } + * ``` + * Description: + * - Both _Invoked Method Reference_ and _Invoked Method_ will refer to the `overrideOnlyMethod`. + * - _Invocation Instruction_ will refer to the JVM bytecode instruction which will invoke the `overrideOnlyMethod`. + * In this sample, this will be `182/invokevirtual` opcode. + * - _Caller Method_ refers to the `usages()` method + * @param invokedMethodReference a reference to the `OverrideOnly` method that is being invoked + * @param invokedMethod the `OverrideOnly` method that is being invoked + * @param invocationInstruction low-level JVM invocation instruction used to invoke the `OverrideOnly` method + * @param callerMethod the method in which the invocation of `OverrideOnly` method occurs. + * @param context the verification context with additional metadata and data + * + */ + private fun isAllowedOverrideOnlyUsage(invokedMethodReference: MethodReference, + invokedMethod: Method, + invocationInstruction: AbstractInsnNode, callerMethod: Method, context: VerificationContext): Boolean { - return isCallOfSuperConstructor(callerMethod, resolvedMethod) - || anActionUpdateMethodAllowedFilter.allowMethodInvocation(methodReference, resolvedMethod, instructionNode, callerMethod, context) + return isCallOfSuperConstructor(callerMethod, invokedMethod) + || anActionUpdateMethodAllowedFilter.allowMethodInvocation(invokedMethodReference, invokedMethod, invocationInstruction, callerMethod, context) } private fun isCallOfSuperConstructor(callerMethod: Method, resolvedMethod: Method) = From cff7dbbc38f313431d382229be938c5f161b4c64 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Tue, 12 Mar 2024 15:52:12 +0100 Subject: [PATCH 10/31] Reduce number of duplicate parameters --- .../verifiers/filter/ApiUsageFilter.kt | 6 ++---- .../OverrideOnlyMethodAllowedUsageFilter.kt | 18 ++++++++---------- .../OverrideOnlyMethodUsageProcessor.kt | 2 +- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/ApiUsageFilter.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/ApiUsageFilter.kt index 8b553b537..c6db60c31 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/ApiUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/ApiUsageFilter.kt @@ -1,14 +1,12 @@ package com.jetbrains.pluginverifier.verifiers.filter -import com.jetbrains.pluginverifier.results.reference.MethodReference import com.jetbrains.pluginverifier.verifiers.VerificationContext import com.jetbrains.pluginverifier.verifiers.resolution.Method import org.objectweb.asm.tree.AbstractInsnNode interface ApiUsageFilter { - fun allowMethodInvocation(methodReference: MethodReference, - resolvedMethod: Method, - instructionNode: AbstractInsnNode, + fun allowMethodInvocation(invokedMethod: Method, + invocationInstruction: AbstractInsnNode, callerMethod: Method, context: VerificationContext): Boolean = true } \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt index d8f78f658..cbd99d7b2 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt @@ -1,7 +1,6 @@ package com.jetbrains.pluginverifier.usages.overrideOnly import com.jetbrains.plugin.structure.classes.resolvers.ResolutionResult -import com.jetbrains.pluginverifier.results.reference.MethodReference import com.jetbrains.pluginverifier.verifiers.VerificationContext import com.jetbrains.pluginverifier.verifiers.extractClassNameFromDescriptor import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter @@ -13,13 +12,12 @@ import org.objectweb.asm.tree.* class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: MethodDescriptor, private val superclassName: BinaryClassName) : ApiUsageFilter { - override fun allowMethodInvocation(methodReference: MethodReference, - resolvedMethod: Method, - instructionNode: AbstractInsnNode, + override fun allowMethodInvocation(invokedMethod: Method, + invocationInstruction: AbstractInsnNode, callerMethod: Method, context: VerificationContext): Boolean { - return isSuperCall(callerMethod, resolvedMethod, instructionNode) - || isDelegateCall(resolvedMethod, instructionNode, context) + return isSuperCall(callerMethod, invokedMethod, invocationInstruction) + || isDelegateCall(invokedMethod, invocationInstruction, context) } private fun isSuperCall(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { @@ -28,15 +26,15 @@ class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: } @Suppress("UNUSED_VARIABLE") - private fun isDelegateCall(resolvedMethod: Method, - instruction: AbstractInsnNode, + private fun isDelegateCall(invokedMethod: Method, + invocationInstruction: AbstractInsnNode, context: VerificationContext): Boolean = with(context.classResolver) { - val isCallingAllowedMethod = resolvedMethod.matches(allowedMethodDescriptor) + val isCallingAllowedMethod = invokedMethod.matches(allowedMethodDescriptor) if (!isCallingAllowedMethod) { return false } - var ins = instruction + var ins = invocationInstruction val callMethod = ins.narrow() ?: return false ins = ins.previous val loadMethodParameter = ins.narrow() ?: return false diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt index 425211980..599622768 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt @@ -68,7 +68,7 @@ class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: Overri callerMethod: Method, context: VerificationContext): Boolean { return isCallOfSuperConstructor(callerMethod, invokedMethod) - || anActionUpdateMethodAllowedFilter.allowMethodInvocation(invokedMethodReference, invokedMethod, invocationInstruction, callerMethod, context) + || anActionUpdateMethodAllowedFilter.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) } private fun isCallOfSuperConstructor(callerMethod: Method, resolvedMethod: Method) = From 148e3130571418482f0a7a6a30850213671cff1b Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Tue, 12 Mar 2024 15:59:15 +0100 Subject: [PATCH 11/31] Add calling method to parameters --- .../overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt index cbd99d7b2..ecadb0223 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt @@ -17,7 +17,7 @@ class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: callerMethod: Method, context: VerificationContext): Boolean { return isSuperCall(callerMethod, invokedMethod, invocationInstruction) - || isDelegateCall(invokedMethod, invocationInstruction, context) + || isDelegateCall(callerMethod, invokedMethod, invocationInstruction, context) } private fun isSuperCall(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { @@ -25,8 +25,9 @@ class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: && isCallOfSuperMethod(callerMethod, resolvedMethod, instructionNode)) } - @Suppress("UNUSED_VARIABLE") - private fun isDelegateCall(invokedMethod: Method, + @Suppress("UNUSED_VARIABLE", "UNUSED_PARAMETER") + private fun isDelegateCall(callerMethod: Method, + invokedMethod: Method, invocationInstruction: AbstractInsnNode, context: VerificationContext): Boolean = with(context.classResolver) { val isCallingAllowedMethod = invokedMethod.matches(allowedMethodDescriptor) From 11be503b659050381fbb5995636c791563a4fcfd Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Tue, 12 Mar 2024 16:21:14 +0100 Subject: [PATCH 12/31] Use invocation predicate class to filter out necessary calls --- .../OverrideOnlyMethodAllowedUsageFilter.kt | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt index ecadb0223..f2990976a 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt @@ -16,8 +16,20 @@ class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: invocationInstruction: AbstractInsnNode, callerMethod: Method, context: VerificationContext): Boolean { - return isSuperCall(callerMethod, invokedMethod, invocationInstruction) - || isDelegateCall(callerMethod, invokedMethod, invocationInstruction, context) + + val superCall = isSuperCall(callerMethod, invokedMethod, invocationInstruction) + + val invocationPredicate = object : InvocationPredicate { + override fun accept(caller: Method, callee: Method): Boolean { + return callee.matches(allowedMethodDescriptor) + } + + override fun accept(caller: Method, callee: MethodInsnNode): Boolean { + return callee.matches(allowedMethodDescriptor) + } + } + val delegateCall = isDelegateCall(callerMethod, invokedMethod, invocationInstruction, context, invocationPredicate) + return superCall || delegateCall } private fun isSuperCall(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { @@ -29,8 +41,10 @@ class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: private fun isDelegateCall(callerMethod: Method, invokedMethod: Method, invocationInstruction: AbstractInsnNode, - context: VerificationContext): Boolean = with(context.classResolver) { - val isCallingAllowedMethod = invokedMethod.matches(allowedMethodDescriptor) + context: VerificationContext, + invocationPredicate: InvocationPredicate + ): Boolean = with(context.classResolver) { + val isCallingAllowedMethod = invocationPredicate.accept(callerMethod, invokedMethod) if (!isCallingAllowedMethod) { return false } @@ -52,7 +66,7 @@ class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: if (!isSubclass) { return false } - if (!callMethod.matches(allowedMethodDescriptor)) { + if (!invocationPredicate.accept(callerMethod, callMethod)) { return false } return true @@ -75,3 +89,8 @@ fun Method.matches(methodDescriptor: MethodDescriptor): Boolean = fun MethodInsnNode.matches(methodDescriptor: MethodDescriptor): Boolean = MethodDescriptor(name, desc) == methodDescriptor +interface InvocationPredicate { + fun accept(caller: Method, callee: Method): Boolean = true + fun accept(caller: Method, callee: MethodInsnNode): Boolean = true +} + From 25134074e982d2c46a671a280892a62350bf1900 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Tue, 12 Mar 2024 16:44:37 +0100 Subject: [PATCH 13/31] Extract to separate delegate checker class --- .../AnActionUpdateMethodAllowedUsageFilter.kt | 2 +- .../DelegateCallOnOverrideOnlyUsageFilter.kt | 62 ++++++++++++++++ .../OverrideOnlyMethodAllowedUsageFilter.kt | 73 ++----------------- 3 files changed, 68 insertions(+), 69 deletions(-) create mode 100644 intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt index dd1fb0c96..241f44daf 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt @@ -10,4 +10,4 @@ const val anActionClass: BinaryClassName = "com/intellij/openapi/actionSystem/An class AnActionUpdateMethodAllowedUsageFilter( private val delegate: ApiUsageFilter - = OverrideOnlyMethodAllowedUsageFilter(anActionUpdateMethodDescriptor, anActionClass)) : ApiUsageFilter by delegate \ No newline at end of file + = OverrideOnlyMethodAllowedUsageFilter(anActionUpdateMethodDescriptor)) : ApiUsageFilter by delegate \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt new file mode 100644 index 000000000..359f6c527 --- /dev/null +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt @@ -0,0 +1,62 @@ +package com.jetbrains.pluginverifier.usages.overrideOnly + +import com.jetbrains.plugin.structure.classes.resolvers.ResolutionResult +import com.jetbrains.pluginverifier.verifiers.VerificationContext +import com.jetbrains.pluginverifier.verifiers.extractClassNameFromDescriptor +import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter +import com.jetbrains.pluginverifier.verifiers.isSubclassOrSelf +import com.jetbrains.pluginverifier.verifiers.resolution.Method +import org.objectweb.asm.tree.* + +class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter { + //FIXME novotnyr remove when autodetect is enabled + private val superclassName: String = anActionClass + + override fun allowMethodInvocation(invokedMethod: Method, + invocationInstruction: AbstractInsnNode, + callerMethod: Method, + context: VerificationContext): Boolean = with(context.classResolver) { + val isCallingAllowedMethod = isInvokedMethodAllowed(callerMethod, invokedMethod) + if (!isCallingAllowedMethod) { + return false + } + + var ins = invocationInstruction + val callMethod = ins.narrow() ?: return false + ins = ins.previous + val loadMethodParameter = ins.narrow() ?: return false + ins = ins.previous + val getDelegateField = ins.narrow() ?: return false + + val delegateBinaryClassName = getDelegateField.fieldClass ?: return false + val delegateClassNode = when (val classResolution = resolveClass(delegateBinaryClassName)) { + is ResolutionResult.Found -> classResolution.value + else -> return false + } + // field delegate must be a subclass of allowed class + val isSubclass = isSubclassOrSelf(delegateClassNode.name, superclassName) + if (!isSubclass) { + return false + } + if (!isDelegateInvocationAllowed(callerMethod, callMethod)) { + return false + } + return true + } + + private fun isDelegateInvocationAllowed(callerMethod: Method, delegateMethodInvocationInstruction: MethodInsnNode): Boolean { + return callerMethod.name == delegateMethodInvocationInstruction.name && + callerMethod.descriptor == delegateMethodInvocationInstruction.desc + } + + private fun isInvokedMethodAllowed(callerMethod: Method, invokedMethod: Method): Boolean { + return callerMethod.name == invokedMethod.name && callerMethod.descriptor == invokedMethod.descriptor + } + + private inline fun AbstractInsnNode.narrow(): T? { + return if (this is T) this else null + } + + private val FieldInsnNode.fieldClass: BinaryClassName? + get() = desc.extractClassNameFromDescriptor() +} \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt index f2990976a..3b6f81d89 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt @@ -1,16 +1,14 @@ package com.jetbrains.pluginverifier.usages.overrideOnly -import com.jetbrains.plugin.structure.classes.resolvers.ResolutionResult import com.jetbrains.pluginverifier.verifiers.VerificationContext -import com.jetbrains.pluginverifier.verifiers.extractClassNameFromDescriptor import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter -import com.jetbrains.pluginverifier.verifiers.isSubclassOrSelf import com.jetbrains.pluginverifier.verifiers.resolution.Method import com.jetbrains.pluginverifier.verifiers.resolution.isCallOfSuperMethod -import org.objectweb.asm.tree.* +import org.objectweb.asm.tree.AbstractInsnNode -class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: MethodDescriptor, - private val superclassName: BinaryClassName) : ApiUsageFilter { +class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: MethodDescriptor) : ApiUsageFilter { + + private val isDelegateCall = DelegateCallOnOverrideOnlyUsageFilter() override fun allowMethodInvocation(invokedMethod: Method, invocationInstruction: AbstractInsnNode, @@ -18,18 +16,7 @@ class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: context: VerificationContext): Boolean { val superCall = isSuperCall(callerMethod, invokedMethod, invocationInstruction) - - val invocationPredicate = object : InvocationPredicate { - override fun accept(caller: Method, callee: Method): Boolean { - return callee.matches(allowedMethodDescriptor) - } - - override fun accept(caller: Method, callee: MethodInsnNode): Boolean { - return callee.matches(allowedMethodDescriptor) - } - } - val delegateCall = isDelegateCall(callerMethod, invokedMethod, invocationInstruction, context, invocationPredicate) - return superCall || delegateCall + return superCall || isDelegateCall.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) } private fun isSuperCall(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { @@ -37,48 +24,6 @@ class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: && isCallOfSuperMethod(callerMethod, resolvedMethod, instructionNode)) } - @Suppress("UNUSED_VARIABLE", "UNUSED_PARAMETER") - private fun isDelegateCall(callerMethod: Method, - invokedMethod: Method, - invocationInstruction: AbstractInsnNode, - context: VerificationContext, - invocationPredicate: InvocationPredicate - ): Boolean = with(context.classResolver) { - val isCallingAllowedMethod = invocationPredicate.accept(callerMethod, invokedMethod) - if (!isCallingAllowedMethod) { - return false - } - - var ins = invocationInstruction - val callMethod = ins.narrow() ?: return false - ins = ins.previous - val loadMethodParameter = ins.narrow() ?: return false - ins = ins.previous - val getDelegateField = ins.narrow() ?: return false - - val delegateBinaryClassName = getDelegateField.fieldClass ?: return false - val delegateClassNode = when (val classResolution = resolveClass(delegateBinaryClassName)) { - is ResolutionResult.Found -> classResolution.value - else -> return false - } - // field delegate must be a subclass of allowed class - val isSubclass = isSubclassOrSelf(delegateClassNode.name, superclassName) - if (!isSubclass) { - return false - } - if (!invocationPredicate.accept(callerMethod, callMethod)) { - return false - } - return true - } - - private inline fun AbstractInsnNode.narrow(): T? { - return if (this is T) this else null - } - - private val FieldInsnNode.fieldClass: BinaryClassName? - get() = desc.extractClassNameFromDescriptor() - } data class MethodDescriptor(private val methodName: String, private val descriptor: String) @@ -86,11 +31,3 @@ data class MethodDescriptor(private val methodName: String, private val descript fun Method.matches(methodDescriptor: MethodDescriptor): Boolean = MethodDescriptor(name, descriptor) == methodDescriptor -fun MethodInsnNode.matches(methodDescriptor: MethodDescriptor): Boolean = - MethodDescriptor(name, desc) == methodDescriptor - -interface InvocationPredicate { - fun accept(caller: Method, callee: Method): Boolean = true - fun accept(caller: Method, callee: MethodInsnNode): Boolean = true -} - From 89a7eb461db4f05dc2a9590844befdc79da4e153 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Tue, 12 Mar 2024 17:03:27 +0100 Subject: [PATCH 14/31] Refactor to a separate supercall class --- .../AnActionUpdateMethodAllowedUsageFilter.kt | 2 +- .../DelegateCallOnOverrideOnlyUsageFilter.kt | 2 +- .../OverrideOnlyMethodAllowedUsageFilter.kt | 17 +++++++---------- .../SuperclassCallOnOverrideOnlyUsageFilter.kt | 14 ++++++++++++++ 4 files changed, 23 insertions(+), 12 deletions(-) create mode 100644 intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/SuperclassCallOnOverrideOnlyUsageFilter.kt diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt index 241f44daf..a49e7deb0 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt @@ -10,4 +10,4 @@ const val anActionClass: BinaryClassName = "com/intellij/openapi/actionSystem/An class AnActionUpdateMethodAllowedUsageFilter( private val delegate: ApiUsageFilter - = OverrideOnlyMethodAllowedUsageFilter(anActionUpdateMethodDescriptor)) : ApiUsageFilter by delegate \ No newline at end of file + = OverrideOnlyMethodAllowedUsageFilter()) : ApiUsageFilter by delegate \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt index 359f6c527..d94a481d3 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt @@ -50,7 +50,7 @@ class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter { } private fun isInvokedMethodAllowed(callerMethod: Method, invokedMethod: Method): Boolean { - return callerMethod.name == invokedMethod.name && callerMethod.descriptor == invokedMethod.descriptor + return callerMethod.matches(invokedMethod) } private inline fun AbstractInsnNode.narrow(): T? { diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt index 3b6f81d89..f3e623ecb 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt @@ -3,27 +3,22 @@ package com.jetbrains.pluginverifier.usages.overrideOnly import com.jetbrains.pluginverifier.verifiers.VerificationContext import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter import com.jetbrains.pluginverifier.verifiers.resolution.Method -import com.jetbrains.pluginverifier.verifiers.resolution.isCallOfSuperMethod import org.objectweb.asm.tree.AbstractInsnNode -class OverrideOnlyMethodAllowedUsageFilter(private val allowedMethodDescriptor: MethodDescriptor) : ApiUsageFilter { +class OverrideOnlyMethodAllowedUsageFilter : ApiUsageFilter { private val isDelegateCall = DelegateCallOnOverrideOnlyUsageFilter() + private val isSuperclassCall = SuperclassCallOnOverrideOnlyUsageFilter() override fun allowMethodInvocation(invokedMethod: Method, invocationInstruction: AbstractInsnNode, callerMethod: Method, context: VerificationContext): Boolean { - val superCall = isSuperCall(callerMethod, invokedMethod, invocationInstruction) - return superCall || isDelegateCall.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) + val isSupercall = isSuperclassCall.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) + val isDelegateCall = isDelegateCall.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) + return isSupercall || isDelegateCall } - - private fun isSuperCall(callerMethod: Method, resolvedMethod: Method, instructionNode: AbstractInsnNode): Boolean { - return (resolvedMethod.matches(allowedMethodDescriptor) - && isCallOfSuperMethod(callerMethod, resolvedMethod, instructionNode)) - } - } data class MethodDescriptor(private val methodName: String, private val descriptor: String) @@ -31,3 +26,5 @@ data class MethodDescriptor(private val methodName: String, private val descript fun Method.matches(methodDescriptor: MethodDescriptor): Boolean = MethodDescriptor(name, descriptor) == methodDescriptor +fun Method.matches(method: Method): Boolean = + MethodDescriptor(name, descriptor) == MethodDescriptor(method.name, method.descriptor) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/SuperclassCallOnOverrideOnlyUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/SuperclassCallOnOverrideOnlyUsageFilter.kt new file mode 100644 index 000000000..f75efe00a --- /dev/null +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/SuperclassCallOnOverrideOnlyUsageFilter.kt @@ -0,0 +1,14 @@ +package com.jetbrains.pluginverifier.usages.overrideOnly + +import com.jetbrains.pluginverifier.verifiers.VerificationContext +import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter +import com.jetbrains.pluginverifier.verifiers.resolution.Method +import com.jetbrains.pluginverifier.verifiers.resolution.isCallOfSuperMethod +import org.objectweb.asm.tree.AbstractInsnNode + +class SuperclassCallOnOverrideOnlyUsageFilter : ApiUsageFilter { + override fun allowMethodInvocation(invokedMethod: Method, invocationInstruction: AbstractInsnNode, callerMethod: Method, context: VerificationContext): Boolean { + return callerMethod.matches(invokedMethod) + && isCallOfSuperMethod(callerMethod, invokedMethod, invocationInstruction) + } +} \ No newline at end of file From ddca392c685e6663e25ea0bc3f8e35191f4af59d Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 11:52:45 +0100 Subject: [PATCH 15/31] Add KDoc --- .../jetbrains/pluginverifier/verifiers/resolution/Methods.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt index 611b74502..0a860c91d 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt @@ -38,6 +38,10 @@ private val Method.visibilityRating: Int else -> -1 } +/** + * Search for all methods in the parent hierarchy that the receiver overrides. + * @return list of methods, sorted from bottom-to-top in the inheritance hierarchy. + */ fun Method.searchParentOverrides(resolver: Resolver): List { return mutableListOf().apply { searchParentOverrides(resolver) { klass: ClassFile, method: Method -> From 495cb22bcbdcebfd56d39d66ebd5678128f72157 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 12:07:43 +0100 Subject: [PATCH 16/31] Automatically discover allowed types for delegates --- .../DelegateCallOnOverrideOnlyUsageFilter.kt | 25 ++++++++++++++++--- .../plugin/overrideOnly/DelegatingAction.java | 3 ++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt index d94a481d3..601dc9dcf 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt @@ -1,17 +1,17 @@ package com.jetbrains.pluginverifier.usages.overrideOnly import com.jetbrains.plugin.structure.classes.resolvers.ResolutionResult +import com.jetbrains.plugin.structure.classes.resolvers.Resolver import com.jetbrains.pluginverifier.verifiers.VerificationContext import com.jetbrains.pluginverifier.verifiers.extractClassNameFromDescriptor import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter import com.jetbrains.pluginverifier.verifiers.isSubclassOrSelf import com.jetbrains.pluginverifier.verifiers.resolution.Method +import com.jetbrains.pluginverifier.verifiers.resolution.searchParentOverrides import org.objectweb.asm.tree.* class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter { - //FIXME novotnyr remove when autodetect is enabled - private val superclassName: String = anActionClass - + @Suppress("UNUSED_VARIABLE") override fun allowMethodInvocation(invokedMethod: Method, invocationInstruction: AbstractInsnNode, callerMethod: Method, @@ -33,8 +33,13 @@ class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter { is ResolutionResult.Found -> classResolution.value else -> return false } + + // Search for top-most class that holds a method in which the invocation occurs. + // Such class is a top-most type of the delegate field. + val callerMethodTopMostSuperClass = callerMethod.getSuperClassName(resolver = this) + // field delegate must be a subclass of allowed class - val isSubclass = isSubclassOrSelf(delegateClassNode.name, superclassName) + val isSubclass = isSubclassOrSelf(delegateClassNode.name, callerMethodTopMostSuperClass) if (!isSubclass) { return false } @@ -44,6 +49,18 @@ class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter { return true } + /** + * Search for top-most class that declares a method specified by the receiver. + * The receiver is by definition overriding the discovered method. + * + * @return the class name of the discovered class. + */ + // + private fun Method.getSuperClassName(resolver: Resolver): BinaryClassName { + val topMostClass = searchParentOverrides(resolver).lastOrNull() ?: return containingClassFile.name + return topMostClass.klass.name + } + private fun isDelegateInvocationAllowed(callerMethod: Method, delegateMethodInvocationInstruction: MethodInsnNode): Boolean { return callerMethod.name == delegateMethodInvocationInstruction.name && callerMethod.descriptor == delegateMethodInvocationInstruction.desc diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingAction.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingAction.java index df43d5c52..85fa03939 100644 --- a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingAction.java +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingAction.java @@ -4,9 +4,10 @@ import com.intellij.openapi.actionSystem.AnActionEvent; import org.jetbrains.annotations.NotNull; -public class DelegatingAction extends AnAction { +public class DelegatingAction extends BaseAction { private final AnAction delegateAction = new SimpleAction(); + @SuppressWarnings("RedundantMethodOverride") @Override protected void actionPerformed(AnActionEvent e) { // no-op From fbe7b5736902148fc1f6f0a76cbb18a524d2883e Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 15:25:34 +0100 Subject: [PATCH 17/31] Cover ActionGroup calls with OverrideOnly in unit tests --- .../openapi/actionSystem/ActionGroup.java | 13 ++++++++ .../openapi/actionSystem/ActionGroup.java | 13 ++++++++ ...GroupUpdatingAnotherActionActionGroup.java | 30 +++++++++++++++++++ .../ActionGroupUpdatingItselfActionGroup.java | 30 +++++++++++++++++++ .../overrideOnly/DelegatingActionGroup.java | 26 ++++++++++++++++ .../plugin/overrideOnly/EmptyActionGroup.java | 24 +++++++++++++++ 6 files changed, 136 insertions(+) create mode 100644 intellij-plugin-verifier/verifier-test/after-idea/src/main/java/com/intellij/openapi/actionSystem/ActionGroup.java create mode 100644 intellij-plugin-verifier/verifier-test/before-idea/src/main/java/com/intellij/openapi/actionSystem/ActionGroup.java create mode 100644 intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionGroupUpdatingAnotherActionActionGroup.java create mode 100644 intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionGroupUpdatingItselfActionGroup.java create mode 100644 intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingActionGroup.java create mode 100644 intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/EmptyActionGroup.java diff --git a/intellij-plugin-verifier/verifier-test/after-idea/src/main/java/com/intellij/openapi/actionSystem/ActionGroup.java b/intellij-plugin-verifier/verifier-test/after-idea/src/main/java/com/intellij/openapi/actionSystem/ActionGroup.java new file mode 100644 index 000000000..7207ea452 --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/after-idea/src/main/java/com/intellij/openapi/actionSystem/ActionGroup.java @@ -0,0 +1,13 @@ +package com.intellij.openapi.actionSystem; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public abstract class ActionGroup extends AnAction { + // Method has an explicit `OverrideOnly` annotation to be aligned with IDEA-336988 + @ApiStatus.OverrideOnly + public abstract AnAction @NotNull [] getChildren(@Nullable AnActionEvent e); +} + + diff --git a/intellij-plugin-verifier/verifier-test/before-idea/src/main/java/com/intellij/openapi/actionSystem/ActionGroup.java b/intellij-plugin-verifier/verifier-test/before-idea/src/main/java/com/intellij/openapi/actionSystem/ActionGroup.java new file mode 100644 index 000000000..7207ea452 --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/before-idea/src/main/java/com/intellij/openapi/actionSystem/ActionGroup.java @@ -0,0 +1,13 @@ +package com.intellij.openapi.actionSystem; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public abstract class ActionGroup extends AnAction { + // Method has an explicit `OverrideOnly` annotation to be aligned with IDEA-336988 + @ApiStatus.OverrideOnly + public abstract AnAction @NotNull [] getChildren(@Nullable AnActionEvent e); +} + + diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionGroupUpdatingAnotherActionActionGroup.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionGroupUpdatingAnotherActionActionGroup.java new file mode 100644 index 000000000..86066f588 --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionGroupUpdatingAnotherActionActionGroup.java @@ -0,0 +1,30 @@ +package mock.plugin.overrideOnly; + +import com.intellij.openapi.actionSystem.ActionGroup; +import com.intellij.openapi.actionSystem.AnAction; +import com.intellij.openapi.actionSystem.AnActionEvent; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public class ActionGroupUpdatingAnotherActionActionGroup extends ActionGroup { + /*expected(OVERRIDE_ONLY) + Invocation of override-only method mock.plugin.overrideOnly.SimpleAction.update(AnActionEvent) + + Override-only method mock.plugin.overrideOnly.SimpleAction.update(AnActionEvent) is invoked in mock.plugin.overrideOnly.ActionGroupUpdatingAnotherActionActionGroup.actionPerformed(AnActionEvent) : void. This method is marked with @org.jetbrains.annotations.ApiStatus.OverrideOnly annotation, which indicates that the method must be only overridden but not invoked by client code. See documentation of the @ApiStatus.OverrideOnly for more info. + */ + @Override + public AnAction @NotNull [] getChildren(@Nullable AnActionEvent e) { + return new AnAction[0]; + } + + @Override + protected void actionPerformed(AnActionEvent e) { + // this should be prohibited as per IDEA-336988 + new SimpleAction().update(e); + } + + @Override + public void update(@NotNull AnActionEvent e) { + // no-op + } +} diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionGroupUpdatingItselfActionGroup.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionGroupUpdatingItselfActionGroup.java new file mode 100644 index 000000000..a7f63ee2c --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ActionGroupUpdatingItselfActionGroup.java @@ -0,0 +1,30 @@ +package mock.plugin.overrideOnly; + +import com.intellij.openapi.actionSystem.ActionGroup; +import com.intellij.openapi.actionSystem.AnAction; +import com.intellij.openapi.actionSystem.AnActionEvent; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public class ActionGroupUpdatingItselfActionGroup extends ActionGroup { + /*expected(OVERRIDE_ONLY) + Invocation of override-only method mock.plugin.overrideOnly.EmptyActionGroup.update(AnActionEvent) + + Override-only method mock.plugin.overrideOnly.EmptyActionGroup.update(AnActionEvent) is invoked in mock.plugin.overrideOnly.ActionGroupUpdatingItselfActionGroup.actionPerformed(AnActionEvent) : void. This method is marked with @org.jetbrains.annotations.ApiStatus.OverrideOnly annotation, which indicates that the method must be only overridden but not invoked by client code. See documentation of the @ApiStatus.OverrideOnly for more info. + */ + @Override + public AnAction @NotNull [] getChildren(@Nullable AnActionEvent e) { + return new AnAction[0]; + } + + @Override + protected void actionPerformed(AnActionEvent e) { + // this should be prohibited as per IDEA-336988 + new EmptyActionGroup().update(e); + } + + @Override + public void update(@NotNull AnActionEvent e) { + // no-op + } +} diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingActionGroup.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingActionGroup.java new file mode 100644 index 000000000..cf415bb9f --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/DelegatingActionGroup.java @@ -0,0 +1,26 @@ +package mock.plugin.overrideOnly; + +import com.intellij.openapi.actionSystem.ActionGroup; +import com.intellij.openapi.actionSystem.AnAction; +import com.intellij.openapi.actionSystem.AnActionEvent; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public class DelegatingActionGroup extends ActionGroup { + private final ActionGroup delegate = new EmptyActionGroup(); + + @Override + public AnAction @NotNull [] getChildren(@Nullable AnActionEvent e) { + return delegate.getChildren(e); + } + + @Override + protected void actionPerformed(AnActionEvent e) { + // no-op + } + + @Override + public void update(@NotNull AnActionEvent e) { + // no-op + } +} diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/EmptyActionGroup.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/EmptyActionGroup.java new file mode 100644 index 000000000..e2138754b --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/EmptyActionGroup.java @@ -0,0 +1,24 @@ +package mock.plugin.overrideOnly; + +import com.intellij.openapi.actionSystem.ActionGroup; +import com.intellij.openapi.actionSystem.AnAction; +import com.intellij.openapi.actionSystem.AnActionEvent; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public class EmptyActionGroup extends ActionGroup { + @Override + public AnAction @NotNull [] getChildren(@Nullable AnActionEvent e) { + return new AnAction[0]; + } + + @Override + protected void actionPerformed(AnActionEvent e) { + // no-op + } + + @Override + public void update(@NotNull AnActionEvent e) { + // no-op + } +} From b19b24d353fa2b7dfff2158a0b37b26939c584bd Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 15:26:05 +0100 Subject: [PATCH 18/31] Fix typo --- .../overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt index f3e623ecb..e06156387 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt @@ -15,9 +15,9 @@ class OverrideOnlyMethodAllowedUsageFilter : ApiUsageFilter { callerMethod: Method, context: VerificationContext): Boolean { - val isSupercall = isSuperclassCall.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) + val isSuperCall = isSuperclassCall.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) val isDelegateCall = isDelegateCall.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) - return isSupercall || isDelegateCall + return isSuperCall || isDelegateCall } } From 7fe012a0f4d42fafad8cf06af9a6c645b19248b8 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 15:26:23 +0100 Subject: [PATCH 19/31] Remove unnecessary method --- .../overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt index e06156387..de926dab1 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt @@ -23,8 +23,5 @@ class OverrideOnlyMethodAllowedUsageFilter : ApiUsageFilter { data class MethodDescriptor(private val methodName: String, private val descriptor: String) -fun Method.matches(methodDescriptor: MethodDescriptor): Boolean = - MethodDescriptor(name, descriptor) == methodDescriptor - fun Method.matches(method: Method): Boolean = MethodDescriptor(name, descriptor) == MethodDescriptor(method.name, method.descriptor) From 2a443eff656e9788ae4ce9ca40d6ed8628f546e4 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 15:30:18 +0100 Subject: [PATCH 20/31] Consolidate method matching --- .../overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt | 3 +-- .../overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt index 601dc9dcf..4656bec70 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt @@ -62,8 +62,7 @@ class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter { } private fun isDelegateInvocationAllowed(callerMethod: Method, delegateMethodInvocationInstruction: MethodInsnNode): Boolean { - return callerMethod.name == delegateMethodInvocationInstruction.name && - callerMethod.descriptor == delegateMethodInvocationInstruction.desc + return delegateMethodInvocationInstruction.matches(callerMethod) } private fun isInvokedMethodAllowed(callerMethod: Method, invokedMethod: Method): Boolean { diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt index de926dab1..89d3fa69e 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt @@ -4,6 +4,7 @@ import com.jetbrains.pluginverifier.verifiers.VerificationContext import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter import com.jetbrains.pluginverifier.verifiers.resolution.Method import org.objectweb.asm.tree.AbstractInsnNode +import org.objectweb.asm.tree.MethodInsnNode class OverrideOnlyMethodAllowedUsageFilter : ApiUsageFilter { @@ -25,3 +26,6 @@ data class MethodDescriptor(private val methodName: String, private val descript fun Method.matches(method: Method): Boolean = MethodDescriptor(name, descriptor) == MethodDescriptor(method.name, method.descriptor) + +fun MethodInsnNode.matches(method: Method): Boolean = + MethodDescriptor(name, desc) == MethodDescriptor(method.name, method.descriptor) From 4284ba9858aa8ee3056c0c970de2c20a5d8ea3c5 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 15:33:01 +0100 Subject: [PATCH 21/31] Move method matching to helpers --- .../pluginverifier/verifiers/resolution/Methods.kt | 11 ++++++++++- .../AnActionUpdateMethodAllowedUsageFilter.kt | 1 + .../DelegateCallOnOverrideOnlyUsageFilter.kt | 1 + .../OverrideOnlyMethodAllowedUsageFilter.kt | 9 --------- .../SuperclassCallOnOverrideOnlyUsageFilter.kt | 1 + 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt index 0a860c91d..5c7cc19c7 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt @@ -5,6 +5,7 @@ import com.jetbrains.plugin.structure.classes.resolvers.Resolver import org.objectweb.asm.Opcodes import org.objectweb.asm.tree.AbstractInsnNode import org.objectweb.asm.tree.ClassNode +import org.objectweb.asm.tree.MethodInsnNode import org.slf4j.Logger import org.slf4j.LoggerFactory @@ -82,4 +83,12 @@ fun isCallOfSuperMethod(callerMethod: Method, calledMethod: Method, instructionN return callerMethod.name == calledMethod.name && callerMethod.descriptor == calledMethod.descriptor && instructionNode.opcode == Opcodes.INVOKESPECIAL -} \ No newline at end of file +} + +data class MethodDescriptor(private val methodName: String, private val descriptor: String) + +fun Method.matches(method: Method): Boolean = + MethodDescriptor(name, descriptor) == MethodDescriptor(method.name, method.descriptor) + +fun MethodInsnNode.matches(method: Method): Boolean = + MethodDescriptor(name, desc) == MethodDescriptor(method.name, method.descriptor) \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt index a49e7deb0..62ce782a6 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt @@ -1,6 +1,7 @@ package com.jetbrains.pluginverifier.usages.overrideOnly import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter +import com.jetbrains.pluginverifier.verifiers.resolution.MethodDescriptor typealias BinaryClassName = String diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt index 4656bec70..82eb91a55 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt @@ -7,6 +7,7 @@ import com.jetbrains.pluginverifier.verifiers.extractClassNameFromDescriptor import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter import com.jetbrains.pluginverifier.verifiers.isSubclassOrSelf import com.jetbrains.pluginverifier.verifiers.resolution.Method +import com.jetbrains.pluginverifier.verifiers.resolution.matches import com.jetbrains.pluginverifier.verifiers.resolution.searchParentOverrides import org.objectweb.asm.tree.* diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt index 89d3fa69e..3805caa99 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt @@ -4,7 +4,6 @@ import com.jetbrains.pluginverifier.verifiers.VerificationContext import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter import com.jetbrains.pluginverifier.verifiers.resolution.Method import org.objectweb.asm.tree.AbstractInsnNode -import org.objectweb.asm.tree.MethodInsnNode class OverrideOnlyMethodAllowedUsageFilter : ApiUsageFilter { @@ -21,11 +20,3 @@ class OverrideOnlyMethodAllowedUsageFilter : ApiUsageFilter { return isSuperCall || isDelegateCall } } - -data class MethodDescriptor(private val methodName: String, private val descriptor: String) - -fun Method.matches(method: Method): Boolean = - MethodDescriptor(name, descriptor) == MethodDescriptor(method.name, method.descriptor) - -fun MethodInsnNode.matches(method: Method): Boolean = - MethodDescriptor(name, desc) == MethodDescriptor(method.name, method.descriptor) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/SuperclassCallOnOverrideOnlyUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/SuperclassCallOnOverrideOnlyUsageFilter.kt index f75efe00a..90a820c51 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/SuperclassCallOnOverrideOnlyUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/SuperclassCallOnOverrideOnlyUsageFilter.kt @@ -4,6 +4,7 @@ import com.jetbrains.pluginverifier.verifiers.VerificationContext import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter import com.jetbrains.pluginverifier.verifiers.resolution.Method import com.jetbrains.pluginverifier.verifiers.resolution.isCallOfSuperMethod +import com.jetbrains.pluginverifier.verifiers.resolution.matches import org.objectweb.asm.tree.AbstractInsnNode class SuperclassCallOnOverrideOnlyUsageFilter : ApiUsageFilter { From d0fac3153e2f0f011bf81058fcddcaa9b4da8b24 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 15:42:55 +0100 Subject: [PATCH 22/31] Use filtered implementation in the usage processor --- .../usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt index 599622768..e7536673a 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt @@ -19,7 +19,7 @@ private val LOG: Logger = LoggerFactory.getLogger(OverrideOnlyMethodUsageProcess class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: OverrideOnlyRegistrar) : ApiUsageProcessor { - private val anActionUpdateMethodAllowedFilter = AnActionUpdateMethodAllowedUsageFilter() + private val allowedOverrideOnlyUsageFilter = OverrideOnlyMethodAllowedUsageFilter() override fun processMethodInvocation( methodReference: MethodReference, @@ -68,7 +68,7 @@ class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: Overri callerMethod: Method, context: VerificationContext): Boolean { return isCallOfSuperConstructor(callerMethod, invokedMethod) - || anActionUpdateMethodAllowedFilter.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) + || allowedOverrideOnlyUsageFilter.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) } private fun isCallOfSuperConstructor(callerMethod: Method, resolvedMethod: Method) = From 15cfd9e43a208a485dd409fedc802c8f21e1914a Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 15:57:39 +0100 Subject: [PATCH 23/31] Implement and use composite API usage filter --- .../verifiers/filter/CompositeApiUsageFilter.kt | 15 +++++++++++++++ .../OverrideOnlyMethodUsageProcessor.kt | 6 +++++- 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/CompositeApiUsageFilter.kt diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/CompositeApiUsageFilter.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/CompositeApiUsageFilter.kt new file mode 100644 index 000000000..c3dc1c530 --- /dev/null +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/CompositeApiUsageFilter.kt @@ -0,0 +1,15 @@ +package com.jetbrains.pluginverifier.verifiers.filter + +import com.jetbrains.pluginverifier.verifiers.VerificationContext +import com.jetbrains.pluginverifier.verifiers.resolution.Method +import org.objectweb.asm.tree.AbstractInsnNode + +class CompositeApiUsageFilter(private val apiUsageFilters: List) : ApiUsageFilter { + constructor(vararg filters: ApiUsageFilter) : this(filters.toList()) + + override fun allowMethodInvocation(invokedMethod: Method, invocationInstruction: AbstractInsnNode, callerMethod: Method, context: VerificationContext): Boolean { + return apiUsageFilters.any { + it.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) + } + } +} \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt index e7536673a..fa718ce1e 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt @@ -8,6 +8,7 @@ import com.jetbrains.pluginverifier.results.reference.MethodReference import com.jetbrains.pluginverifier.usages.ApiUsageProcessor import com.jetbrains.pluginverifier.usages.util.findEffectiveMemberAnnotation import com.jetbrains.pluginverifier.verifiers.VerificationContext +import com.jetbrains.pluginverifier.verifiers.filter.CompositeApiUsageFilter import com.jetbrains.pluginverifier.verifiers.findAnnotation import com.jetbrains.pluginverifier.verifiers.resolution.Method import com.jetbrains.pluginverifier.verifiers.resolution.searchParentOverrides @@ -19,7 +20,10 @@ private val LOG: Logger = LoggerFactory.getLogger(OverrideOnlyMethodUsageProcess class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: OverrideOnlyRegistrar) : ApiUsageProcessor { - private val allowedOverrideOnlyUsageFilter = OverrideOnlyMethodAllowedUsageFilter() + private val allowedOverrideOnlyUsageFilter = CompositeApiUsageFilter( + DelegateCallOnOverrideOnlyUsageFilter(), + SuperclassCallOnOverrideOnlyUsageFilter() + ) override fun processMethodInvocation( methodReference: MethodReference, From bea5393ece70eb894f24efb0806cc1947b07dcdf Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 15:59:16 +0100 Subject: [PATCH 24/31] Move BinaryClassName alias to method helpers --- .../pluginverifier/verifiers/resolution/Methods.kt | 6 ++++++ .../overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt | 2 +- .../overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt index 5c7cc19c7..39b2df663 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt @@ -11,6 +11,12 @@ import org.slf4j.LoggerFactory private val LOG: Logger = LoggerFactory.getLogger(Method::class.java) +/** + * Indicates a binary class name where packages are delimited by '/' instead of '.'. + * This is in line with Java Language Specification. + */ +typealias BinaryClassName = String + fun Method.isOverriding(anotherMethod: Method): Boolean = nonStaticAndNonFinal(anotherMethod) && sameName(this, anotherMethod) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt index 62ce782a6..6af4b8978 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt @@ -1,10 +1,10 @@ package com.jetbrains.pluginverifier.usages.overrideOnly import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter +import com.jetbrains.pluginverifier.verifiers.resolution.BinaryClassName import com.jetbrains.pluginverifier.verifiers.resolution.MethodDescriptor -typealias BinaryClassName = String val anActionUpdateMethodDescriptor = MethodDescriptor("update", "(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V") const val anActionClass: BinaryClassName = "com/intellij/openapi/actionSystem/AnAction" diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt index 82eb91a55..3fffa2b38 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt @@ -6,6 +6,7 @@ import com.jetbrains.pluginverifier.verifiers.VerificationContext import com.jetbrains.pluginverifier.verifiers.extractClassNameFromDescriptor import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter import com.jetbrains.pluginverifier.verifiers.isSubclassOrSelf +import com.jetbrains.pluginverifier.verifiers.resolution.BinaryClassName import com.jetbrains.pluginverifier.verifiers.resolution.Method import com.jetbrains.pluginverifier.verifiers.resolution.matches import com.jetbrains.pluginverifier.verifiers.resolution.searchParentOverrides From dc349e953a1b03fbf3ed3cb53cdc82863a233e67 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 16:00:10 +0100 Subject: [PATCH 25/31] Remove technically specific implementation in favour of general one --- .../AnActionUpdateMethodAllowedUsageFilter.kt | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt deleted file mode 100644 index 6af4b8978..000000000 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/AnActionUpdateMethodAllowedUsageFilter.kt +++ /dev/null @@ -1,14 +0,0 @@ -package com.jetbrains.pluginverifier.usages.overrideOnly - -import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter -import com.jetbrains.pluginverifier.verifiers.resolution.BinaryClassName -import com.jetbrains.pluginverifier.verifiers.resolution.MethodDescriptor - - - -val anActionUpdateMethodDescriptor = MethodDescriptor("update", "(Lcom/intellij/openapi/actionSystem/AnActionEvent;)V") -const val anActionClass: BinaryClassName = "com/intellij/openapi/actionSystem/AnAction" - -class AnActionUpdateMethodAllowedUsageFilter( - private val delegate: ApiUsageFilter - = OverrideOnlyMethodAllowedUsageFilter()) : ApiUsageFilter by delegate \ No newline at end of file From c6b01c865d454a7d1359fb607ccf83fdfe4c1fd1 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 16:03:20 +0100 Subject: [PATCH 26/31] Extract OverrideOnly super constructor call to a separate class --- ...ConstructorOverrideOnlyAllowedUsageFilter.kt | 17 +++++++++++++++++ .../OverrideOnlyMethodUsageProcessor.kt | 9 ++------- 2 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/CallOfSuperConstructorOverrideOnlyAllowedUsageFilter.kt diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/CallOfSuperConstructorOverrideOnlyAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/CallOfSuperConstructorOverrideOnlyAllowedUsageFilter.kt new file mode 100644 index 000000000..dacf0314c --- /dev/null +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/CallOfSuperConstructorOverrideOnlyAllowedUsageFilter.kt @@ -0,0 +1,17 @@ +package com.jetbrains.pluginverifier.usages.overrideOnly + +import com.jetbrains.pluginverifier.verifiers.VerificationContext +import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter +import com.jetbrains.pluginverifier.verifiers.resolution.Method +import org.objectweb.asm.tree.AbstractInsnNode + +class CallOfSuperConstructorOverrideOnlyAllowedUsageFilter: ApiUsageFilter { + override fun allowMethodInvocation(invokedMethod: Method, + invocationInstruction: AbstractInsnNode, + callerMethod: Method, + context: VerificationContext): Boolean { + return invokedMethod.isConstructor + && callerMethod.isConstructor + && callerMethod.containingClassFile.superName == invokedMethod.containingClassFile.name + } +} \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt index fa718ce1e..d0bf6f877 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt @@ -21,6 +21,7 @@ private val LOG: Logger = LoggerFactory.getLogger(OverrideOnlyMethodUsageProcess class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: OverrideOnlyRegistrar) : ApiUsageProcessor { private val allowedOverrideOnlyUsageFilter = CompositeApiUsageFilter( + CallOfSuperConstructorOverrideOnlyAllowedUsageFilter(), DelegateCallOnOverrideOnlyUsageFilter(), SuperclassCallOnOverrideOnlyUsageFilter() ) @@ -71,15 +72,9 @@ class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: Overri invocationInstruction: AbstractInsnNode, callerMethod: Method, context: VerificationContext): Boolean { - return isCallOfSuperConstructor(callerMethod, invokedMethod) - || allowedOverrideOnlyUsageFilter.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) + return allowedOverrideOnlyUsageFilter.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) } - private fun isCallOfSuperConstructor(callerMethod: Method, resolvedMethod: Method) = - resolvedMethod.isConstructor - && callerMethod.isConstructor - && callerMethod.containingClassFile.superName == resolvedMethod.containingClassFile.name - private fun Method.isOverrideOnlyMethod(context: VerificationContext): Boolean = annotations.findAnnotation(overrideOnlyAnnotationName) != null || containingClassFile.annotations.findAnnotation(overrideOnlyAnnotationName) != null From 72dbe05ac770677676475ab4b339d940eff00f79 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Wed, 13 Mar 2024 16:04:02 +0100 Subject: [PATCH 27/31] Remove superflous class Replaced by composite allowed usage filter --- .../OverrideOnlyMethodAllowedUsageFilter.kt | 22 ------------------- 1 file changed, 22 deletions(-) delete mode 100644 intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt deleted file mode 100644 index 3805caa99..000000000 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodAllowedUsageFilter.kt +++ /dev/null @@ -1,22 +0,0 @@ -package com.jetbrains.pluginverifier.usages.overrideOnly - -import com.jetbrains.pluginverifier.verifiers.VerificationContext -import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter -import com.jetbrains.pluginverifier.verifiers.resolution.Method -import org.objectweb.asm.tree.AbstractInsnNode - -class OverrideOnlyMethodAllowedUsageFilter : ApiUsageFilter { - - private val isDelegateCall = DelegateCallOnOverrideOnlyUsageFilter() - private val isSuperclassCall = SuperclassCallOnOverrideOnlyUsageFilter() - - override fun allowMethodInvocation(invokedMethod: Method, - invocationInstruction: AbstractInsnNode, - callerMethod: Method, - context: VerificationContext): Boolean { - - val isSuperCall = isSuperclassCall.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) - val isDelegateCall = isDelegateCall.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context) - return isSuperCall || isDelegateCall - } -} From a039c185c18a018633ac54e5a2888510c915b3c5 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Mon, 18 Mar 2024 15:32:56 +0100 Subject: [PATCH 28/31] Polish --- .../usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt index 3fffa2b38..d21b58087 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt @@ -57,7 +57,6 @@ class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter { * * @return the class name of the discovered class. */ - // private fun Method.getSuperClassName(resolver: Resolver): BinaryClassName { val topMostClass = searchParentOverrides(resolver).lastOrNull() ?: return containingClassFile.name return topMostClass.klass.name From a0d7360cafe825d90c8f5825ec8f6923ff545927 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Mon, 18 Mar 2024 16:21:54 +0100 Subject: [PATCH 29/31] Test method overrides in java.lang.Object --- .../verifiers/resolution/Methods.kt | 9 ++- .../verifiers/resolution/MethodsTest.kt | 55 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt index 39b2df663..0e6eece29 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt @@ -17,6 +17,8 @@ private val LOG: Logger = LoggerFactory.getLogger(Method::class.java) */ typealias BinaryClassName = String +const val JAVA_LANG_OBJECT: BinaryClassName = "java/lang/Object" + fun Method.isOverriding(anotherMethod: Method): Boolean = nonStaticAndNonFinal(anotherMethod) && sameName(this, anotherMethod) @@ -49,10 +51,13 @@ private val Method.visibilityRating: Int * Search for all methods in the parent hierarchy that the receiver overrides. * @return list of methods, sorted from bottom-to-top in the inheritance hierarchy. */ -fun Method.searchParentOverrides(resolver: Resolver): List { +fun Method.searchParentOverrides(resolver: Resolver, ignoreJavaLangObject: Boolean = false): List { return mutableListOf().apply { searchParentOverrides(resolver) { klass: ClassFile, method: Method -> - add(MethodInClass(method, klass)) + when { + klass.name == JAVA_LANG_OBJECT && ignoreJavaLangObject -> Unit + else -> add(MethodInClass(method, klass)) + } } } } diff --git a/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt b/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt new file mode 100644 index 000000000..cd4e566db --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt @@ -0,0 +1,55 @@ +package com.jetbrains.pluginverifier.verifiers.resolution + +import com.jetbrains.plugin.structure.classes.resolvers.* +import com.jetbrains.pluginverifier.jdk.JdkDescriptorCreator +import com.jetbrains.pluginverifier.tests.findMockPluginJarPath +import org.hamcrest.CoreMatchers.`is` +import org.hamcrest.MatcherAssert.assertThat +import org.junit.Assert.fail +import org.junit.Test +import java.nio.file.Path + +const val JAVA_LANG_OBJECT: BinaryClassName = "java/lang/Object" + +class MethodsTest { + @Test + fun testSearchParentOverrides() { + val javaHome = System.getProperty("java.home") + val jdkDescriptor = JdkDescriptorCreator.createJdkDescriptor(Path.of(javaHome)) + + val jdkResolver = jdkDescriptor.jdkResolver + val mockPluginResolver = createTestResolver() + val resolver = CompositeResolver.create( + jdkResolver, mockPluginResolver + ) + + val observedClassName = "mock/plugin/method/MethodOverriddenFromJavaLangObject" + val overriddenMethodName = "toString" + val observerClassResult = resolver.resolveClass(observedClassName) + if (observerClassResult !is ResolutionResult.Found) { + fail("Class '$observedClassName' not found in the mock plugin by resolver") + } else { + val classNode = observerClassResult.value + val toStringMethod = classNode.methods.first { it.name == overriddenMethodName } + val classFile = ClassFileAsm(observerClassResult.value, observerClassResult.fileOrigin) + val method: Method = MethodAsm(classFile, toStringMethod) + + val methodOverrides = method.searchParentOverrides(resolver) + assertThat(methodOverrides.size, `is`(1)) + val javaLangObjectToString = methodOverrides.first() + assertThat(javaLangObjectToString.klass.name, `is`(JAVA_LANG_OBJECT)) + + val nonJavaLangObjectMethodOverrides = method.searchParentOverrides(resolver, ignoreJavaLangObject = true) + assertThat(nonJavaLangObjectMethodOverrides.size, `is`(0)) + } + } + + private fun createTestResolver(): Resolver = + JarFileResolver( + findMockPluginJarPath(), + Resolver.ReadMode.FULL, + object : FileOrigin { + override val parent: FileOrigin? = null + } + ) +} \ No newline at end of file From 89e2ceeff0568ce10ca4b9c01171a4d831288a0d Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Mon, 18 Mar 2024 16:34:53 +0100 Subject: [PATCH 30/31] Do not treat java.lang.Object methods as override candidates by default --- .../pluginverifier/verifiers/resolution/Methods.kt | 2 +- .../overrideOnly/OverrideOnlyMethodUsageProcessor.kt | 4 ++-- .../plugin/method/MethodOverriddenFromJavaLangObject.java | 8 ++++++++ .../pluginverifier/verifiers/resolution/MethodsTest.kt | 4 ++-- 4 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/method/MethodOverriddenFromJavaLangObject.java diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt index 0e6eece29..bfd5369f7 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt @@ -51,7 +51,7 @@ private val Method.visibilityRating: Int * Search for all methods in the parent hierarchy that the receiver overrides. * @return list of methods, sorted from bottom-to-top in the inheritance hierarchy. */ -fun Method.searchParentOverrides(resolver: Resolver, ignoreJavaLangObject: Boolean = false): List { +fun Method.searchParentOverrides(resolver: Resolver, ignoreJavaLangObject: Boolean = true): List { return mutableListOf().apply { searchParentOverrides(resolver) { klass: ClassFile, method: Method -> when { diff --git a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt index d0bf6f877..89c85a5ce 100644 --- a/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt +++ b/intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt @@ -89,10 +89,10 @@ class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: Overri overriddenMethod.findEffectiveMemberAnnotation(annotationFqn, verificationContext.classResolver) != null } return if (overriddenMethod == null) { - LOG.trace("No overridden method for $name is annotated by [$annotationFqn]") + LOG.atTrace().log("No overridden method for $name is annotated by [$annotationFqn]") false } else { - LOG.debug("Method '${overriddenMethod.method.name}' in [${overriddenMethod.klass.name}] is annotated by [$annotationFqn]") + LOG.atDebug().log("Method '${overriddenMethod.method.name}' in [${overriddenMethod.klass.name}] is annotated by [$annotationFqn]") true } } diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/method/MethodOverriddenFromJavaLangObject.java b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/method/MethodOverriddenFromJavaLangObject.java new file mode 100644 index 000000000..1f91e1fcb --- /dev/null +++ b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/method/MethodOverriddenFromJavaLangObject.java @@ -0,0 +1,8 @@ +package mock.plugin.method; + +public class MethodOverriddenFromJavaLangObject { + @Override + public String toString() { + return "Method overridden from java.lang.Object"; + } +} diff --git a/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt b/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt index cd4e566db..00a5cd4ce 100644 --- a/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt +++ b/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt @@ -34,12 +34,12 @@ class MethodsTest { val classFile = ClassFileAsm(observerClassResult.value, observerClassResult.fileOrigin) val method: Method = MethodAsm(classFile, toStringMethod) - val methodOverrides = method.searchParentOverrides(resolver) + val methodOverrides = method.searchParentOverrides(resolver, ignoreJavaLangObject = false) assertThat(methodOverrides.size, `is`(1)) val javaLangObjectToString = methodOverrides.first() assertThat(javaLangObjectToString.klass.name, `is`(JAVA_LANG_OBJECT)) - val nonJavaLangObjectMethodOverrides = method.searchParentOverrides(resolver, ignoreJavaLangObject = true) + val nonJavaLangObjectMethodOverrides = method.searchParentOverrides(resolver) assertThat(nonJavaLangObjectMethodOverrides.size, `is`(0)) } } From 0e498efe2c853318a749a84738cd586d50229042 Mon Sep 17 00:00:00 2001 From: Robert Novotny Date: Mon, 18 Mar 2024 20:49:00 +0100 Subject: [PATCH 31/31] Ignore java.lang.Object methods in overrides --- .../pluginverifier/verifiers/resolution/Methods.kt | 4 ++-- .../verifiers/resolution/MethodsTest.kt | 11 ++--------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt index bfd5369f7..c9b9e3bb5 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt +++ b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt @@ -51,11 +51,11 @@ private val Method.visibilityRating: Int * Search for all methods in the parent hierarchy that the receiver overrides. * @return list of methods, sorted from bottom-to-top in the inheritance hierarchy. */ -fun Method.searchParentOverrides(resolver: Resolver, ignoreJavaLangObject: Boolean = true): List { +fun Method.searchParentOverrides(resolver: Resolver): List { return mutableListOf().apply { searchParentOverrides(resolver) { klass: ClassFile, method: Method -> when { - klass.name == JAVA_LANG_OBJECT && ignoreJavaLangObject -> Unit + klass.name == JAVA_LANG_OBJECT -> Unit else -> add(MethodInClass(method, klass)) } } diff --git a/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt b/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt index 00a5cd4ce..8c4689cb6 100644 --- a/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt +++ b/intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/verifiers/resolution/MethodsTest.kt @@ -9,8 +9,6 @@ import org.junit.Assert.fail import org.junit.Test import java.nio.file.Path -const val JAVA_LANG_OBJECT: BinaryClassName = "java/lang/Object" - class MethodsTest { @Test fun testSearchParentOverrides() { @@ -34,13 +32,8 @@ class MethodsTest { val classFile = ClassFileAsm(observerClassResult.value, observerClassResult.fileOrigin) val method: Method = MethodAsm(classFile, toStringMethod) - val methodOverrides = method.searchParentOverrides(resolver, ignoreJavaLangObject = false) - assertThat(methodOverrides.size, `is`(1)) - val javaLangObjectToString = methodOverrides.first() - assertThat(javaLangObjectToString.klass.name, `is`(JAVA_LANG_OBJECT)) - - val nonJavaLangObjectMethodOverrides = method.searchParentOverrides(resolver) - assertThat(nonJavaLangObjectMethodOverrides.size, `is`(0)) + val methodOverrides = method.searchParentOverrides(resolver) + assertThat(methodOverrides.size, `is`(0)) } }