From 20b525a20f68d4f89bb43df1968722d3cb99ed91 Mon Sep 17 00:00:00 2001 From: Daniel Santiago Date: Thu, 23 Feb 2023 08:53:35 -0800 Subject: [PATCH] Fix KAPT config name finding to discover SPI Plugin for aggregating javac compile task. This change also moves the creation of the hiltAnnotationProcessor configuration earlier since creating it during task configuration is not required and can lead to some issues if the configuration contained is locked. Updated the SPIPluginTest.kt to verify behaviour with KAPT. Fixes https://github.com/google/dagger/issues/3464 RELNOTES=N/A PiperOrigin-RevId: 511799130 --- .../hilt/android/plugin/main/build.gradle | 1 + .../hilt/android/plugin/HiltGradlePlugin.kt | 44 +++++++++++-------- .../android/plugin/util/Configurations.kt | 2 +- .../main/src/test/kotlin/GradleTestRunner.kt | 21 +++++++++ .../main/src/test/kotlin/SPIPluginTest.kt | 26 +++++++++-- 5 files changed, 72 insertions(+), 22 deletions(-) diff --git a/java/dagger/hilt/android/plugin/main/build.gradle b/java/dagger/hilt/android/plugin/main/build.gradle index 12ba7f42ce5..ffa1abb4f36 100644 --- a/java/dagger/hilt/android/plugin/main/build.gradle +++ b/java/dagger/hilt/android/plugin/main/build.gradle @@ -72,6 +72,7 @@ dependencies { testImplementation 'junit:junit:4.12' testImplementation 'com.google.truth:truth:1.0.1' testPluginCompile 'com.android.tools.build:gradle:7.1.2' + testPluginCompile 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.8.0' } // Configure the generating task of plugin-under-test-metadata.properties to diff --git a/java/dagger/hilt/android/plugin/main/src/main/kotlin/dagger/hilt/android/plugin/HiltGradlePlugin.kt b/java/dagger/hilt/android/plugin/main/src/main/kotlin/dagger/hilt/android/plugin/HiltGradlePlugin.kt index f2dd881af62..c33586cdf63 100644 --- a/java/dagger/hilt/android/plugin/main/src/main/kotlin/dagger/hilt/android/plugin/HiltGradlePlugin.kt +++ b/java/dagger/hilt/android/plugin/main/src/main/kotlin/dagger/hilt/android/plugin/HiltGradlePlugin.kt @@ -325,6 +325,22 @@ class HiltGradlePlugin @Inject constructor( project.files(variant.javaCompileProvider.map {it.destinationDirectory.get() }) ) + val hiltAnnotationProcessorConfiguration = project.configurations.create( + "hiltAnnotationProcessor${variant.name.capitalize()}" + ).also { config -> + config.isCanBeConsumed = false + config.isCanBeResolved = true + // Add user annotation processor configuration, so that SPI plugins and other processors + // are discoverable. + val apConfigurations: List = mutableListOf().apply { + add(variant.annotationProcessorConfiguration) + project.configurations.findByName(getKaptConfigName(variant))?.let { add(it) } + } + config.extendsFrom(*apConfigurations.toTypedArray()) + // Add hilt-compiler even though it might be in the AP configurations already. + project.dependencies.add(config.name, "com.google.dagger:hilt-compiler:$HILT_VERSION") + } + fun getInputClasspath(artifactAttributeValue: String) = mutableListOf().apply { @Suppress("DEPRECATION") // Older variant API is deprecated @@ -399,21 +415,7 @@ class HiltGradlePlugin @Inject constructor( } compileTask.destinationDirectory.set(componentClasses.singleFile) compileTask.options.apply { - annotationProcessorPath = project.configurations.create( - "hiltAnnotationProcessor${variant.name.capitalize()}" - ).also { config -> - config.isCanBeConsumed = false - config.isCanBeResolved = true - // Add user annotation processor configuration, so that SPI plugins and other processors - // are discoverable. - val apConfigurations: List = mutableListOf().apply { - add(variant.annotationProcessorConfiguration) - project.configurations.findByName(getKaptConfigName(variant))?.let { add(it) } - } - config.extendsFrom(*apConfigurations.toTypedArray()) - // Add hilt-compiler even though it might be in the AP configurations already. - project.dependencies.add(config.name, "com.google.dagger:hilt-compiler:$HILT_VERSION") - } + annotationProcessorPath = hiltAnnotationProcessorConfiguration generatedSourceOutputDirectory.set( project.file( project.buildDir.resolve("generated/hilt/component_sources/${variant.name}/") @@ -491,9 +493,15 @@ class HiltGradlePlugin @Inject constructor( if (project.state.failure != null) { return } - val dependencies = project.configurations.flatMap { configuration -> - configuration.dependencies.map { dependency -> dependency.group to dependency.name } - } + val dependencies = project.configurations + .filterNot { + // Exclude plugin created config since plugin adds the deps to them. + it.name.startsWith("hiltAnnotationProcessor") || + it.name.startsWith("hiltCompileOnly") + } + .flatMap { configuration -> + configuration.dependencies.map { dependency -> dependency.group to dependency.name } + }.toSet() if (!dependencies.contains(LIBRARY_GROUP to "hilt-android")) { error(missingDepError("$LIBRARY_GROUP:hilt-android")) } diff --git a/java/dagger/hilt/android/plugin/main/src/main/kotlin/dagger/hilt/android/plugin/util/Configurations.kt b/java/dagger/hilt/android/plugin/main/src/main/kotlin/dagger/hilt/android/plugin/util/Configurations.kt index 0fe2f8324e4..b68cd383c7d 100644 --- a/java/dagger/hilt/android/plugin/main/src/main/kotlin/dagger/hilt/android/plugin/util/Configurations.kt +++ b/java/dagger/hilt/android/plugin/main/src/main/kotlin/dagger/hilt/android/plugin/util/Configurations.kt @@ -31,6 +31,6 @@ internal fun getKaptConfigName(variant: com.android.build.gradle.api.BaseVariant is com.android.build.gradle.api.UnitTestVariant -> "kaptTest${variant.name.substringBeforeLast("UnitTest").capitalize()}" else -> - "kapt${variant.name}" + "kapt${variant.name.capitalize()}" } } \ No newline at end of file diff --git a/java/dagger/hilt/android/plugin/main/src/test/kotlin/GradleTestRunner.kt b/java/dagger/hilt/android/plugin/main/src/test/kotlin/GradleTestRunner.kt index 9b1d6777727..fb47bdb13f1 100644 --- a/java/dagger/hilt/android/plugin/main/src/test/kotlin/GradleTestRunner.kt +++ b/java/dagger/hilt/android/plugin/main/src/test/kotlin/GradleTestRunner.kt @@ -22,10 +22,13 @@ import org.junit.rules.TemporaryFolder /** Testing utility class that sets up a simple Android project that applies the Hilt plugin. */ class GradleTestRunner(val tempFolder: TemporaryFolder) { + private val pluginClasspaths = mutableListOf() + private val pluginIds = mutableListOf() private val dependencies = mutableListOf() private val activities = mutableListOf() private val additionalAndroidOptions = mutableListOf() private val hiltOptions = mutableListOf() + private val additionalClosures = mutableListOf() private var appClassName: String? = null private var buildFile: File? = null private var gradlePropertiesFile: File? = null @@ -39,6 +42,17 @@ class GradleTestRunner(val tempFolder: TemporaryFolder) { tempFolder.newFolder("src", "main", "res") } + // Adds a Gradle plugin classpath to the test project, + // e.g. "org.jetbrains.kotlin:kotlin-gradle-plugin:1.8.0" + fun addPluginClasspath(pluginClasspath: String) { + pluginClasspaths.add(pluginClasspath) + } + + // Adds a Gradle plugin id to the test project, e.g. "kotlin-android" + fun addPluginId(pluginId: String) { + pluginIds.add(pluginId) + } + // Adds project dependencies, e.g. "implementation ::" fun addDependencies(vararg deps: String) { dependencies.addAll(deps) @@ -61,6 +75,10 @@ class GradleTestRunner(val tempFolder: TemporaryFolder) { hiltOptions.addAll(options) } + fun addAdditionalClosure(closure: String) { + additionalClosures.add(closure) + } + // Adds a source package to the project. The package path is relative to 'src/main/java'. fun addSrcPackage(packagePath: String) { File(tempFolder.root, "src/main/java/$packagePath").mkdirs() @@ -127,12 +145,14 @@ class GradleTestRunner(val tempFolder: TemporaryFolder) { } dependencies { classpath 'com.android.tools.build:gradle:7.1.2' + ${pluginClasspaths.joinToString(separator = "\n") { "classpath '$it'" }} } } plugins { id '${ if (isAppProject) "com.android.application" else "com.android.library" }' id 'com.google.dagger.hilt.android' + ${pluginIds.joinToString(separator = "\n") { "id '$it'" }} } android { @@ -168,6 +188,7 @@ class GradleTestRunner(val tempFolder: TemporaryFolder) { hilt { ${hiltOptions.joinToString(separator = "\n")} } + ${additionalClosures.joinToString(separator = "\n")} """.trimIndent() ) } diff --git a/java/dagger/hilt/android/plugin/main/src/test/kotlin/SPIPluginTest.kt b/java/dagger/hilt/android/plugin/main/src/test/kotlin/SPIPluginTest.kt index ffe6dc0fa22..c1254863041 100644 --- a/java/dagger/hilt/android/plugin/main/src/test/kotlin/SPIPluginTest.kt +++ b/java/dagger/hilt/android/plugin/main/src/test/kotlin/SPIPluginTest.kt @@ -21,8 +21,11 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder +import org.junit.runner.RunWith +import org.junit.runners.Parameterized -class SPIPluginTest { +@RunWith(Parameterized::class) +class SPIPluginTest(val withKapt: Boolean) { @get:Rule val testProjectDir = TemporaryFolder() @@ -40,12 +43,23 @@ class SPIPluginTest { """.trimIndent() ) } + val processorConfig = if (withKapt) "kapt" else "annotationProcessor" + if (withKapt) { + gradleRunner.addPluginClasspath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.8.0") + gradleRunner.addPluginId("kotlin-android") + gradleRunner.addPluginId("kotlin-kapt") + gradleRunner.addAdditionalClosure(""" + |kotlin { + | jvmToolchain(11) + |} + """.trimMargin()) + } gradleRunner.addHiltOption("enableAggregatingTask = true") gradleRunner.addDependencies( "implementation 'androidx.appcompat:appcompat:1.1.0'", "implementation 'com.google.dagger:hilt-android:LOCAL-SNAPSHOT'", - "annotationProcessor 'com.google.dagger:hilt-compiler:LOCAL-SNAPSHOT'", - "annotationProcessor project(':spi-plugin')", + "$processorConfig 'com.google.dagger:hilt-compiler:LOCAL-SNAPSHOT'", + "$processorConfig project(':spi-plugin')", ) gradleRunner.addSrc( srcPath = "minimal/MyApp.java", @@ -74,4 +88,10 @@ class SPIPluginTest { "[spi.TestPlugin] Found component: minimal.MyApp_HiltComponents.SingletonC" ) } + + companion object { + @JvmStatic + @Parameterized.Parameters(name = "withKapt = {0}") + fun params() = listOf(false, true) + } } \ No newline at end of file