From df9d600d871323801d5ea0971d0e4bcbe9d54c99 Mon Sep 17 00:00:00 2001 From: Eric Chang Date: Tue, 19 Apr 2022 16:31:33 -0700 Subject: [PATCH] Handle non-standard capitalization of package names in root classes. Fix #3329. RELNOTES=n/a PiperOrigin-RevId: 442942242 --- .../hilt/android/plugin/root/Aggregator.kt | 55 ++++++- .../src/test/kotlin/NonStandardPackageTest.kt | 149 ++++++++++++++++++ .../aggregatedroot/AggregatedRoot.java | 27 ++++ .../root/AggregatedRootGenerator.java | 19 ++- 4 files changed, 243 insertions(+), 7 deletions(-) create mode 100644 java/dagger/hilt/android/plugin/src/test/kotlin/NonStandardPackageTest.kt diff --git a/java/dagger/hilt/android/plugin/src/main/kotlin/dagger/hilt/android/plugin/root/Aggregator.kt b/java/dagger/hilt/android/plugin/src/main/kotlin/dagger/hilt/android/plugin/root/Aggregator.kt index b1def24b12a..27ea2c7a600 100644 --- a/java/dagger/hilt/android/plugin/src/main/kotlin/dagger/hilt/android/plugin/root/Aggregator.kt +++ b/java/dagger/hilt/android/plugin/src/main/kotlin/dagger/hilt/android/plugin/root/Aggregator.kt @@ -122,24 +122,55 @@ private constructor( AggregatedAnnotation.AGGREGATED_ROOT -> { return object : AnnotationVisitor(asmApiVersion, nextAnnotationVisitor) { lateinit var rootClass: String + var rootPackage: String? = null + val rootSimpleNames: MutableList = mutableListOf() lateinit var originatingRootClass: String + var originatingRootPackage: String? = null + val originatingRootSimpleNames: MutableList = mutableListOf() lateinit var rootAnnotationClassName: Type override fun visit(name: String, value: Any?) { when (name) { "root" -> rootClass = value as String + "rootPackage" -> rootPackage = value as String "originatingRoot" -> originatingRootClass = value as String + "originatingRootPackage" -> originatingRootPackage = value as String "rootAnnotation" -> rootAnnotationClassName = (value as Type) + else -> throw IllegalStateException("Unexpected annotation value: " + name) } super.visit(name, value) } + override fun visitArray(name: String): AnnotationVisitor? { + return object : AnnotationVisitor(asmApiVersion, super.visitArray(name)) { + @Suppress("UNCHECKED_CAST") + override fun visit(passThroughValueName: String?, value: Any?) { + // Note that passThroughValueName should usually be null since the real name + // is the name passed to visitArray. + when (name) { + "rootSimpleNames" -> rootSimpleNames.add(value as String) + "originatingRootSimpleNames" -> originatingRootSimpleNames.add(value as String) + else -> error("Unexpected annotation value: $name") + } + super.visit(passThroughValueName, value) + } + } + } + override fun visitEnd() { + val rootClassName = parseClassName(rootPackage, rootSimpleNames, rootClass) + val originatingRootClassName = + parseClassName( + originatingRootPackage, + originatingRootSimpleNames, + originatingRootClass + ) + aggregatedRoots.add( AggregatedRootIr( fqName = annotatedClassName, - root = ClassName.bestGuess(rootClass), - originatingRoot = ClassName.bestGuess(originatingRootClass), + root = rootClassName, + originatingRoot = originatingRootClassName, rootAnnotation = rootAnnotationClassName.toClassName() ) ) @@ -401,5 +432,25 @@ private constructor( val shortNames = binaryName.substring(packageNameEndIndex + 1).split('$') return ClassName.get(packageName, shortNames.first(), *shortNames.drop(1).toTypedArray()) } + + fun parseClassName( + packageName: String?, + simpleNames: List, + fallbackCanonicalName: String + ): ClassName { + if (packageName != null) { + check(simpleNames.size > 0) + return ClassName.get( + packageName, + simpleNames.first(), + *simpleNames.subList(1, simpleNames.size).toTypedArray() + ) + } else { + // This is very unlikely, but if somehow an aggregated root is coming from a jar build with + // a previous Dagger version before the package name attribute was introduced, we should + // fallback to the old behavior of trying to guess at the name. + return ClassName.bestGuess(fallbackCanonicalName) + } + } } } diff --git a/java/dagger/hilt/android/plugin/src/test/kotlin/NonStandardPackageTest.kt b/java/dagger/hilt/android/plugin/src/test/kotlin/NonStandardPackageTest.kt new file mode 100644 index 00000000000..37b0f5a6c01 --- /dev/null +++ b/java/dagger/hilt/android/plugin/src/test/kotlin/NonStandardPackageTest.kt @@ -0,0 +1,149 @@ +/* + * Copyright (C) 2022 The Dagger Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import org.gradle.testkit.runner.TaskOutcome +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder + +/** + * Test that things still work when a root class is in a package with non-standard capitalization. + * https://github.com/google/dagger/issues/3329 + */ +class NonStandardPackageTest { + @get:Rule val testProjectDir = TemporaryFolder() + + lateinit var gradleRunner: GradleTestRunner + + @Before + fun setup() { + gradleRunner = GradleTestRunner(testProjectDir) + 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'", + "testImplementation 'com.google.dagger:hilt-android-testing:LOCAL-SNAPSHOT'", + "testAnnotationProcessor 'com.google.dagger:hilt-compiler:LOCAL-SNAPSHOT'", + ) + } + + @Test + fun test_capitalizedPackage() { + gradleRunner.addSrc( + srcPath = "NonStandard/MyApp.java", + srcContent = + """ + package NonStandard; + + import android.app.Application; + + @dagger.hilt.android.HiltAndroidApp + public class MyApp extends Application {} + """.trimIndent() + ) + gradleRunner.setAppClassName(".MyApp") + val result = gradleRunner.build() + val assembleTask = result.getTask(":assembleDebug") + assertEquals(TaskOutcome.SUCCESS, assembleTask.outcome) + } + + // This one is useful since a lone capitalized package may also succeed just because it looks like + // an enclosed class with no package. + @Test + fun test_capitalizedPackageWithLowercasePackages() { + gradleRunner.addSrc( + srcPath = "foo/NonStandard/bar/MyApp.java", + srcContent = + """ + package foo.NonStandard.bar; + + import android.app.Application; + + @dagger.hilt.android.HiltAndroidApp + public class MyApp extends Application {} + """.trimIndent() + ) + gradleRunner.setAppClassName(".MyApp") + val result = gradleRunner.build() + val assembleTask = result.getTask(":assembleDebug") + assertEquals(TaskOutcome.SUCCESS, assembleTask.outcome) + } + + @Test + fun test_capitalizedPackageWithLowercasePackagesNested() { + gradleRunner.addSrc( + srcPath = "foo/NonStandard/bar/Foo.java", + srcContent = + """ + package foo.NonStandard.bar; + + import android.app.Application; + + public final class Foo { + @dagger.hilt.android.HiltAndroidApp + public class MyApp extends Application {} + } + """.trimIndent() + ) + gradleRunner.setAppClassName(".Foo") + val result = gradleRunner.build() + val assembleTask = result.getTask(":assembleDebug") + assertEquals(TaskOutcome.SUCCESS, assembleTask.outcome) + } + + @Test + fun test_lowerCaseClassName() { + gradleRunner.addSrc( + srcPath = "foo/myApp.java", + srcContent = + """ + package foo; + + import android.app.Application; + + @dagger.hilt.android.HiltAndroidApp + public class myApp extends Application {} + """.trimIndent() + ) + gradleRunner.setAppClassName(".MyApp") + val result = gradleRunner.build() + val assembleTask = result.getTask(":assembleDebug") + assertEquals(TaskOutcome.SUCCESS, assembleTask.outcome) + } + + @Test + fun test_missingPackage() { + gradleRunner.addSrc( + // GradleTestRunner doesn't let you add files directly to the root so just put this in + // some other directory. The source still doesn't have a package though. + srcPath = "tmp/MyApp.java", + srcContent = + """ + import android.app.Application; + + @dagger.hilt.android.HiltAndroidApp + public class MyApp extends Application { } + """.trimIndent() + ) + gradleRunner.setAppClassName(".MyApp") + val result = gradleRunner.build() + val assembleTask = result.getTask(":assembleDebug") + assertEquals(TaskOutcome.SUCCESS, assembleTask.outcome) + } +} diff --git a/java/dagger/hilt/internal/aggregatedroot/AggregatedRoot.java b/java/dagger/hilt/internal/aggregatedroot/AggregatedRoot.java index a6aa5a50cc9..2b014d71c48 100644 --- a/java/dagger/hilt/internal/aggregatedroot/AggregatedRoot.java +++ b/java/dagger/hilt/internal/aggregatedroot/AggregatedRoot.java @@ -28,9 +28,36 @@ @Target(ElementType.TYPE) @Retention(RetentionPolicy.CLASS) public @interface AggregatedRoot { + /** Canonical name of the root class. Only used if the below package/simple names aren't set. */ String root(); + /** + * Package of the root class, separated because this isn't guaranteed to be distinguishable from + * the canonical name. + */ + String rootPackage(); + + /** + * The root class's simple names, in order of outer to inner. + */ + String[] rootSimpleNames(); + + /** + * Deprecated. Canonical name of the originating root class. Only used if the below package/simple + * names aren't set. + */ String originatingRoot(); + /** + * Package of the originating root class, separated because this isn't guaranteed to be + * distinguishable from the canonical name. + */ + String originatingRootPackage(); + + /** + * The originating root class's simple names, in order of outer to inner. + */ + String[] originatingRootSimpleNames(); + Class rootAnnotation(); } diff --git a/java/dagger/hilt/processor/internal/root/AggregatedRootGenerator.java b/java/dagger/hilt/processor/internal/root/AggregatedRootGenerator.java index d79c0a71054..1abe44b628f 100644 --- a/java/dagger/hilt/processor/internal/root/AggregatedRootGenerator.java +++ b/java/dagger/hilt/processor/internal/root/AggregatedRootGenerator.java @@ -17,6 +17,7 @@ package dagger.hilt.processor.internal.root; import com.squareup.javapoet.AnnotationSpec; +import com.squareup.javapoet.ClassName; import dagger.hilt.processor.internal.ClassNames; import dagger.hilt.processor.internal.Processors; import java.io.IOException; @@ -42,13 +43,21 @@ final class AggregatedRootGenerator { } void generate() throws IOException { - Processors.generateAggregatingClass( - ClassNames.AGGREGATED_ROOT_PACKAGE, - AnnotationSpec.builder(ClassNames.AGGREGATED_ROOT) + AnnotationSpec.Builder aggregatedRootAnnotation = AnnotationSpec.builder( + ClassNames.AGGREGATED_ROOT) .addMember("root", "$S", rootElement.getQualifiedName()) + .addMember("rootPackage", "$S", ClassName.get(rootElement).packageName()) .addMember("originatingRoot", "$S", originatingRootElement.getQualifiedName()) - .addMember("rootAnnotation", "$T.class", rootAnnotation) - .build(), + .addMember("originatingRootPackage", "$S", + ClassName.get(originatingRootElement).packageName()) + .addMember("rootAnnotation", "$T.class", rootAnnotation); + ClassName.get(rootElement).simpleNames().forEach( + name -> aggregatedRootAnnotation.addMember("rootSimpleNames", "$S", name)); + ClassName.get(originatingRootElement).simpleNames().forEach( + name -> aggregatedRootAnnotation.addMember("originatingRootSimpleNames", "$S", name)); + Processors.generateAggregatingClass( + ClassNames.AGGREGATED_ROOT_PACKAGE, + aggregatedRootAnnotation.build(), rootElement, getClass(), processingEnv);