Add unit test for OverrideOnly checks (#1113)

* Add mock verification context
* Add test for super method invocations in delegates
* Add test for static invocations and super invocations

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

@@ -29,8 +29,9 @@ class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter {
     }
 
     val callMethod = invocationInstruction.narrow<MethodInsnNode>() ?: return false
-    val loadMethodParameter = callMethod.previousInstruction<VarInsnNode>() ?: return false
-    val getDelegateField = loadMethodParameter.previousInstruction<FieldInsnNode>() ?: return false
+    val loadMethodParameter = callMethod.previousOf<VarInsnNode>() ?: return false
+    if (loadMethodParameter.isLoadThisReferenceOnOperandStack()) return false
+    val getDelegateField = loadMethodParameter.previousOf<FieldInsnNode>() ?: return false
 
     val delegateBinaryClassName = getDelegateField.fieldClass ?: return false
     val delegateClassNode = when (val classResolution = resolveClass(delegateBinaryClassName)) {
@@ -77,10 +78,6 @@ class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter {
     && !invocationInstruction.isStatic
     && !isCallOfSuperMethod(callerMethod, invokedMethod, invocationInstruction)
 
-  private inline fun <reified T : AbstractInsnNode> AbstractInsnNode?.previousInstruction(): T? {
-    return this?.previous?.narrow<T>()
-  }
-
   private inline fun <reified T : AbstractInsnNode> AbstractInsnNode.narrow(): T? {
     return if (this is T) this else null
   }
@@ -90,4 +87,19 @@ class DelegateCallOnOverrideOnlyUsageFilter : ApiUsageFilter {
 
   private val AbstractInsnNode.isStatic: Boolean
     get() = opcode == Opcodes.INVOKESTATIC
+
+  private fun VarInsnNode.isLoadThisReferenceOnOperandStack(): Boolean {
+    // see JLS§2.6.1: On instance method invocation, local variable 0 is always used to pass `this` in Java
+    return opcode == Opcodes.ALOAD && `var` == 0
+  }
+
+  private val AbstractInsnNode.previousNodes
+    get() = generateSequence(this.previous) {
+      it.previous
+    }
+
+  private inline fun <reified T : AbstractInsnNode> AbstractInsnNode.previousOf(): T? =
+    previousNodes
+      .firstOrNull { it is T }
+      ?.narrow()
 }

intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/ClearCountingContainer.java

@@ -0,0 +1,11 @@
+package mock.plugin.overrideOnly;
+
+public class ClearCountingContainer extends Container {
+    private int clearCount = 0;
+
+    @Override
+    public void clear() {
+        super.clear();
+        clearCount++;
+    }
+}

intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/Container.java

@@ -0,0 +1,7 @@
+package mock.plugin.overrideOnly;
+
+public class Container {
+    public void clear() {
+        // do nothing
+    }
+}

intellij-plugin-verifier/verifier-test/mock-plugin/src/main/java/mock/plugin/overrideOnly/PackageInvokingBox.java

@@ -0,0 +1,14 @@
+package mock.plugin.overrideOnly;
+
+@SuppressWarnings("unused")
+public class PackageInvokingBox {
+    /*expected(DEPRECATED)
+    Deprecated method java.lang.Package.getPackage(String) invocation
+
+    Deprecated method java.lang.Package.getPackage(java.lang.String name) : java.lang.Package is invoked in mock.plugin.overrideOnly.PackageInvokingBox.getPackage(String) : Package
+    */
+    @SuppressWarnings("deprecation")
+    public Package getPackage(String pkg) {
+        return Package.getPackage(pkg);
+    }
+}

intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/tests/mocks/MockVerificationContext.kt

@@ -0,0 +1,28 @@
+package com.jetbrains.pluginverifier.tests.mocks
+
+import com.jetbrains.plugin.structure.classes.resolvers.EmptyResolver
+import com.jetbrains.pluginverifier.results.problems.CompatibilityProblem
+import com.jetbrains.pluginverifier.usages.ApiUsageProcessor
+import com.jetbrains.pluginverifier.verifiers.ProblemRegistrar
+import com.jetbrains.pluginverifier.verifiers.VerificationContext
+import com.jetbrains.pluginverifier.verifiers.packages.DefaultPackageFilter
+import com.jetbrains.pluginverifier.warnings.CompatibilityWarning
+import com.jetbrains.pluginverifier.warnings.WarningRegistrar
+
+class MockVerificationContext : VerificationContext {
+  override val classResolver = EmptyResolver
+
+  override val externalClassesPackageFilter = DefaultPackageFilter(emptyList())
+
+  override val problemRegistrar = object : ProblemRegistrar {
+    override fun registerProblem(problem: CompatibilityProblem) {
+      // NO-OP
+    }
+  }
+  override val warningRegistrar = object : WarningRegistrar {
+    override fun registerCompatibilityWarning(warning: CompatibilityWarning) {
+      // NO-OP
+    }
+  }
+  override val apiUsageProcessors = emptyList<ApiUsageProcessor>()
+}

intellij-plugin-verifier/verifier-test/src/test/java/com/jetbrains/pluginverifier/usages/overrideOnly/DelegateCallOnOverrideOnlyUsageFilterTest.kt

@@ -0,0 +1,115 @@
+package com.jetbrains.pluginverifier.usages.overrideOnly
+
+import com.jetbrains.plugin.structure.classes.resolvers.FileOrigin
+import com.jetbrains.pluginverifier.tests.mocks.MockVerificationContext
+import com.jetbrains.pluginverifier.verifiers.VerificationContext
+import com.jetbrains.pluginverifier.verifiers.filter.ApiUsageFilter
+import com.jetbrains.pluginverifier.verifiers.resolution.BinaryClassName
+import com.jetbrains.pluginverifier.verifiers.resolution.ClassFileAsm
+import com.jetbrains.pluginverifier.verifiers.resolution.FullyQualifiedClassName
+import com.jetbrains.pluginverifier.verifiers.resolution.MethodAsm
+import com.jetbrains.pluginverifier.verifiers.resolution.toBinaryClassName
+import org.junit.Assert.*
+import org.junit.Before
+import org.junit.Test
+import org.objectweb.asm.ClassReader
+import org.objectweb.asm.tree.ClassNode
+import org.objectweb.asm.tree.MethodInsnNode
+
+
+private const val NO_PARAMS_RETURN_VOID_DESCRIPTOR = "()V"
+private const val STRING_PARAM_RETURN_PACKAGE_DESCRIPTOR = "(Ljava/lang/String;)Ljava/lang/Package;"
+
+private const val CLEAR_METHOD = "clear"
+private const val GET_PACKAGE_METHOD = "getPackage"
+
+class DelegateCallOnOverrideOnlyUsageFilterTest {
+  private lateinit var verificationContext: VerificationContext
+
+  private lateinit var filter: ApiUsageFilter
+
+  @Before
+  fun setUp() {
+    verificationContext = MockVerificationContext()
+    filter = DelegateCallOnOverrideOnlyUsageFilter()
+  }
+
+  @Test
+  fun `invocation of super as delegate is intentionally ignored`() {
+    val className = "mock.plugin.overrideOnly.ClearCountingContainer"
+    val clearMethod = ClearCountingContainerNode().loadMethod(CLEAR_METHOD, NO_PARAMS_RETURN_VOID_DESCRIPTOR)
+    assertNotNull("Unable to find '$CLEAR_METHOD' method on the $className", clearMethod)
+    clearMethod!!
+
+    val containerFqn = "mock.plugin.overrideOnly.Container"
+    val containerClearMethodAsm = ContainerNode().loadMethod(CLEAR_METHOD, NO_PARAMS_RETURN_VOID_DESCRIPTOR)
+    assertNotNull("Unable to find '$CLEAR_METHOD' method on the $className", containerClearMethodAsm)
+    containerClearMethodAsm!!
+
+    val superClearInstruction = clearMethod.findInvocation(containerFqn.toBinaryClassName(), CLEAR_METHOD, NO_PARAMS_RETURN_VOID_DESCRIPTOR)
+    if (superClearInstruction == null) fail("Unable to find '$CLEAR_METHOD' method on the $className")
+    superClearInstruction!!
+
+    val isAllowed = filter.allow(containerClearMethodAsm, superClearInstruction, clearMethod, verificationContext)
+    assertFalse(isAllowed)
+  }
+
+  @Test
+  fun `invocation of similarly named static method is ignored`() {
+    val className = "mock.plugin.overrideOnly.PackageInvokingBox"
+    val getPackageMethodAsm = PackageInvokingBoxNode().loadMethod(GET_PACKAGE_METHOD, STRING_PARAM_RETURN_PACKAGE_DESCRIPTOR)
+    assertNotNull("Unable to find '$CLEAR_METHOD' method on the $className", getPackageMethodAsm)
+    getPackageMethodAsm!!
+
+    val targetClassName = "java.lang.Package"
+    val getPackageInstruction = getPackageMethodAsm.findInvocation(targetClassName.toBinaryClassName(), GET_PACKAGE_METHOD, STRING_PARAM_RETURN_PACKAGE_DESCRIPTOR)
+    if (getPackageInstruction == null) fail("Unable to find '$GET_PACKAGE_METHOD' method on the $className")
+    getPackageInstruction!!
+
+    val getPackageTargetMethodAsm = loadMethod(targetClassName, GET_PACKAGE_METHOD, STRING_PARAM_RETURN_PACKAGE_DESCRIPTOR)
+    assertNotNull("Unable to find '$GET_PACKAGE_METHOD' method on the $targetClassName", getPackageTargetMethodAsm)
+    getPackageTargetMethodAsm!!
+
+    val isAllowed = filter.allow(getPackageTargetMethodAsm, getPackageInstruction, getPackageMethodAsm, verificationContext)
+    assertFalse(isAllowed)
+  }
+
+  private fun MethodAsm.findInvocation(
+    invokedMethodOwner: BinaryClassName,
+    invokedMethodName: String,
+    invokedMethodDescriptor: String
+  ): MethodInsnNode? =
+    asmNode.instructions.find {
+      it is MethodInsnNode
+        && it.name == invokedMethodName
+        && it.owner == invokedMethodOwner
+        && it.desc == invokedMethodDescriptor
+    }?.let { it as MethodInsnNode }
+
+  private fun ClassNode.loadMethod(methodName: String, methodDescriptor: String): MethodAsm? {
+    val classFile = ClassFileAsm(this, TestClasspathFileOrigin)
+    return classFile.methods.find { it.name == methodName && it.descriptor == methodDescriptor }
+  }
+
+  private fun loadMethod(className: FullyQualifiedClassName, methodName: String, methodDescriptor: String): MethodAsm? {
+    val classFile = getClassFile(className)
+    return classFile.methods.find { it.name == methodName && it.descriptor == methodDescriptor }
+  }
+
+  private fun getClassFile(className: FullyQualifiedClassName): ClassFileAsm {
+    val classNode = loadClass(className)
+    return ClassFileAsm(classNode, TestClasspathFileOrigin)
+  }
+
+  private fun loadClass(className: FullyQualifiedClassName): ClassNode {
+    val classNode = ClassNode()
+    val classReader = ClassReader(className)
+    classReader.accept(classNode, 0)
+    return classNode
+  }
+
+  private object TestClasspathFileOrigin : FileOrigin {
+    override val parent: FileOrigin? = null
+  }
+
+}