Skip to content

Commit

Permalink
Handle non-standard capitalization of package names in root classes.
Browse files Browse the repository at this point in the history
Fix #3329.

RELNOTES=n/a
PiperOrigin-RevId: 444420586
  • Loading branch information
Chang-Eric authored and Dagger Team committed Apr 26, 2022
1 parent da3f868 commit 329915f
Show file tree
Hide file tree
Showing 4 changed files with 243 additions and 7 deletions.
Expand Up @@ -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<String> = mutableListOf<String>()
lateinit var originatingRootClass: String
var originatingRootPackage: String? = null
val originatingRootSimpleNames: MutableList<String> = mutableListOf<String>()
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()
)
)
Expand Down Expand Up @@ -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<String>,
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)
}
}
}
}
@@ -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)
}
}
27 changes: 27 additions & 0 deletions java/dagger/hilt/internal/aggregatedroot/AggregatedRoot.java
Expand Up @@ -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();
}
Expand Up @@ -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;
Expand All @@ -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);
Expand Down

0 comments on commit 329915f

Please sign in to comment.