MP-5395 in between interface

intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/method/KotlinMethods.kt

@@ -1,3 +1,6 @@
+/*
+ * Copyright 2000-2023 JetBrains s.r.o. and other contributors. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+ */
 package com.jetbrains.pluginverifier.verifiers.method
 
 import com.google.common.cache.Cache
@@ -7,6 +10,8 @@ import com.jetbrains.pluginverifier.verifiers.resolution.Method
 import org.objectweb.asm.Opcodes
 import org.objectweb.asm.tree.AbstractInsnNode
 import org.objectweb.asm.tree.MethodInsnNode
+import org.objectweb.asm.tree.TypeInsnNode
+import org.objectweb.asm.tree.VarInsnNode
 
 object KotlinMethods {
   private const val CAPACITY = 100L
@@ -61,14 +66,15 @@ object KotlinMethods {
   }
 
   private fun Method.isKotlinMethodInvokingDefaultImpls(method: Method): Boolean {
-    // filter non kotlin classes
-    if (!method.containingClassFile.annotations.any { it.desc == "Lkotlin/Metadata;" }) {
-      return false
-    }
-
-    // Sanity check : if the method does not have bytecode
-    // this heuristic cannot run
-    if (instructions.isEmpty()) {
+    if (
+      // filter non kotlin classes
+      !method.containingClassFile.annotations.any { it.desc == "Lkotlin/Metadata;" }
+      // Sanity check: if the method does not have bytecode, or it's a constructor or class initializer,
+      // this heuristic cannot run
+      || instructions.isEmpty()
+      || method.isConstructor
+      || method.isClassInitializer
+      ) {
       return false
     }
 
@@ -93,24 +99,28 @@ object KotlinMethods {
       candidateOpcodes.add(currentInsnNode)
     } while (++i < instructions.size)
 
+    val isDefaultCallingDefaultOfParentInterface = isDefaultCallingDefaultOfParentInterface(method, candidateOpcodes) // checkcast this
 
     val expectedOpcodes = (
-      3 // aload this + invokestatic + (return or areturn)
-        + method.methodParameters.size // aload for each parameter
+      if (isDefaultCallingDefaultOfParentInterface)
+        4 // aload this + checkcast + invokestatic + (return or areturn)
+          + method.methodParameters.size // aload for each parameter
+          - 1 // Skip the first parameter, it's `this` or the interface (this)
+      else
+        3 // aload this + invokestatic + (return or areturn)
+          + method.methodParameters.size // aload for each parameter
       )
 
+    val startParameterIndexExcludingExcludingSelf = if (isDefaultCallingDefaultOfParentInterface)
+      2 // before: aload this + checkcast
+    else
+      1 // before: aload this
+
     if (candidateOpcodes.size != expectedOpcodes
-      || candidateOpcodes[0].opcode != Opcodes.ALOAD // aload this
-      || candidateOpcodes.slice(1..method.methodParameters.size).any() { it.opcode != Opcodes.ALOAD } // parameters
+      || candidateOpcodes[0].opcode != Opcodes.ALOAD // aload this, always a reference
+      || candidateOpcodes.slice(startParameterIndexExcludingExcludingSelf..method.methodParameters.size).any() { it.opcode !in loadOpCodes } // parameters
       || candidateOpcodes[candidateOpcodes.lastIndex - 1].opcode != Opcodes.INVOKESTATIC
-      || candidateOpcodes.last().opcode !in intArrayOf(
-        Opcodes.RETURN,
-        Opcodes.ARETURN,
-        Opcodes.DRETURN,
-        Opcodes.FRETURN,
-        Opcodes.IRETURN,
-        Opcodes.LRETURN
-      )
+      || candidateOpcodes.last().opcode !in returnOpCodes
     ) {
       return false
     }
@@ -137,11 +147,66 @@ object KotlinMethods {
     val isAParent = method.containingClassFile.interfaces.any {
       it == actualKotlinOwner
     }
-    if (!isAParent) {
+    @Suppress("RedundantIf") // for reading clarity
+    if (!isAParent && !isDefaultCallingDefaultOfParentInterface) {
       return false
     }
 
     // The method is a kotlin default method
     return true
   }
-}
+
+  /**
+   * Checks whether this method is forwarding to the DefaultImpls of the parent interface.
+   *
+   * In this case, Kotlin generates a default implementation `MyInterface$DefaultImpls`
+   * that forward the call to a `ParentInterface$DefaultImpls` of the parent interface.
+   * This forwarded is **static** and checks the type of the first parameter if
+   * it matches the parent interface.
+   *
+   * ```
+   * // access flags 0x9
+   * public static topInternal(Lmock/plugin/internal/defaultMethod/NoInternalTypeUsageInterface;)Linternal/defaultMethod/AnInternalType;
+   * @Lorg/jetbrains/annotations/Nullable;() // invisible
+   *   // annotable parameter count: 1 (invisible)
+   *   @Lorg/jetbrains/annotations/NotNull;() // invisible, parameter 0
+   *  L0
+   *   LINENUMBER 5 L0
+   *   ALOAD 0
+   *   CHECKCAST internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI
+   *   INVOKESTATIC internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI$DefaultImpls.topInternal (Linternal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI;)Linternal/defaultMethod/AnInternalType;
+   *  L1
+   *   LINENUMBER 11 L1
+   *   ARETURN
+   *  L2
+   *   LOCALVARIABLE $this Lmock/plugin/internal/defaultMethod/NoInternalTypeUsageInterface; L0 L2 0
+   *   MAXSTACK = 1
+   *   MAXLOCALS = 1
+   * ```
+   */
+  private fun isDefaultCallingDefaultOfParentInterface(method: Method, candidateOpcodes: MutableList<AbstractInsnNode>) =
+    method.isStatic
+      && candidateOpcodes[0].opcode == Opcodes.ALOAD
+      && (candidateOpcodes[0] as VarInsnNode).`var` == 0 // aload 0
+      && candidateOpcodes[1].opcode == Opcodes.CHECKCAST
+       && method.containingClassFile.innerClasses.any {
+         (candidateOpcodes[1] as TypeInsnNode).desc == it.outerName
+       }
+
+  private val returnOpCodes = intArrayOf(
+    Opcodes.RETURN,
+    Opcodes.ARETURN,
+    Opcodes.DRETURN,
+    Opcodes.FRETURN,
+    Opcodes.IRETURN,
+    Opcodes.LRETURN
+  )
+
+  private val loadOpCodes = intArrayOf(
+    Opcodes.ALOAD,
+    Opcodes.ILOAD,
+    Opcodes.DLOAD,
+    Opcodes.FLOAD,
+    Opcodes.ILOAD,
+  )
+}

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

@@ -7,6 +7,7 @@ package com.jetbrains.pluginverifier.verifiers.resolution
 import com.jetbrains.plugin.structure.classes.resolvers.FileOrigin
 import com.jetbrains.pluginverifier.results.location.ClassLocation
 import org.objectweb.asm.tree.AnnotationNode
+import org.objectweb.asm.tree.InnerClassNode
 
 interface ClassFile : ClassFileMember {
   override val location: ClassLocation
@@ -24,6 +25,7 @@ interface ClassFile : ClassFileMember {
   val javaVersion: Int
   val enclosingClassName: String?
 
+  val innerClasses: List<InnerClassNode>
   override val annotations: List<AnnotationNode>
 
   val isAbstract: Boolean

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

@@ -10,6 +10,7 @@ import com.jetbrains.pluginverifier.results.modifiers.Modifiers
 import com.jetbrains.pluginverifier.verifiers.getAccessType
 import org.objectweb.asm.Opcodes
 import org.objectweb.asm.tree.ClassNode
+import org.objectweb.asm.tree.InnerClassNode
 
 class ClassFileAsm(private val asmNode: ClassNode, override val classFileOrigin: FileOrigin) : ClassFile {
   override val location
@@ -66,6 +67,11 @@ class ClassFileAsm(private val asmNode: ClassNode, override val classFileOrigin:
       return asmNode.innerClasses.find { it.name == name }?.outerName
     }
 
+  override val innerClasses: List<InnerClassNode>
+    get() {
+      return asmNode.innerClasses
+    }
+
   override val annotations
     get() = asmNode.invisibleAnnotations.orEmpty() + asmNode.visibleAnnotations.orEmpty()
 

intellij-plugin-verifier/verifier-test/after-idea/src/main/kotlin/internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI.kt

@@ -7,4 +7,5 @@ interface InterfaceWithDefaultMethodUsingInternalAPI : TopInterfaceWithDefaultMe
   fun returningInternal() : AnInternalType? = null
   fun internalArgsReturningInternal(anInternalType: AnInternalType, s: String) : AnInternalType? = null
   fun internalArgsReturningVoid(anInternalType: AnInternalType, s: String) {}
+  fun internalArgsAndPrimitiveArgs(anInternalType: AnInternalType, i: Int, b: Boolean, ui: UInt, objectArray: Array<String>, intArray: Array<Int>) {}
 }

intellij-plugin-verifier/verifier-test/before-idea/src/main/kotlin/internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI.kt

@@ -6,4 +6,5 @@ interface InterfaceWithDefaultMethodUsingInternalAPI : TopInterfaceWithDefaultMe
   fun returningInternal() : AnInternalType? = null
   fun internalArgsReturningInternal(anInternalType: AnInternalType, s: String) : AnInternalType? = null
   fun internalArgsReturningVoid(anInternalType: AnInternalType, s: String) {}
+  fun internalArgsAndPrimitiveArgs(anInternalType: AnInternalType, i: Int, b: Boolean, ui: UInt, objectArray: Array<String>, intArray: Array<Int>) {}
 }

intellij-plugin-verifier/verifier-test/mock-plugin/src/main/kotlin/mock/plugin/internal/defaultMethod/NoInternalTypeUsageInterface.kt

@@ -0,0 +1,29 @@
+package mock.plugin.internal.defaultMethod
+
+import internal.defaultMethod.InterfaceWithDefaultMethodUsingInternalAPI
+
+// this interface when compiled with kotlin could have a nested DefaultImpls
+// that may refer to internal types, this should not raise warnings.
+interface NoInternalTypeUsageInterface : InterfaceWithDefaultMethodUsingInternalAPI {
+  // Kotlin should generate a default implementation for the interface
+  // default methods. E.g. a `NoInternalTypeUsageInterface$DefaultImpls` class
+  // with the default implementations of the interface methods. E.g. :
+  //
+  //  // access flags 0x9
+  //  public static topInternal(Lmock/plugin/internal/defaultMethod/NoInternalTypeUsageInterface;)Linternal/defaultMethod/AnInternalType;
+  //  @Lorg/jetbrains/annotations/Nullable;() // invisible
+  //    // annotable parameter count: 1 (invisible)
+  //    @Lorg/jetbrains/annotations/NotNull;() // invisible, parameter 0
+  //   L0
+  //    LINENUMBER 5 L0
+  //    ALOAD 0
+  //    CHECKCAST internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI
+  //    INVOKESTATIC internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI$DefaultImpls.topInternal (Linternal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI;)Linternal/defaultMethod/AnInternalType;
+  //   L1
+  //    LINENUMBER 11 L1
+  //    ARETURN
+  //   L2
+  //    LOCALVARIABLE $this Lmock/plugin/internal/defaultMethod/NoInternalTypeUsageInterface; L0 L2 0
+  //    MAXSTACK = 1
+  //    MAXLOCALS = 1
+}