-
Notifications
You must be signed in to change notification settings - Fork 38
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Report usages of Kotlin classes with
internal
visibility modifier (#…
…1101) * Move common internal API usage detection to parent class * Add dependency on kotlinx-metadata-jvm * Add wrapper that detects Kotlin internal visibility * Use kotlinx-metadata-jvm in tests * Detect `internal` usages in methods, classes and fields * Open the ASM node to use in Kotlin language-specific features
- Loading branch information
Showing
19 changed files
with
1,066 additions
and
109 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
30 changes: 30 additions & 0 deletions
30
...ructure-classes/src/main/java/com/jetbrains/plugin/structure/classes/utils/KtClassNode.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package com.jetbrains.plugin.structure.classes.utils | ||
|
||
import kotlinx.metadata.KmClass | ||
import kotlinx.metadata.Visibility | ||
import kotlinx.metadata.jvm.KotlinClassMetadata | ||
import kotlinx.metadata.jvm.signature | ||
import kotlinx.metadata.visibility | ||
import org.objectweb.asm.tree.ClassNode | ||
|
||
class KtClassNode(private val classNode: ClassNode, private val metadata: KotlinClassMetadata.Class) { | ||
|
||
val isInternal: Boolean | ||
get() = cls.visibility == Visibility.INTERNAL | ||
|
||
val name: String | ||
get() = classNode.name | ||
|
||
fun isInternalField(fieldName: String): Boolean { | ||
return cls.properties.any { it.name == fieldName && it.visibility == Visibility.INTERNAL } | ||
} | ||
|
||
fun isInternalFunction(functionName: String, jvmDescriptor: String): Boolean { | ||
return cls.functions.any { it.name == functionName | ||
&& it.signature?.descriptor == jvmDescriptor | ||
&& it.visibility == Visibility.INTERNAL } | ||
} | ||
|
||
private val cls: KmClass | ||
get() = metadata.kmClass | ||
} |
79 changes: 79 additions & 0 deletions
79
...ure-classes/src/main/java/com/jetbrains/plugin/structure/classes/utils/KtClassResolver.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
package com.jetbrains.plugin.structure.classes.utils | ||
|
||
import kotlinx.metadata.jvm.KotlinClassMetadata | ||
import kotlinx.metadata.jvm.Metadata | ||
import org.objectweb.asm.tree.AnnotationNode | ||
import org.objectweb.asm.tree.ClassNode | ||
|
||
internal const val KOTLIN_METADATA_ANNOTATION_DESC = "Lkotlin/Metadata;" | ||
|
||
private typealias Signature = String | ||
|
||
class KtClassResolver { | ||
private val cache = HashMap<Signature, KtClassNode>() | ||
|
||
operator fun get(classNode: ClassNode): KtClassNode? { | ||
val signature: Signature = classNode.signature ?: return classNode.ktClassNode | ||
return if (signature in cache) { | ||
cache[signature] | ||
} else { | ||
classNode.ktClassNode?.also { | ||
cache[signature] = it | ||
} | ||
} | ||
} | ||
|
||
private val ClassNode.ktClassNode: KtClassNode? | ||
get() = findMetadataAnnotation(this) | ||
?.let { annotation -> getKtClassNode(this, annotation) } | ||
|
||
private fun getKtClassNode(classNode: ClassNode, metadataAnnotation: Metadata): KtClassNode? { | ||
val metadata = KotlinClassMetadata.readStrict(metadataAnnotation) | ||
return if (metadata is KotlinClassMetadata.Class) { | ||
KtClassNode(classNode, metadata) | ||
} else { | ||
null | ||
} | ||
} | ||
|
||
private fun findMetadataAnnotation(classNode: ClassNode): Metadata? { | ||
return classNode.allAnnotations | ||
.firstOrNull { it.desc == KOTLIN_METADATA_ANNOTATION_DESC } | ||
?.toMetadata() | ||
} | ||
|
||
private fun AnnotationNode.toMetadata() = Metadata( | ||
kind = int("k"), | ||
metadataVersion = ints("mv"), | ||
data1 = strings("d1"), | ||
data2 = strings("d2")) | ||
|
||
private fun AnnotationNode.int(key: String) = get<Int>(key) | ||
private fun AnnotationNode.ints(key: String) = get<List<Int>?>(key)?.toIntArray() | ||
private fun AnnotationNode.strings(key: String) = get<List<String>?>(key)?.toTypedArray() | ||
|
||
private inline fun <reified T> AnnotationNode.get(key: String): T? { | ||
return getAnnotationValue(key)?.let { value -> | ||
if (value is T) value else null | ||
} | ||
} | ||
|
||
private fun AnnotationNode.getAnnotationValue(key: String): Any? { | ||
val values = values ?: return null | ||
for (i in 0 until values.size step 2) { | ||
if (values[i] == key) { | ||
return values[i + 1] | ||
} | ||
} | ||
return null | ||
} | ||
|
||
companion object { | ||
fun hasKotlinMetadataAnnotation(classNode: ClassNode): Boolean { | ||
return classNode.allAnnotations.any { it.desc == KOTLIN_METADATA_ANNOTATION_DESC } | ||
} | ||
} | ||
} | ||
|
||
private val ClassNode.allAnnotations: List<AnnotationNode> | ||
get() = (visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty()) |
44 changes: 44 additions & 0 deletions
44
...tests/src/test/kotlin/com/jetbrains/plugin/structure/classes/utils/KtClassResolverTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package com.jetbrains.plugin.structure.classes.utils | ||
|
||
import com.jetbrains.plugin.structure.classes.utils.AsmUtil.readClassNode | ||
import org.junit.Assert.* | ||
import org.junit.Before | ||
import org.junit.Test | ||
|
||
class KtClassResolverTest { | ||
private lateinit var ktClassNode: KtClassNode | ||
|
||
@Before | ||
fun setUp() { | ||
val classResolver = KtClassResolver() | ||
|
||
val mockInternalClass = KtClassResolverTest::class.java.getResourceAsStream("MockInternalClass.class").use { | ||
if (it == null) fail("Cannot load resource") | ||
val classNode = readClassNode("com.jetbrains.plugin.structure.classes.utils.MockInternalClass", it!!) | ||
classNode | ||
} | ||
|
||
val ktClassNode = classResolver[mockInternalClass] | ||
assertNotNull(ktClassNode) | ||
ktClassNode!! | ||
assertEquals("com/jetbrains/plugin/structure/classes/utils/MockInternalClass", ktClassNode.name) | ||
this.ktClassNode = ktClassNode | ||
} | ||
|
||
@Test | ||
fun `internal class is resolved as internal`() { | ||
assertEquals(true, ktClassNode.isInternal) | ||
} | ||
|
||
@Test | ||
fun `internal class fields are resolved as internal`() { | ||
assertTrue(ktClassNode.isInternalField("internalField")) | ||
assertFalse(ktClassNode.isInternalField("privateField")) | ||
} | ||
|
||
@Test | ||
fun `internal class functions are resolved as internal`() { | ||
assertTrue(ktClassNode.isInternalFunction("internalMethod", "(Ljava/lang/String;I)Ljava/lang/String;")) | ||
assertFalse(ktClassNode.isInternalFunction("internalMethod", "(Ljava/lang/String;)Ljava/lang/String;")) | ||
} | ||
} |
11 changes: 11 additions & 0 deletions
11
...e/tests/src/test/kotlin/com/jetbrains/plugin/structure/classes/utils/MockInternalClass.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package com.jetbrains.plugin.structure.classes.utils | ||
|
||
@Suppress("unused") | ||
internal class MockInternalClass { | ||
private val privateField = "private" | ||
|
||
internal val internalField = "internal" | ||
|
||
internal fun internalMethod(s: String, i: Int) = "internal" | ||
fun internalMethod(s: String) = "internal" | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
42 changes: 42 additions & 0 deletions
42
...n/java/com/jetbrains/pluginverifier/usages/internal/AnnotatedInternalApiUsageRegistrar.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package com.jetbrains.pluginverifier.usages.internal | ||
|
||
import com.jetbrains.pluginverifier.PluginVerificationDescriptor | ||
import com.jetbrains.pluginverifier.results.location.ClassLocation | ||
import com.jetbrains.pluginverifier.results.location.FieldLocation | ||
import com.jetbrains.pluginverifier.results.location.Location | ||
import com.jetbrains.pluginverifier.results.location.MethodLocation | ||
import com.jetbrains.pluginverifier.results.reference.ClassReference | ||
import com.jetbrains.pluginverifier.results.reference.FieldReference | ||
import com.jetbrains.pluginverifier.results.reference.MethodReference | ||
import com.jetbrains.pluginverifier.verifiers.PluginVerificationContext | ||
|
||
class AnnotatedInternalApiUsageRegistrar(private val verificationContext: PluginVerificationContext) : InternalUsageRegistrar { | ||
override fun registerClass(classReference: ClassReference, apiElement: ClassLocation, usageLocation: Location) { | ||
register(InternalClassUsage(classReference, apiElement, usageLocation)) | ||
} | ||
|
||
override fun registerMethod( | ||
methodReference: MethodReference, | ||
apiElement: MethodLocation, | ||
usageLocation: MethodLocation | ||
) { | ||
register(InternalMethodUsage(methodReference, apiElement, usageLocation)) | ||
} | ||
|
||
override fun registerField(fieldReference: FieldReference, apiElement: FieldLocation, usageLocation: MethodLocation) { | ||
register(InternalFieldUsage(fieldReference, apiElement, usageLocation)) | ||
} | ||
|
||
private fun register(usage: InternalApiUsage) { | ||
// MP-3421 Plugin Verifier must report compatibility errors for usages of internal FUS APIs | ||
if (usage.apiElement.containingClass.packageName.startsWith("com/intellij/internal/statistic") | ||
&& verificationContext.idePlugin.vendor?.contains("JetBrains", true) != true | ||
&& verificationContext.verificationDescriptor is PluginVerificationDescriptor.IDE | ||
&& verificationContext.verificationDescriptor.ideVersion.baselineVersion >= 211 | ||
) { | ||
verificationContext.registerProblem(InternalFusApiUsageCompatibilityProblem(usage)) | ||
} else { | ||
verificationContext.registerInternalApiUsage(usage) | ||
} | ||
} | ||
} |
110 changes: 110 additions & 0 deletions
110
...c/main/java/com/jetbrains/pluginverifier/usages/internal/BaseInternalApiUsageProcessor.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
package com.jetbrains.pluginverifier.usages.internal | ||
|
||
import com.jetbrains.pluginverifier.results.instruction.Instruction | ||
import com.jetbrains.pluginverifier.results.location.Location | ||
import com.jetbrains.pluginverifier.results.location.toReference | ||
import com.jetbrains.pluginverifier.results.problems.CompatibilityProblem | ||
import com.jetbrains.pluginverifier.results.reference.ClassReference | ||
import com.jetbrains.pluginverifier.results.reference.FieldReference | ||
import com.jetbrains.pluginverifier.results.reference.MethodReference | ||
import com.jetbrains.pluginverifier.usages.ApiUsageProcessor | ||
import com.jetbrains.pluginverifier.usages.util.isFromVerifiedPlugin | ||
import com.jetbrains.pluginverifier.verifiers.ProblemRegistrar | ||
import com.jetbrains.pluginverifier.verifiers.VerificationContext | ||
import com.jetbrains.pluginverifier.verifiers.resolution.ClassFile | ||
import com.jetbrains.pluginverifier.verifiers.resolution.ClassFileMember | ||
import com.jetbrains.pluginverifier.verifiers.resolution.ClassUsageType | ||
import com.jetbrains.pluginverifier.verifiers.resolution.Field | ||
import com.jetbrains.pluginverifier.verifiers.resolution.Method | ||
import com.jetbrains.pluginverifier.verifiers.resolution.MethodResolver | ||
import com.jetbrains.pluginverifier.warnings.CompatibilityWarning | ||
import com.jetbrains.pluginverifier.warnings.WarningRegistrar | ||
import org.objectweb.asm.tree.AbstractInsnNode | ||
|
||
abstract class BaseInternalApiUsageProcessor( | ||
private val internalUsageRegistrar: InternalUsageRegistrar | ||
) : | ||
ApiUsageProcessor { | ||
|
||
override fun processClassReference( | ||
classReference: ClassReference, | ||
resolvedClass: ClassFile, | ||
context: VerificationContext, | ||
referrer: ClassFileMember, | ||
classUsageType: ClassUsageType | ||
) { | ||
val usageLocation = referrer.location | ||
if (isInternal(resolvedClass, context, usageLocation) && context.isFromVerifiedPlugin(referrer)) { | ||
internalUsageRegistrar.registerClass(classReference, resolvedClass.location, usageLocation) | ||
} | ||
} | ||
|
||
override fun processMethodInvocation( | ||
methodReference: MethodReference, | ||
resolvedMethod: Method, | ||
instructionNode: AbstractInsnNode, | ||
callerMethod: Method, | ||
context: VerificationContext | ||
) { | ||
val usageLocation = callerMethod.location | ||
if (isInternal(resolvedMethod, context, usageLocation)) { | ||
// Check if the method is an override, and if so check top declaration | ||
val canBeOverridden = !resolvedMethod.isStatic && !resolvedMethod.isPrivate | ||
&& resolvedMethod.name != "<init>" && resolvedMethod.name != "<clinit>" | ||
|
||
// Taken from MethodOverridingVerifier | ||
val overriddenMethod = if (canBeOverridden) { | ||
MethodResolver().resolveMethod( | ||
ClassFileWithNoMethodsWrapper(resolvedMethod.containingClassFile), | ||
resolvedMethod.location.toReference(), | ||
if (resolvedMethod.containingClassFile.isInterface) Instruction.INVOKE_INTERFACE else Instruction.INVOKE_VIRTUAL, | ||
resolvedMethod, | ||
VerificationContextWithSilentProblemRegistrar(context) | ||
) | ||
} else { | ||
null | ||
} | ||
|
||
if (overriddenMethod == null || isInternal(overriddenMethod, context, usageLocation)) { | ||
internalUsageRegistrar.registerMethod(methodReference, resolvedMethod.location, usageLocation) | ||
} | ||
} | ||
} | ||
|
||
private class ClassFileWithNoMethodsWrapper( | ||
private val classFile: ClassFile | ||
) : ClassFile by classFile { | ||
override val methods: Sequence<Method> get() = emptySequence() | ||
} | ||
|
||
private class VerificationContextWithSilentProblemRegistrar( | ||
private val delegate: VerificationContext | ||
) : VerificationContext by delegate { | ||
override val problemRegistrar: ProblemRegistrar = object : ProblemRegistrar { | ||
override fun registerProblem(problem: CompatibilityProblem) = Unit | ||
} | ||
|
||
override val warningRegistrar: WarningRegistrar = object : WarningRegistrar { | ||
override fun registerCompatibilityWarning(warning: CompatibilityWarning) = Unit | ||
} | ||
} | ||
|
||
override fun processFieldAccess( | ||
fieldReference: FieldReference, | ||
resolvedField: Field, | ||
context: VerificationContext, | ||
callerMethod: Method | ||
) { | ||
val usageLocation = callerMethod.location | ||
if (isInternal(resolvedField, context, usageLocation)) { | ||
internalUsageRegistrar.registerField(fieldReference, resolvedField.location, usageLocation) | ||
} | ||
} | ||
|
||
protected abstract fun isInternal( | ||
resolvedMember: ClassFileMember, | ||
context: VerificationContext, | ||
usageLocation: Location | ||
): Boolean | ||
|
||
} |
Oops, something went wrong.