diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/method/KotlinMethods.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/method/KotlinMethods.kt index 782fb28cf1..d3f5053b85 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/method/KotlinMethods.kt +++ b/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 } -} \ No newline at end of file + + /** + * 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) = + 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, + ) +} diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/ClassFile.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/ClassFile.kt index c74bb1a676..30de53ab1f 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/ClassFile.kt +++ b/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 override val annotations: List val isAbstract: Boolean diff --git a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/ClassFileAsm.kt b/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/ClassFileAsm.kt index 729db8c6aa..374409cfa1 100644 --- a/intellij-plugin-verifier/verifier-core/src/main/java/com/jetbrains/pluginverifier/verifiers/resolution/ClassFileAsm.kt +++ b/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 + get() { + return asmNode.innerClasses + } + override val annotations get() = asmNode.invisibleAnnotations.orEmpty() + asmNode.visibleAnnotations.orEmpty() diff --git a/intellij-plugin-verifier/verifier-test/after-idea/src/main/kotlin/internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI.kt b/intellij-plugin-verifier/verifier-test/after-idea/src/main/kotlin/internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI.kt index 456d075f4c..35a0186744 100644 --- a/intellij-plugin-verifier/verifier-test/after-idea/src/main/kotlin/internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI.kt +++ b/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, intArray: Array) {} } \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-test/before-idea/src/main/kotlin/internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI.kt b/intellij-plugin-verifier/verifier-test/before-idea/src/main/kotlin/internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI.kt index 486a45cf6f..87c828a859 100644 --- a/intellij-plugin-verifier/verifier-test/before-idea/src/main/kotlin/internal/defaultMethod/InterfaceWithDefaultMethodUsingInternalAPI.kt +++ b/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, intArray: Array) {} } \ No newline at end of file diff --git a/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/kotlin/mock/plugin/internal/defaultMethod/NoInternalTypeUsageInterface.kt b/intellij-plugin-verifier/verifier-test/mock-plugin/src/main/kotlin/mock/plugin/internal/defaultMethod/NoInternalTypeUsageInterface.kt new file mode 100644 index 0000000000..660581fe36 --- /dev/null +++ b/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 +} \ No newline at end of file