From 72b1641d5c91981f4c99bb6b98ad61340c2ed623 Mon Sep 17 00:00:00 2001 From: Steve Cosenza Date: Tue, 20 Feb 2024 10:56:30 -0800 Subject: [PATCH 1/9] Improve Kotlin jdeps logic to find more explicit and implicit dependencies (#17) # Problem Kotlin jdeps do not collect many direct and indirect dependencies that are required for compilation. These missing jdep found dependencies were discovered as Airbnb tests a "jdeps compile-time pruning" optimization which provides significant IntelliJ indexing, UI, and compilation performance benefits in our fork of the [Bazel IntelliJ Plugin](https://github.com/bazelbuild/intellij). # Solution As a followup to https://github.com/bazelbuild/rules_kotlin/pull/1045, write test cases to repro missing dependencies found in Airbnb's monorepo, and then fix the underlying issues. In addition to these fixes helping our "jdeps compile-time pruning" optimization, these fixes should also help when the normal Bazel IntelliJ Plugin is used, as it also consults jdeps when deciding what jars to import into IntelliJ. # Test Plan: * Wrote Kotlin and Java test cases to repro missing dependencies found in the Airbnb monorepo * Updated Airbnb's minimal rules_kotlin fork to use the changes in this PR, which has been successfully running CI for over a week. --- .bazelrc | 2 +- .../kotlin/plugin/jdeps/JdepsGenExtension.kt | 174 +++++++---- .../io/bazel/kotlin/builder/tasks/BUILD.bazel | 14 +- .../tasks/jvm/KotlinBuilderJvmJdepsTest.kt | 270 +++++++++++++++++- .../testFixtures/AlwaysClosedStrategy.java | 5 + .../builder/tasks/testFixtures/BusConfig.java | 4 + .../CircuitBreakerStrategies.java | 8 + .../testFixtures/CircuitBreakerStrategy.java | 4 + .../builder/tasks/testFixtures/Client1.java | 4 + .../tasks/testFixtures/ConsumerConfig.java | 7 + .../tasks/testFixtures/GlobalTracer.java | 7 + .../testFixtures/JavaBaseWithTypeParam.java | 2 + .../builder/tasks/testFixtures/JavaClass.java | 4 + .../tasks/testFixtures/JavaException.java | 4 + .../tasks/testFixtures/JavaWidgetFactory.java | 7 + .../tasks/testFixtures/MyHttpClient.java | 7 + .../testFixtures/MyHttpClientBuilder.java | 16 ++ .../builder/tasks/testFixtures/MySpan.java | 8 + .../builder/tasks/testFixtures/Span.java | 4 + .../builder/tasks/testFixtures/Tracer.java | 5 + .../builder/tasks/testFixtures/Widget.java | 4 + .../tasks/testFixtures2/BaseException.java | 4 + .../builder/tasks/testFixtures2/Client2.java | 4 + .../tasks/testFixtures2/MyErrorReporter.java | 5 + .../tasks/testFixtures2/TestClient2.java | 4 + 25 files changed, 520 insertions(+), 57 deletions(-) create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/AlwaysClosedStrategy.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/BusConfig.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/CircuitBreakerStrategies.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/CircuitBreakerStrategy.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Client1.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/ConsumerConfig.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/GlobalTracer.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaException.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaWidgetFactory.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MyHttpClient.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MyHttpClientBuilder.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MySpan.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Span.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Tracer.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Widget.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/BaseException.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/Client2.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/MyErrorReporter.java create mode 100644 src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/TestClient2.java diff --git a/.bazelrc b/.bazelrc index c1d9ef968..e856fcbcb 100644 --- a/.bazelrc +++ b/.bazelrc @@ -3,4 +3,4 @@ common --enable_bzlmod=true build --strategy=KotlinCompile=worker build --test_output=all build --verbose_failures - +common --jvmopt=-Djava.security.manager=allow 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 b6343d3ee..3f672b468 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt @@ -11,18 +11,20 @@ import org.jetbrains.kotlin.container.useInstance import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithSource +import org.jetbrains.kotlin.descriptors.ValueDescriptor +import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.descriptors.ModuleDescriptor -import org.jetbrains.kotlin.descriptors.ParameterDescriptor 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.JavaMethodDescriptor -import org.jetbrains.kotlin.load.java.descriptors.JavaPropertyDescriptor +import org.jetbrains.kotlin.js.translate.callTranslator.getReturnType +import org.jetbrains.kotlin.load.java.descriptors.JavaClassConstructorDescriptor import org.jetbrains.kotlin.load.java.sources.JavaSourceElement import org.jetbrains.kotlin.load.java.structure.impl.classFiles.BinaryJavaClass import org.jetbrains.kotlin.load.java.structure.impl.classFiles.BinaryJavaField +import org.jetbrains.kotlin.load.kotlin.JvmPackagePartSource import org.jetbrains.kotlin.load.kotlin.KotlinJvmBinarySourceElement import org.jetbrains.kotlin.load.kotlin.VirtualFileKotlinClass import org.jetbrains.kotlin.load.kotlin.getContainingKotlinJvmBinaryClass @@ -30,17 +32,22 @@ import org.jetbrains.kotlin.platform.TargetPlatform import org.jetbrains.kotlin.psi.KtDeclaration import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.resolve.BindingTrace -import org.jetbrains.kotlin.resolve.FunctionImportedFromObject -import org.jetbrains.kotlin.resolve.PropertyImportedFromObject +import org.jetbrains.kotlin.resolve.ImportedFromObjectCallableDescriptor import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext +import org.jetbrains.kotlin.resolve.calls.model.ExpressionKotlinCallArgument import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall -import org.jetbrains.kotlin.resolve.calls.util.FakeCallableDescriptorForObject +import org.jetbrains.kotlin.resolve.calls.tower.NewAbstractResolvedCall +import org.jetbrains.kotlin.resolve.calls.tower.PSIKotlinCallArgument import org.jetbrains.kotlin.resolve.checkers.DeclarationChecker import org.jetbrains.kotlin.resolve.checkers.DeclarationCheckerContext +import org.jetbrains.kotlin.resolve.descriptorUtil.getImportableDescriptor import org.jetbrains.kotlin.resolve.jvm.extensions.AnalysisHandlerExtension +import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedTypeAliasDescriptor +import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.TypeConstructor +import org.jetbrains.kotlin.types.getAbbreviation import org.jetbrains.kotlin.types.typeUtil.supertypes import java.io.BufferedOutputStream import java.io.File @@ -77,7 +84,21 @@ class JdepsGenExtension( * @return the path corresponding to the JAR where this class was loaded from, or null. */ fun getClassCanonicalPath(descriptor: DeclarationDescriptorWithSource): String? { - return when (val sourceElement: SourceElement = descriptor.source) { + // Get the descriptor's source element which may be a type alias + val sourceElement = if (descriptor.source != SourceElement.NO_SOURCE) { + descriptor.source + } else { + when(val declarationDescriptor = descriptor.getImportableDescriptor()) { + is DeserializedTypeAliasDescriptor -> { + declarationDescriptor.containerSource + } + else -> { + null + } + } + } + + return when (sourceElement) { is JavaSourceElement -> if (sourceElement.javaElement is BinaryJavaClass) { (sourceElement.javaElement as BinaryJavaClass).virtualFile.canonicalPath @@ -94,11 +115,16 @@ class JdepsGenExtension( } is KotlinJvmBinarySourceElement -> (sourceElement.binaryClass as VirtualFileKotlinClass).file.canonicalPath - else -> null + is JvmPackagePartSource -> { // This case is needed to collect type aliases + ((sourceElement.knownJvmBinaryClass) as VirtualFileKotlinClass).file.canonicalPath + } + else -> { + null + } } } - fun getClassCanonicalPath(typeConstructor: TypeConstructor): String? { + private fun getClassCanonicalPath(typeConstructor: TypeConstructor): String? { return (typeConstructor.declarationDescriptor as? DeclarationDescriptorWithSource)?.let { getClassCanonicalPath( it, @@ -130,54 +156,87 @@ class JdepsGenExtension( reportOn: PsiElement, context: CallCheckerContext, ) { - when (val resultingDescriptor = resolvedCall.resultingDescriptor) { - is FunctionImportedFromObject -> { - collectTypeReferences(resultingDescriptor.containingObject.defaultType) - } - is PropertyImportedFromObject -> { - collectTypeReferences(resultingDescriptor.containingObject.defaultType) + // First collect type from the Resolved Call + collectExplicitTypes(resolvedCall) + + resolvedCall.valueArguments.keys.forEach { valueArgument -> + collectTypeReferences(valueArgument.type, isExplicit = false) + } + + // And then collect types from any ResultingDescriptor + val resultingDescriptor = resolvedCall.resultingDescriptor + collectExplicitTypes(resultingDescriptor) + + val isExplicitReturnType = resultingDescriptor is JavaClassConstructorDescriptor + collectTypeReferences(resolvedCall.getReturnType(), isExplicit = isExplicitReturnType, collectTypeArguments = false) + resultingDescriptor.returnType?.let { + collectTypeReferences(it, isExplicit = isExplicitReturnType, collectTypeArguments = false) + } + + resultingDescriptor.valueParameters.forEach { valueParameter -> + collectTypeReferences(valueParameter.type, isExplicit = false) + } + + // Finally, collect types that depend on the type of the ResultingDescriptor and note that + // these descriptors may be composed of multiple classes that we need to extract types from + if (resultingDescriptor is DeclarationDescriptor) { + val containingDeclaration = resultingDescriptor.containingDeclaration + if (containingDeclaration is ClassDescriptor) { + collectTypeReferences(containingDeclaration.defaultType) } - is JavaMethodDescriptor -> { - getClassCanonicalPath( - (resultingDescriptor.containingDeclaration as ClassDescriptor).typeConstructor, - )?.let { explicitClassesCanonicalPaths.add(it) } + + if (resultingDescriptor is PropertyDescriptor) { + (resultingDescriptor.getter + ?.correspondingProperty as? SyntheticJavaPropertyDescriptor) + ?.let { syntheticJavaPropertyDescriptor -> + collectTypeReferences(syntheticJavaPropertyDescriptor.type, isExplicit = false) + + val functionDescriptor = syntheticJavaPropertyDescriptor.getMethod + functionDescriptor.dispatchReceiverParameter?.type?.let { dispatchReceiverType -> + collectTypeReferences(dispatchReceiverType, isExplicit = false) + } + } } - is FunctionDescriptor -> { - resultingDescriptor.returnType?.let { - collectTypeReferences(it, isExplicit = false, collectTypeArguments = false) - } - resultingDescriptor.valueParameters.forEach { valueParameter -> - collectTypeReferences(valueParameter.type, isExplicit = false) + } + + if (resultingDescriptor is ImportedFromObjectCallableDescriptor<*>) { + collectTypeReferences(resultingDescriptor.containingObject.defaultType, isExplicit = false) + } + + if (resultingDescriptor is ValueDescriptor) { + collectTypeReferences(resultingDescriptor.type, isExplicit = false) + } + } + + private fun collectExplicitTypes(resultingDescriptor: CallableDescriptor) { + val kotlinJvmBinaryClass = resultingDescriptor.getContainingKotlinJvmBinaryClass() + if (kotlinJvmBinaryClass is VirtualFileKotlinClass) { + explicitClassesCanonicalPaths.add(kotlinJvmBinaryClass.file.path) + } + } + + private fun collectExplicitTypes(resolvedCall: ResolvedCall<*>) { + val kotlinCallArguments = (resolvedCall as? NewAbstractResolvedCall) + ?.resolvedCallAtom?.atom?.argumentsInParenthesis + + // note that callArgument can be both a PSIKotlinCallArgument and an ExpressionKotlinCallArgument + kotlinCallArguments?.forEach { callArgument -> + if (callArgument is PSIKotlinCallArgument) { + val dataFlowInfos = listOf(callArgument.dataFlowInfoBeforeThisArgument) + + callArgument.dataFlowInfoAfterThisArgument + + dataFlowInfos.forEach { dataFlowInfo -> + dataFlowInfo.completeTypeInfo.values().flatten().forEach { kotlinType -> + collectTypeReferences(kotlinType, isExplicit = true) + } } - val virtualFileClass = - resultingDescriptor.getContainingKotlinJvmBinaryClass() as? VirtualFileKotlinClass - ?: return - explicitClassesCanonicalPaths.add(virtualFileClass.file.path) - } - is ParameterDescriptor -> { - getClassCanonicalPath(resultingDescriptor)?.let { explicitClassesCanonicalPaths.add(it) } } - is FakeCallableDescriptorForObject -> { - collectTypeReferences(resultingDescriptor.type) - } - is JavaPropertyDescriptor -> { - getClassCanonicalPath(resultingDescriptor)?.let { explicitClassesCanonicalPaths.add(it) } - } - is PropertyDescriptor -> { - when (resultingDescriptor.containingDeclaration) { - is ClassDescriptor -> collectTypeReferences( - (resultingDescriptor.containingDeclaration as ClassDescriptor).defaultType, - ) - else -> { - val virtualFileClass = - (resultingDescriptor).getContainingKotlinJvmBinaryClass() as? VirtualFileKotlinClass - ?: return - explicitClassesCanonicalPaths.add(virtualFileClass.file.path) - } + + if (callArgument is ExpressionKotlinCallArgument) { + callArgument.receiver.allOriginalTypes.forEach { kotlinType -> + collectTypeReferences(kotlinType, isExplicit = true) } - collectTypeReferences(resultingDescriptor.type, isExplicit = false) } - else -> return } } @@ -193,7 +252,7 @@ class JdepsGenExtension( } } is FunctionDescriptor -> { - descriptor.returnType?.let { collectTypeReferences(it) } + descriptor.returnType?.let { collectTypeReferences(it, collectTypeArguments = false ) } descriptor.valueParameters.forEach { valueParameter -> collectTypeReferences(valueParameter.type) } @@ -245,6 +304,17 @@ class JdepsGenExtension( } } + // Collect type aliases aka abbreviations + // See: https://github.com/Kotlin/KEEP/blob/master/proposals/type-aliases.md#type-alias-expansion + kotlinType.getAbbreviation()?.let { abbreviationType -> + collectTypeReferences( + abbreviationType, + isExplicit = isExplicit, + collectTypeArguments = collectTypeArguments, + visitedKotlinTypes = visitedKotlinTypes, + ) + } + kotlinType.supertypes().forEach { supertype -> collectTypeReferences( supertype, diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel b/src/test/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel index d7e8cd0f5..9a2c48e1c 100644 --- a/src/test/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel @@ -19,6 +19,15 @@ java_library( name = "JdepsParserTestFixtures", testonly = True, srcs = glob(["testFixtures/*.java"]), + deps = [ + ":JdepsParserTestFixtures2", + ], +) + +java_library( + name = "JdepsParserTestFixtures2", + testonly = True, + srcs = glob(["testFixtures2/*.java"]), ) kt_rules_test( @@ -44,7 +53,10 @@ kt_rules_test( name = "KotlinBuilderJvmJdepsTest", size = "large", srcs = ["jvm/KotlinBuilderJvmJdepsTest.kt"], - data = [":JdepsParserTestFixtures"], + data = [ + ":JdepsParserTestFixtures", + ":JdepsParserTestFixtures2", + ], ) kt_rules_test( 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 c47ef7f80..610492c41 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 @@ -46,6 +46,7 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { val ctx = KotlinJvmTestBuilder() val TEST_FIXTURES_DEP = Dep.fromLabel(":JdepsParserTestFixtures") + val TEST_FIXTURES2_DEP = Dep.fromLabel(":JdepsParserTestFixtures2") @Test fun `no kotlin source produces empty jdeps`() { @@ -155,6 +156,57 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { assertIncomplete(jdeps).isEmpty() } + @Test + fun `find explicit exception and its supertypes from throws annotation`() { + val baseExceptionTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "BaseException.kt", + """ + package something + + open class BaseException : Exception() + """ + ) + } + + val exceptionTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "MyException.kt", + """ + package something + + class MyException : something.BaseException() + """ + ) + c.addDirectDependencies(baseExceptionTarget) + } + + val dependingTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "AnotherClass.kt", + """ + package something + + import something.MyException + + class AnotherClass { + @Throws(MyException::class) + fun hasAnnotation() {} + } + """ + ) + c.addDirectDependencies(exceptionTarget) + c.addTransitiveDependencies(baseExceptionTarget) + } + val jdeps = depsProto(dependingTarget) + + assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) + + assertExplicit(jdeps).contains(exceptionTarget.singleCompileJar()) + assertImplicit(jdeps).contains(baseExceptionTarget.singleCompileJar()) + assertUnused(jdeps).isEmpty() + assertIncomplete(jdeps).isEmpty() + } @Test fun `annotation on class is an explict dep`() { @@ -355,6 +407,46 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { assertIncomplete(jdeps).isEmpty() } + @Test + fun `pattern match exception`() { + val connectionNotFoundExceptionDep = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "ConnectionNotFoundException.kt", + """ + package something2 + + class ConnectionNotFoundException : Exception() + """ + ) + } + + println("=====================================") + val dependingTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "PatternMatchException.kt", + """ + package something + import something2.ConnectionNotFoundException + + fun isRetryable(e: Exception): Boolean { + return when (e) { + is ConnectionNotFoundException -> false + else -> true + } + } + """ + ) + c.addDirectDependencies(connectionNotFoundExceptionDep) + } + val jdeps = depsProto(dependingTarget) + + assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) + assertExplicit(jdeps).contains(connectionNotFoundExceptionDep.singleCompileJar()) + assertImplicit(jdeps).isEmpty() + assertUnused(jdeps).isEmpty() + assertIncomplete(jdeps).isEmpty() + } + @Test fun `kotlin property reference`() { val dependentTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> @@ -636,6 +728,86 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { assertIncomplete(jdeps).isEmpty() } + @Test + fun `java static repro`() { + val dependingTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "HasPropertyDefinition.kt", + """ + package something + + class HasPropertyDefinition { + fun provideMyHttpClient(): MyHttpClient { + return MyHttpClient.builder() + .name("ApiClient") + .circuitBreakerStrategy(CircuitBreakerStrategies.alwaysClosed()) + .build() + } + } + """ + ) + c.addDirectDependencies(TEST_FIXTURES_DEP) + } + val jdeps = depsProto(dependingTarget) + + assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) + + assertExplicit(jdeps).containsExactly(TEST_FIXTURES_DEP.singleCompileJar()) + assertImplicit(jdeps).isEmpty() + assertUnused(jdeps).isEmpty() + assertIncomplete(jdeps).isEmpty() + } + + @Test + fun `java static repro 2`() { + val dependingTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "Repro.kt", + """ + package something + + class Repro { + fun hi() { + (GlobalTracer.get()?.activeSpan() as? MySpan)?.report(RuntimeException(), false) + } + } + """ + ) + c.addDirectDependencies(TEST_FIXTURES_DEP) + c.addTransitiveDependencies(TEST_FIXTURES2_DEP) + } + val jdeps = depsProto(dependingTarget) + + assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) + + assertExplicit(jdeps).contains(TEST_FIXTURES_DEP.singleCompileJar()) + assertImplicit(jdeps).contains(TEST_FIXTURES2_DEP.singleCompileJar()) + assertUnused(jdeps).isEmpty() + assertIncomplete(jdeps).isEmpty() + } + + @Test + fun `test client1 and client2 are explicit jdeps`() { + val dependingTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "AClass.kt", + """ + package something + import something.JavaBaseWithTypeParam + + class AClass: JavaBaseWithTypeParam() { + val client1Instance = JavaWidgetFactory.create(client1, client2) + } + """ + ) + c.addDirectDependencies(TEST_FIXTURES_DEP, TEST_FIXTURES2_DEP) + } + + val jdeps = depsProto(dependingTarget) + assertExplicit(jdeps).contains(TEST_FIXTURES_DEP.singleCompileJar()) + assertExplicit(jdeps).contains(TEST_FIXTURES2_DEP.singleCompileJar()) + } + @Test fun `java enum reference`() { val dependingTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> @@ -729,6 +901,70 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { assertIncomplete(jdeps).isEmpty() } + @Test + fun `inline reified test`() { + val objectMapperTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "ObjectMapper.kt", + """ + package something + + class ObjectMapper { + } + + object Mappers { + val TEST_MAPPER = ObjectMapper() + } + """ + ) + } + + val dependentTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "ArgumentMatchers.kt", + """ + package something + import something.Mappers.TEST_MAPPER + + object ArgumentMatchers { + inline fun argThatMatchesJson( + expected: String, + policies: List = listOf(), + mapper: ObjectMapper = TEST_MAPPER + ): T = T::class.java.newInstance() + } + """ + ) + c.addDirectDependencies(objectMapperTarget) + } + + val dependingTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "HasGenericTypeDependency.kt", + """ + package something + + import something.ArgumentMatchers.argThatMatchesJson + + fun hi(msg: String): String { + return msg + } + + fun something() { + hi(argThatMatchesJson(""${'"'}{"userIds": [123]}""${'"'})) + } + """ + ) + c.addDirectDependencies(dependentTarget) + c.addTransitiveDependencies(objectMapperTarget) + } + + val jdeps = depsProto(dependingTarget) + assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) + assertExplicit(jdeps).contains(dependentTarget.singleCompileJar()) + assertImplicit(jdeps).contains(objectMapperTarget.singleCompileJar()) + } + @Test fun `inlined constant dependency recorded`() { val dependentTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> @@ -1385,7 +1621,7 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { assertExplicit(jdeps).contains(depWithFunction.singleCompileJar()) assertImplicit(jdeps).contains(depWithReturnType.singleCompileJar()) - assertImplicit(jdeps).doesNotContain(depWithReturnTypesSuperType) + assertImplicit(jdeps).contains(depWithReturnTypesSuperType.singleCompileJar()) } @Test @@ -1650,7 +1886,8 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { """ ) c.addDirectDependencies(depWithFunction) - c.addTransitiveDependencies(depWithReturnType, depWithReturnTypesSuperType) + c.addTransitiveDependencies(depWithReturnType) + c.addTransitiveDependencies(depWithReturnTypesSuperType) c.setLabel("dependingTarget") } val jdeps = depsProto(dependingTarget) @@ -1659,7 +1896,34 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { assertExplicit(jdeps).containsAtLeast(depWithFunction.singleCompileJar(), depWithReturnType.singleCompileJar()) assertImplicit(jdeps).doesNotContain(depWithReturnType.singleCompileJar()) - assertImplicit(jdeps).doesNotContain(depWithReturnTypesSuperType) + assertImplicit(jdeps).contains(depWithReturnTypesSuperType.singleCompileJar()) + } + + @Test + fun `string interpolation`() { + val dependingTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "ReferencesConsumerConfig.kt", + """ + package something + + private val log = Log() + fun doit(consumerConfig: ConsumerConfig?) { + log.info("busConfig: ${'$'}{consumerConfig?.busConfig}") + } + + class Log { + fun info(message: String) {} + } + """ + ) + c.addDirectDependencies(TEST_FIXTURES_DEP) + c.setLabel("dependingTarget") + } + val jdeps = depsProto(dependingTarget) + + assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) + assertExplicit(jdeps).contains(TEST_FIXTURES_DEP.singleCompileJar()) } private fun depsProto(jdeps: Dep) = diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/AlwaysClosedStrategy.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/AlwaysClosedStrategy.java new file mode 100644 index 000000000..aac65385c --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/AlwaysClosedStrategy.java @@ -0,0 +1,5 @@ +package something; + +public class AlwaysClosedStrategy implements CircuitBreakerStrategy { + public static final AlwaysClosedStrategy INSTANCE = new AlwaysClosedStrategy(); +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/BusConfig.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/BusConfig.java new file mode 100644 index 000000000..a828e9641 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/BusConfig.java @@ -0,0 +1,4 @@ +package something; + +public class BusConfig { +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/CircuitBreakerStrategies.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/CircuitBreakerStrategies.java new file mode 100644 index 000000000..cb0d8808d --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/CircuitBreakerStrategies.java @@ -0,0 +1,8 @@ +package something; + +public class CircuitBreakerStrategies { + public static CircuitBreakerStrategy alwaysClosed() { + return AlwaysClosedStrategy.INSTANCE; + } +} + diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/CircuitBreakerStrategy.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/CircuitBreakerStrategy.java new file mode 100644 index 000000000..80f182046 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/CircuitBreakerStrategy.java @@ -0,0 +1,4 @@ +package something; + +public interface CircuitBreakerStrategy { +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Client1.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Client1.java new file mode 100644 index 000000000..cae487bc4 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Client1.java @@ -0,0 +1,4 @@ +package something; + +public class Client1 { +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/ConsumerConfig.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/ConsumerConfig.java new file mode 100644 index 000000000..808d37a16 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/ConsumerConfig.java @@ -0,0 +1,7 @@ +package something; + +public class ConsumerConfig { + public BusConfig getBusConfig() { + return new BusConfig(); + } +} \ No newline at end of file diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/GlobalTracer.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/GlobalTracer.java new file mode 100644 index 000000000..2d50786e1 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/GlobalTracer.java @@ -0,0 +1,7 @@ +package something; + +public final class GlobalTracer { + public static Tracer get() { + return null; + } +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaBaseWithTypeParam.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaBaseWithTypeParam.java index 21c77da36..852b279d7 100644 --- a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaBaseWithTypeParam.java +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaBaseWithTypeParam.java @@ -4,4 +4,6 @@ public class JavaBaseWithTypeParam { public T passthru(T t) { return t; } + protected Client1 client1 = new Client1(); + protected TestClient2 client2 = new TestClient2(); } diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaClass.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaClass.java index b2af3509b..aac2362f0 100644 --- a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaClass.java +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaClass.java @@ -8,4 +8,8 @@ public interface InnerJavaClass { public static boolean staticMethod() { return true; } + + public static boolean staticMethod2() { + return true; + } } \ No newline at end of file diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaException.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaException.java new file mode 100644 index 000000000..f8d9c909a --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaException.java @@ -0,0 +1,4 @@ +package something; + +public class JavaException extends BaseException { +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaWidgetFactory.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaWidgetFactory.java new file mode 100644 index 000000000..8b8785b0c --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/JavaWidgetFactory.java @@ -0,0 +1,7 @@ +package something; + +public class JavaWidgetFactory { + public static Widget create(Client1 client1, Client2 client2) { + return new Widget(); + } +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MyHttpClient.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MyHttpClient.java new file mode 100644 index 000000000..216a3ac4f --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MyHttpClient.java @@ -0,0 +1,7 @@ +package something; + +public class MyHttpClient { + public static MyHttpClientBuilder builder() { + return new MyHttpClientBuilder(); + } +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MyHttpClientBuilder.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MyHttpClientBuilder.java new file mode 100644 index 000000000..6f1adb1fb --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MyHttpClientBuilder.java @@ -0,0 +1,16 @@ +package something; + + +public class MyHttpClientBuilder { + public MyHttpClientBuilder name(String name) { + return this; + } + + public MyHttpClientBuilder circuitBreakerStrategy(CircuitBreakerStrategy strategy) { + return this; + } + + public something.MyHttpClient build() { + return new MyHttpClient(); + } +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MySpan.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MySpan.java new file mode 100644 index 000000000..e2b907e32 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/MySpan.java @@ -0,0 +1,8 @@ +package something; + +public class MySpan implements Span, MyErrorReporter { + @Override + public void report(Throwable e, boolean fatal) { + System.out.println("Error: " + e); + } +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Span.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Span.java new file mode 100644 index 000000000..f14ec2c1e --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Span.java @@ -0,0 +1,4 @@ +package something; + +public interface Span { +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Tracer.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Tracer.java new file mode 100644 index 000000000..357a37463 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Tracer.java @@ -0,0 +1,5 @@ +package something; + +interface Tracer { + Span activeSpan(); +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Widget.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Widget.java new file mode 100644 index 000000000..644ef308d --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures/Widget.java @@ -0,0 +1,4 @@ +package something; + +public class Widget { +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/BaseException.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/BaseException.java new file mode 100644 index 000000000..96ed166b0 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/BaseException.java @@ -0,0 +1,4 @@ +package something; + +public class BaseException extends Exception { +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/Client2.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/Client2.java new file mode 100644 index 000000000..4906d4c51 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/Client2.java @@ -0,0 +1,4 @@ +package something; + +public class Client2 { +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/MyErrorReporter.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/MyErrorReporter.java new file mode 100644 index 000000000..0cb2bcfa2 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/MyErrorReporter.java @@ -0,0 +1,5 @@ +package something; + +public interface MyErrorReporter { + public void report(Throwable e, boolean fatal); +} diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/TestClient2.java b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/TestClient2.java new file mode 100644 index 000000000..ec638d5a6 --- /dev/null +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/testFixtures2/TestClient2.java @@ -0,0 +1,4 @@ +package something; + +public class TestClient2 extends Client2 { +} From 59c38408945d1ef6788f5adbadd1a3a0c26f2e1c Mon Sep 17 00:00:00 2001 From: Steve Cosenza Date: Mon, 26 Feb 2024 11:20:53 -0800 Subject: [PATCH 2/9] formatting --- .../io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) 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 3f672b468..dae957c18 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt @@ -88,10 +88,11 @@ class JdepsGenExtension( val sourceElement = if (descriptor.source != SourceElement.NO_SOURCE) { descriptor.source } else { - when(val declarationDescriptor = descriptor.getImportableDescriptor()) { + when (val declarationDescriptor = descriptor.getImportableDescriptor()) { is DeserializedTypeAliasDescriptor -> { declarationDescriptor.containerSource } + else -> { null } @@ -113,11 +114,14 @@ class JdepsGenExtension( // Ignore Java source local to this module. null } + is KotlinJvmBinarySourceElement -> (sourceElement.binaryClass as VirtualFileKotlinClass).file.canonicalPath + is JvmPackagePartSource -> { // This case is needed to collect type aliases ((sourceElement.knownJvmBinaryClass) as VirtualFileKotlinClass).file.canonicalPath } + else -> { null } @@ -251,8 +255,9 @@ class JdepsGenExtension( collectTypeReferences(it) } } + is FunctionDescriptor -> { - descriptor.returnType?.let { collectTypeReferences(it, collectTypeArguments = false ) } + descriptor.returnType?.let { collectTypeReferences(it, collectTypeArguments = false) } descriptor.valueParameters.forEach { valueParameter -> collectTypeReferences(valueParameter.type) } @@ -263,6 +268,7 @@ class JdepsGenExtension( collectTypeReferences(it) } } + is PropertyDescriptor -> { collectTypeReferences(descriptor.type) descriptor.annotations.forEach { annotation -> @@ -272,6 +278,7 @@ class JdepsGenExtension( collectTypeReferences(annotation.type) } } + is LocalVariableDescriptor -> { collectTypeReferences(descriptor.type) } From c58339e5adb4dd778999a20fc61160cfd4b47902 Mon Sep 17 00:00:00 2001 From: Steve Cosenza Date: Mon, 26 Feb 2024 11:57:37 -0800 Subject: [PATCH 3/9] formatting --- .../kotlin/plugin/jdeps/JdepsGenExtension.kt | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) 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 dae957c18..a68fc57f2 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt @@ -8,15 +8,15 @@ import org.jetbrains.kotlin.analyzer.AnalysisResult import org.jetbrains.kotlin.config.CompilerConfiguration import org.jetbrains.kotlin.container.StorageComponentContainer import org.jetbrains.kotlin.container.useInstance +import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithSource -import org.jetbrains.kotlin.descriptors.ValueDescriptor -import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.descriptors.ModuleDescriptor import org.jetbrains.kotlin.descriptors.PropertyDescriptor import org.jetbrains.kotlin.descriptors.SourceElement +import org.jetbrains.kotlin.descriptors.ValueDescriptor import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor import org.jetbrains.kotlin.extensions.StorageComponentContainerContributor import org.jetbrains.kotlin.js.translate.callTranslator.getReturnType @@ -172,7 +172,11 @@ class JdepsGenExtension( collectExplicitTypes(resultingDescriptor) val isExplicitReturnType = resultingDescriptor is JavaClassConstructorDescriptor - collectTypeReferences(resolvedCall.getReturnType(), isExplicit = isExplicitReturnType, collectTypeArguments = false) + collectTypeReferences( + resolvedCall.getReturnType(), + isExplicit = isExplicitReturnType, + collectTypeArguments = false, + ) resultingDescriptor.returnType?.let { collectTypeReferences(it, isExplicit = isExplicitReturnType, collectTypeArguments = false) } @@ -190,8 +194,10 @@ class JdepsGenExtension( } if (resultingDescriptor is PropertyDescriptor) { - (resultingDescriptor.getter - ?.correspondingProperty as? SyntheticJavaPropertyDescriptor) + ( + resultingDescriptor.getter + ?.correspondingProperty as? SyntheticJavaPropertyDescriptor + ) ?.let { syntheticJavaPropertyDescriptor -> collectTypeReferences(syntheticJavaPropertyDescriptor.type, isExplicit = false) From 816086e729ead583826962ed8a09a88e01add024 Mon Sep 17 00:00:00 2001 From: Steve Cosenza Date: Mon, 26 Feb 2024 12:06:07 -0800 Subject: [PATCH 4/9] revert --- .bazelrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bazelrc b/.bazelrc index e856fcbcb..c1d9ef968 100644 --- a/.bazelrc +++ b/.bazelrc @@ -3,4 +3,4 @@ common --enable_bzlmod=true build --strategy=KotlinCompile=worker build --test_output=all build --verbose_failures -common --jvmopt=-Djava.security.manager=allow + From ab93f8712134606ecdd42905b09cb1c53aeaf21a Mon Sep 17 00:00:00 2001 From: Steve Cosenza Date: Tue, 27 Feb 2024 07:45:59 -0800 Subject: [PATCH 5/9] Remove println used for testing --- .../bazel/kotlin/builder/tasks/jvm/KotlinBuilderJvmJdepsTest.kt | 1 - 1 file changed, 1 deletion(-) 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 610492c41..11959c682 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 @@ -420,7 +420,6 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { ) } - println("=====================================") val dependingTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> c.addSource( "PatternMatchException.kt", From 425a78f60ab24f734524b0ed89fd89f5fb4e529d Mon Sep 17 00:00:00 2001 From: Steve Cosenza Date: Sat, 2 Mar 2024 10:18:46 -0800 Subject: [PATCH 6/9] use new assertion style in jdeps test --- .../tasks/jvm/KotlinBuilderJvmJdepsTest.kt | 88 +++++++++++-------- 1 file changed, 52 insertions(+), 36 deletions(-) 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 4e01357e4..7147913b9 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 @@ -160,8 +160,8 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { val expected = Deps.Dependencies.newBuilder() .setRuleLabel("//:dependingTarget") .setSuccess(true) - .addExplicitDep(KOTLIN_STDLIB_DEP.singleCompileJar()) .addExplicitDep(TEST_FIXTURES_DEP.singleCompileJar()) + .addExplicitDep(KOTLIN_STDLIB_DEP.singleCompileJar()) .build() assertThat(jdeps).isEqualTo(expected) } @@ -209,13 +209,14 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { c.addTransitiveDependencies(baseExceptionTarget) } val jdeps = depsProto(dependingTarget) - - assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) - - assertExplicit(jdeps).contains(exceptionTarget.singleCompileJar()) - assertImplicit(jdeps).contains(baseExceptionTarget.singleCompileJar()) - assertUnused(jdeps).isEmpty() - assertIncomplete(jdeps).isEmpty() + val expected = Deps.Dependencies.newBuilder() + .setRuleLabel(dependingTarget.label()) + .setSuccess(true) + .addExplicitDep(exceptionTarget.singleCompileJar()) + .addExplicitDep(KOTLIN_STDLIB_DEP.singleCompileJar()) + .addImplicitDep(baseExceptionTarget.singleCompileJar()) + .build() + assertThat(jdeps).isEqualTo(expected) } @Test @@ -452,12 +453,13 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { c.addDirectDependencies(connectionNotFoundExceptionDep) } val jdeps = depsProto(dependingTarget) - - assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) - assertExplicit(jdeps).contains(connectionNotFoundExceptionDep.singleCompileJar()) - assertImplicit(jdeps).isEmpty() - assertUnused(jdeps).isEmpty() - assertIncomplete(jdeps).isEmpty() + val expected = Deps.Dependencies.newBuilder() + .setRuleLabel(dependingTarget.label()) + .setSuccess(true) + .addExplicitDep(connectionNotFoundExceptionDep.singleCompileJar()) + .addExplicitDep(KOTLIN_STDLIB_DEP.singleCompileJar()) + .build() + assertThat(jdeps).isEqualTo(expected) } @Test @@ -766,13 +768,12 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { c.addDirectDependencies(TEST_FIXTURES_DEP) } val jdeps = depsProto(dependingTarget) - - assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) - - assertExplicit(jdeps).containsExactly(TEST_FIXTURES_DEP.singleCompileJar()) - assertImplicit(jdeps).isEmpty() - assertUnused(jdeps).isEmpty() - assertIncomplete(jdeps).isEmpty() + val expected = Deps.Dependencies.newBuilder() + .setRuleLabel(dependingTarget.label()) + .setSuccess(true) + .addExplicitDep(TEST_FIXTURES_DEP.singleCompileJar()) + .build() + assertThat(jdeps).isEqualTo(expected) } @Test @@ -794,13 +795,14 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { c.addTransitiveDependencies(TEST_FIXTURES2_DEP) } val jdeps = depsProto(dependingTarget) - - assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) - - assertExplicit(jdeps).contains(TEST_FIXTURES_DEP.singleCompileJar()) - assertImplicit(jdeps).contains(TEST_FIXTURES2_DEP.singleCompileJar()) - assertUnused(jdeps).isEmpty() - assertIncomplete(jdeps).isEmpty() + val expected = Deps.Dependencies.newBuilder() + .setRuleLabel(dependingTarget.label()) + .setSuccess(true) + .addExplicitDep(TEST_FIXTURES_DEP.singleCompileJar()) + .addExplicitDep(KOTLIN_STDLIB_DEP.singleCompileJar()) + .addImplicitDep(TEST_FIXTURES2_DEP.singleCompileJar()) + .build() + assertThat(jdeps).isEqualTo(expected) } @Test @@ -821,8 +823,13 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { } val jdeps = depsProto(dependingTarget) - assertExplicit(jdeps).contains(TEST_FIXTURES_DEP.singleCompileJar()) - assertExplicit(jdeps).contains(TEST_FIXTURES2_DEP.singleCompileJar()) + val expected = Deps.Dependencies.newBuilder() + .setRuleLabel(dependingTarget.label()) + .setSuccess(true) + .addExplicitDep(TEST_FIXTURES_DEP.singleCompileJar()) + .addExplicitDep(TEST_FIXTURES2_DEP.singleCompileJar()) + .build() + assertThat(jdeps).isEqualTo(expected) } @Test @@ -982,9 +989,14 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { } val jdeps = depsProto(dependingTarget) - assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) - assertExplicit(jdeps).contains(dependentTarget.singleCompileJar()) - assertImplicit(jdeps).contains(objectMapperTarget.singleCompileJar()) + val expected = Deps.Dependencies.newBuilder() + .setRuleLabel(dependingTarget.label()) + .setSuccess(true) + .addExplicitDep(dependentTarget.singleCompileJar()) + .addExplicitDep(KOTLIN_STDLIB_DEP.singleCompileJar()) + .addImplicitDep(objectMapperTarget.singleCompileJar()) + .build() + assertThat(jdeps).isEqualTo(expected) } @Test @@ -1983,9 +1995,13 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { c.setLabel("dependingTarget") } val jdeps = depsProto(dependingTarget) - - assertThat(jdeps.ruleLabel).isEqualTo(dependingTarget.label()) - assertExplicit(jdeps).contains(TEST_FIXTURES_DEP.singleCompileJar()) + val expected = Deps.Dependencies.newBuilder() + .setRuleLabel(dependingTarget.label()) + .setSuccess(true) + .addExplicitDep(KOTLIN_STDLIB_DEP.singleCompileJar()) + .addExplicitDep(TEST_FIXTURES_DEP.singleCompileJar()) + .build() + assertThat(jdeps).isEqualTo(expected) } private fun depsProto(jdeps: Dep) = From 80ed7fbcca6e93014d1269ba08bb7ea1ef664137 Mon Sep 17 00:00:00 2001 From: Steve Cosenza Date: Sat, 2 Mar 2024 10:34:45 -0800 Subject: [PATCH 7/9] add newline to trigger another build --- .../bazel/kotlin/builder/tasks/jvm/KotlinBuilderJvmJdepsTest.kt | 1 + 1 file changed, 1 insertion(+) 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 41113558f..30a1dcea8 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 @@ -1993,6 +1993,7 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { c.addDirectDependencies(TEST_FIXTURES_DEP) c.setLabel("dependingTarget") } + val jdeps = depsProto(dependingTarget) val expected = Deps.Dependencies.newBuilder() .setRuleLabel(dependingTarget.label()) From f8605f8b94d3984ee450f744409638fcf8d5054b Mon Sep 17 00:00:00 2001 From: Steve Cosenza Date: Sun, 3 Mar 2024 16:37:08 -0800 Subject: [PATCH 8/9] Remove class in "js" package, that shouldn't be used and also is no longer needed. --- .../kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt | 5 ----- 1 file changed, 5 deletions(-) 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 a68fc57f2..cf86a96c4 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt @@ -172,11 +172,6 @@ class JdepsGenExtension( collectExplicitTypes(resultingDescriptor) val isExplicitReturnType = resultingDescriptor is JavaClassConstructorDescriptor - collectTypeReferences( - resolvedCall.getReturnType(), - isExplicit = isExplicitReturnType, - collectTypeArguments = false, - ) resultingDescriptor.returnType?.let { collectTypeReferences(it, isExplicit = isExplicitReturnType, collectTypeArguments = false) } From 46ca4d9707a49b9947a00c17ce40f3fbcdd8c17e Mon Sep 17 00:00:00 2001 From: Steve Cosenza Date: Sun, 3 Mar 2024 16:48:31 -0800 Subject: [PATCH 9/9] Remove class in "js" package, that shouldn't be used and also is no longer needed. --- .../kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt | 1 - 1 file changed, 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 cf86a96c4..890a99050 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt @@ -19,7 +19,6 @@ import org.jetbrains.kotlin.descriptors.SourceElement import org.jetbrains.kotlin.descriptors.ValueDescriptor import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor import org.jetbrains.kotlin.extensions.StorageComponentContainerContributor -import org.jetbrains.kotlin.js.translate.callTranslator.getReturnType import org.jetbrains.kotlin.load.java.descriptors.JavaClassConstructorDescriptor import org.jetbrains.kotlin.load.java.sources.JavaSourceElement import org.jetbrains.kotlin.load.java.structure.impl.classFiles.BinaryJavaClass