From 8a8e884f236a2779eb10bdf8693935374b250ac8 Mon Sep 17 00:00:00 2001 From: Jonathan Gerrish Date: Mon, 15 Feb 2021 10:43:28 -0800 Subject: [PATCH] Additional jdeps cases in Kotlin * Track function parameters dependencies * Track Java static method dependencies * Track extension property dependencies * When tracking super types collect all super types in hierarchy. --- .../kotlin/plugin/jdeps/JdepsGenExtension.kt | 14 +- .../tasks/jvm/KotlinBuilderJvmJdepsTest.kt | 203 ++++++++++++++++++ 2 files changed, 216 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt index 4b19cfab5..d95064336 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt @@ -19,6 +19,8 @@ import org.jetbrains.kotlin.descriptors.PropertyDescriptor import org.jetbrains.kotlin.descriptors.SourceElement import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor import org.jetbrains.kotlin.extensions.StorageComponentContainerContributor +import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor +import org.jetbrains.kotlin.load.java.descriptors.JavaMethodDescriptor import org.jetbrains.kotlin.load.java.descriptors.JavaPropertyDescriptor import org.jetbrains.kotlin.load.java.sources.JavaSourceElement import org.jetbrains.kotlin.load.java.structure.impl.classFiles.BinaryJavaClass @@ -125,8 +127,14 @@ class JdepsGenExtension( context: CallCheckerContext ) { when (val resultingDescriptor = resolvedCall.resultingDescriptor) { + is JavaMethodDescriptor -> { + getClassCanonicalPath((resultingDescriptor.containingDeclaration as ClassDescriptor).typeConstructor)?.let { explicitClassesCanonicalPaths.add(it) } + } is FunctionDescriptor -> { resultingDescriptor.returnType?.let { addImplicitDep(it) } + resultingDescriptor.valueParameters.forEach { valueParameter -> + addImplicitDep(valueParameter.type) + } val virtualFileClass = resultingDescriptor.getContainingKotlinJvmBinaryClass() as? VirtualFileKotlinClass ?: return explicitClassesCanonicalPaths.add(virtualFileClass.file.path) @@ -143,6 +151,10 @@ class JdepsGenExtension( is PropertyImportedFromObject -> { collectTypeReferences((resolvedCall.resultingDescriptor as PropertyImportedFromObject).containingObject.defaultType) } + is PropertyDescriptor -> { + val virtualFileClass = (resultingDescriptor).getContainingKotlinJvmBinaryClass() as? VirtualFileKotlinClass ?: return + explicitClassesCanonicalPaths.add(virtualFileClass.file.path) + } else -> return } } @@ -194,7 +206,7 @@ class JdepsGenExtension( addExplicitDep(kotlinType) if (collectSuperTypes) { - kotlinType.constructor.supertypes.forEach { + kotlinType.supertypes().forEach { addImplicitDep(it) } } diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinBuilderJvmJdepsTest.kt b/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinBuilderJvmJdepsTest.kt index f3f16f3f8..293d87ef4 100644 --- a/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinBuilderJvmJdepsTest.kt +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinBuilderJvmJdepsTest.kt @@ -113,6 +113,42 @@ class KotlinBuilderJvmJdepsTest { assertIncomplete(jdeps).isEmpty() } + @Test + fun `java class static reference`() { + + val dependentTarget = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("JavaClass.java", + """ + package something; + + class JavaClass { + public static boolean staticMethod() { return true; } + } + """) + c.outputJar() + c.compileJava() + }) + + val dependingTarget = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("AClass.kt", + """ + package something + + val result = JavaClass.staticMethod() + """) + c.outputJar() + c.compileKotlin() + c.outputJdeps() + c.addDirectDependencies(dependentTarget) + }) + val jdeps = depsProto(dependingTarget) + + assertExplicit(jdeps).containsExactly(dependentTarget.singleCompileJar()) + assertImplicit(jdeps).isEmpty() + assertUnused(jdeps).isEmpty() + assertIncomplete(jdeps).isEmpty() + } + @Test fun `java constant reference`() { @@ -362,6 +398,42 @@ class KotlinBuilderJvmJdepsTest { assertIncomplete(jdeps).isEmpty() } + @Test + fun `kotlin extension property reference`() { + val dependentTarget = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("AClass.kt", + """ + package something + + val String.doubleLength + get() = length * 2 + """) + c.outputJar() + c.compileKotlin() + }) + + val dependingTarget = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("HasPropertyDependency.kt", + """ + package something + + val property2 = "Hello".doubleLength + """) + c.outputJar() + c.compileKotlin() + c.addDirectDependencies(dependentTarget) + c.outputJdeps() + }) + val jdeps = depsProto(dependingTarget) + + assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) + + assertExplicit(jdeps).containsExactly(dependentTarget.singleCompileJar()) + assertImplicit(jdeps).isEmpty() + assertUnused(jdeps).isEmpty() + assertIncomplete(jdeps).isEmpty() + } + @Test fun `kotlin property definition`() { val dependentTarget = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> @@ -514,6 +586,40 @@ class KotlinBuilderJvmJdepsTest { assertIncomplete(jdeps).isEmpty() } + @Test + fun `object inlined constant dependency recorded`() { + val dependentTarget = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("HasConstants.kt", + """ + package dependency + object HasConstants { + const val CONSTANT_VAL = 42 + } + """) + c.outputJar() + c.compileKotlin() + }) + + val dependingTarget = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("HasPropertyDependency.kt", + """ + package something + import dependency.HasConstants.CONSTANT_VAL + val property2 = CONSTANT_VAL + """) + c.outputJar() + c.compileKotlin() + c.addDirectDependencies(dependentTarget) + c.outputJdeps() + }) + val jdeps = depsProto(dependingTarget) + + assertExplicit(jdeps).containsExactly(dependentTarget.singleCompileJar()) + assertImplicit(jdeps).isEmpty() + assertUnused(jdeps).isEmpty() + assertIncomplete(jdeps).isEmpty() + } + @Test fun `companion object inlined constant dependency recorded`() { val dependentTarget = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> @@ -734,6 +840,58 @@ class KotlinBuilderJvmJdepsTest { assertIncomplete(jdeps).isEmpty() } + @Test + fun `class declaration all super class references should be an implicit dependency`() { + val implicitSuperClassDep = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("Base.kt", + """ + package something + + open class Base + """) + c.outputJar() + c.compileKotlin() + }) + + val explicitSuperClassDep = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("Derived.kt", + """ + package something + + open class Derived : Base() + """) + c.addSource("Derived2.kt", + """ + package something + + open class Derived2 : Derived() + """) + c.outputJar() + c.compileKotlin() + c.addDirectDependencies(implicitSuperClassDep) + }) + + val dependingTarget = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("DependingClass.kt", + """ + package something + + abstract class DependingClass : Derived2() + """) + c.outputJar() + c.compileKotlin() + c.addDirectDependencies(explicitSuperClassDep) + c.addTransitiveDependencies(implicitSuperClassDep) + c.outputJdeps() + }) + val jdeps = depsProto(dependingTarget) + + assertExplicit(jdeps).containsExactly(explicitSuperClassDep.singleCompileJar()) + assertImplicit(jdeps).containsExactly(implicitSuperClassDep.singleCompileJar()) + assertUnused(jdeps).isEmpty() + assertIncomplete(jdeps).isEmpty() + } + @Test fun `generic type as constructor parameter`() { val implicitSuperClassDep = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> @@ -924,6 +1082,51 @@ class KotlinBuilderJvmJdepsTest { assertImplicit(jdeps).doesNotContain(depWithReturnTypesSuperType) } + @Test + fun `constructor parameters are required implicit dependencies`() { + val fooDep = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("FooClass.kt", + """ + package something + + class FooClass + """) + c.outputJar() + c.compileKotlin() + }) + val barDep = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("BarClass.kt", + """ + package something + + class BarClass(private val foo: FooClass = FooClass()) { } + """) + c.outputJar() + c.compileKotlin() + c.addDirectDependencies(fooDep) + }) + + val dependingTarget = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource("ReferencesClassWithSuperClass.kt", + """ + package something + + class Dummy { + val result = BarClass() + } + """) + c.outputJar() + c.compileKotlin() + c.addDirectDependencies(barDep) + c.addTransitiveDependencies(fooDep) + c.outputJdeps() + }) + val jdeps = depsProto(dependingTarget) + + assertExplicit(jdeps).contains(barDep.singleCompileJar()) + assertImplicit(jdeps).contains(fooDep.singleCompileJar()) + } + @Test fun `function call return type type parameter should not be a dependency`() { val depWithTypeParameter = ctx.runCompileTask(Consumer { c: KotlinJvmTestBuilder.TaskBuilder ->