Allow OverrideOnly method calls for delegation and wrapping in the same class hierarchy

intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/filter/ApiUsageFilter.kt

@@ -0,0 +1,12 @@
+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
+
+interface ApiUsageFilter {
+ fun allowMethodInvocation(invokedMethod: Method,
+ invocationInstruction: AbstractInsnNode,
+ callerMethod: Method,
+ context: VerificationContext): Boolean = true
+}

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>) : 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)
+ }
+ }
+}

intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/Methods.kt

@@ -0,0 +1,105 @@
+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.objectweb.asm.tree.MethodInsnNode
+import org.slf4j.Logger
+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
+
+const val JAVA_LANG_OBJECT: BinaryClassName = "java/lang/Object"
+
+fun Method.isOverriding(anotherMethod: Method): Boolean =
+ nonStaticAndNonFinal(anotherMethod)
+ && sameName(this, anotherMethod)
+ && 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
+
+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
+ }
+
+/**
+ * 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<MethodInClass> {
+ return mutableListOf<MethodInClass>().apply {
+ searchParentOverrides(resolver) { klass: ClassFile, method: Method ->
+ when {
+ klass.name == JAVA_LANG_OBJECT -> Unit
+ else -> 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)) {
+ is ResolutionResult.Found<ClassNode> -> 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
+ }
+ }
+}
+
+fun isCallOfSuperMethod(callerMethod: Method, calledMethod: Method, instructionNode: AbstractInsnNode): Boolean {
+ return callerMethod.name == calledMethod.name
+ && callerMethod.descriptor == calledMethod.descriptor
+ && instructionNode.opcode == Opcodes.INVOKESPECIAL
+}
+
+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)

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
+ }
+}

intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilter.kt

@@ -0,0 +1,79 @@
+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.BinaryClassName
+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.*
+
+class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter {
+ @Suppress("UNUSED_VARIABLE")
+ 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<MethodInsnNode>() ?: return false
+ ins = ins.previous
+ val loadMethodParameter = ins.narrow<VarInsnNode>() ?: return false
+ ins = ins.previous
+ val getDelegateField = ins.narrow<FieldInsnNode>() ?: return false
+
+ val delegateBinaryClassName = getDelegateField.fieldClass ?: return false
+ val delegateClassNode = when (val classResolution = resolveClass(delegateBinaryClassName)) {
+ is ResolutionResult.Found<ClassNode> -> 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, callerMethodTopMostSuperClass)
+ if (!isSubclass) {
+ return false
+ }
+ if (!isDelegateInvocationAllowed(callerMethod, callMethod)) {
+ return false
+ }
+ 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 delegateMethodInvocationInstruction.matches(callerMethod)
+ }
+
+ private fun isInvokedMethodAllowed(callerMethod: Method, invokedMethod: Method): Boolean {
+ return callerMethod.matches(invokedMethod)
+ }
+
+ private inline fun <reified T : AbstractInsnNode> AbstractInsnNode.narrow(): T? {
+ return if (this is T) this else null
+ }
+
+ private val FieldInsnNode.fieldClass: BinaryClassName?
+ get() = desc.extractClassNameFromDescriptor()
+}

intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/OverrideOnlyMethodUsageProcessor.kt

@@ -6,37 +6,100 @@ 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.filter.CompositeApiUsageFilter
 import com.jetbrains.pluginverifier.verifiers.findAnnotation
 import com.jetbrains.pluginverifier.verifiers.resolution.Method
+import com.jetbrains.pluginverifier.verifiers.resolution.searchParentOverrides
 import org.objectweb.asm.tree.AbstractInsnNode
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+private val LOG: Logger = LoggerFactory.getLogger(OverrideOnlyMethodUsageProcessor::class.java)
 
 class OverrideOnlyMethodUsageProcessor(private val overrideOnlyRegistrar: OverrideOnlyRegistrar) : ApiUsageProcessor {
 
+ private val allowedOverrideOnlyUsageFilter = CompositeApiUsageFilter(
+ CallOfSuperConstructorOverrideOnlyAllowedUsageFilter(),
+ DelegateCallOnOverrideOnlyUsageFilter(),
+ SuperclassCallOnOverrideOnlyUsageFilter()
+ )
+
  override fun processMethodInvocation(
  methodReference: MethodReference,
  resolvedMethod: Method,
  instructionNode: AbstractInsnNode,
  callerMethod: Method,
  context: VerificationContext
  ) {
- if (resolvedMethod.isOverrideOnlyMethod() && !isCallOfSuperConstructor(callerMethod, resolvedMethod)) {
+
+
+ if (resolvedMethod.isOverrideOnlyMethod(context)
+ && !isAllowedOverrideOnlyUsage(methodReference, resolvedMethod, instructionNode, callerMethod, context))
+ {
  overrideOnlyRegistrar.registerOverrideOnlyMethodUsage(
  OverrideOnlyMethodUsage(methodReference, resolvedMethod.location, callerMethod.location)
  )
  }
  }
 
- private fun isCallOfSuperConstructor(callerMethod: Method, resolvedMethod: Method) =
- resolvedMethod.isConstructor
- && callerMethod.isConstructor
- && callerMethod.containingClassFile.superName == resolvedMethod.containingClassFile.name
+ /**
+ * 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 allowedOverrideOnlyUsageFilter.allowMethodInvocation(invokedMethod, invocationInstruction, callerMethod, context)
+ }
 
- private fun Method.isOverrideOnlyMethod(): Boolean =
+ 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.atTrace().log("No overridden method for $name is annotated by [$annotationFqn]")
+ false
+ } else {
+ LOG.atDebug().log("Method '${overriddenMethod.method.name}' in [${overriddenMethod.klass.name}] is annotated by [$annotationFqn]")
+ true
+ }
+ }
 
  private companion object {
  const val overrideOnlyAnnotationName = "org/jetbrains/annotations/ApiStatus\$OverrideOnly"
  }
-}
+}
+
+

intellij-plugin-verifier/verifier-intellij/src/main/java/com/jetbrains/pluginverifier/usages/overrideOnly/SuperclassCallOnOverrideOnlyUsageFilter.kt

@@ -0,0 +1,15 @@
+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 com.jetbrains.pluginverifier.verifiers.resolution.matches
+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)
+ }
+}

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);
+}
+
+

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);
 }