diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java index d3bb04eccb..ae05891fa8 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java @@ -2796,4 +2796,37 @@ public void testOuterWithNonDefaultableInner() { } catch (IllegalStateException expected) { } } + + @SuppressWarnings("JavaLangClash") + @AutoValue + public abstract static class RedeclareJavaLangClasses { + // If you really really want to do this, we have you covered. + + public static class Object {} + public static class String {} + + public abstract Object alienObject(); + public abstract String alienString(); + + public static Builder builder() { + return new AutoValue_AutoValueTest_RedeclareJavaLangClasses.Builder(); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder setAlienObject(Object x); + public abstract Builder setAlienString(String x); + public abstract RedeclareJavaLangClasses build(); + } + } + + @Test + public void testRedeclareJavaLangClasses() { + RedeclareJavaLangClasses x = + RedeclareJavaLangClasses.builder() + .setAlienObject(new RedeclareJavaLangClasses.Object()) + .setAlienString(new RedeclareJavaLangClasses.String()) + .build(); + assertThat(x).isNotNull(); + } } diff --git a/value/src/main/java/com/google/auto/value/processor/TypeEncoder.java b/value/src/main/java/com/google/auto/value/processor/TypeEncoder.java index 14e8604098..6197464eb6 100644 --- a/value/src/main/java/com/google/auto/value/processor/TypeEncoder.java +++ b/value/src/main/java/com/google/auto/value/processor/TypeEncoder.java @@ -325,7 +325,7 @@ String rewrite() { Set referencedClasses = findReferencedClasses(); // Make a type simplifier based on these referenced types. TypeSimplifier typeSimplifier = - new TypeSimplifier(typeUtils, packageName, referencedClasses, baseType); + new TypeSimplifier(elementUtils, typeUtils, packageName, referencedClasses, baseType); StringBuilder output = new StringBuilder(); int copyStart; diff --git a/value/src/main/java/com/google/auto/value/processor/TypeSimplifier.java b/value/src/main/java/com/google/auto/value/processor/TypeSimplifier.java index 37ae68972e..8717872a1b 100644 --- a/value/src/main/java/com/google/auto/value/processor/TypeSimplifier.java +++ b/value/src/main/java/com/google/auto/value/processor/TypeSimplifier.java @@ -38,6 +38,7 @@ import javax.lang.model.type.TypeVariable; import javax.lang.model.type.WildcardType; import javax.lang.model.util.ElementFilter; +import javax.lang.model.util.Elements; import javax.lang.model.util.SimpleTypeVisitor8; import javax.lang.model.util.Types; @@ -67,6 +68,8 @@ private static class Spelling { /** * Makes a new simplifier for the given package and set of types. * + * @param elementUtils the result of {@code ProcessingEnvironment.getElementUtils()} for the + * current annotation processing environment. * @param typeUtils the result of {@code ProcessingEnvironment.getTypeUtils()} for the current * annotation processing environment. * @param packageName the name of the package from which classes will be referenced. Classes that @@ -79,14 +82,19 @@ private static class Spelling { * @throws MissingTypeException if one of the input types contains an error (typically, * is undefined). */ - TypeSimplifier(Types typeUtils, String packageName, Set types, TypeMirror base) { + TypeSimplifier( + Elements elementUtils, + Types typeUtils, + String packageName, + Set types, + TypeMirror base) { Set typesPlusBase = new TypeMirrorSet(types); if (base != null) { typesPlusBase.add(base); } Set topLevelTypes = topLevelTypes(typeUtils, typesPlusBase); Set defined = nonPrivateDeclaredTypes(typeUtils, base); - this.imports = findImports(typeUtils, packageName, topLevelTypes, defined); + this.imports = findImports(elementUtils, typeUtils, packageName, topLevelTypes, defined); } /** @@ -181,7 +189,7 @@ static String simpleNameOf(String s) { * goal should be complete correctness, and the only way to achieve that is to operate on the real * representations of types. * - * @param packageName The name of the package where the class containing these references is + * @param codePackageName The name of the package where the class containing these references is * defined. Other classes within the same package do not need to be imported. * @param referenced The complete set of declared types (classes and interfaces) that will be * referenced in the generated code. @@ -192,7 +200,11 @@ static String simpleNameOf(String s) { * whether the type should be imported, and how the type should be spelled in the source code. */ private static Map findImports( - Types typeUtils, String packageName, Set referenced, Set defined) { + Elements elementUtils, + Types typeUtils, + String codePackageName, + Set referenced, + Set defined) { Map imports = new HashMap<>(); Set typesInScope = new TypeMirrorSet(); typesInScope.addAll(referenced); @@ -208,7 +220,10 @@ private static Map findImports( if (ambiguous.contains(simpleName)) { importIt = false; spelling = fullName; - } else if (pkg.equals(packageName) || pkg.equals("java.lang")) { + } else if (pkg.equals("java.lang")) { + importIt = false; + spelling = javaLangSpelling(elementUtils, codePackageName, typeElement); + } else if (pkg.equals(codePackageName)) { importIt = false; spelling = fullName.substring(pkg.isEmpty() ? 0 : pkg.length() + 1); } else { @@ -220,6 +235,25 @@ private static Map findImports( return imports; } + /** + * Handles the tricky case where the class being referred to is in {@code java.lang}, but the + * package of the referring code contains another class of the same name. For example, if the + * current package is {@code foo.bar} and there is a {@code foo.bar.Compiler}, then we will refer + * to {@code java.lang.Compiler} by its full name. The plain name {@code Compiler} would reference + * {@code foo.bar.Compiler} in this case. We need to write {@code java.lang.Compiler} even if the + * other {@code Compiler} class is not being considered here, so the {@link #ambiguousNames} logic + * is not enough. We have to look to see if the class exists. + */ + private static String javaLangSpelling( + Elements elementUtils, String codePackageName, TypeElement typeElement) { + // If this is java.lang.Thread.State or the like, we have to look for a clash with Thread. + TypeElement topLevelType = topLevelType(typeElement); + TypeElement clash = + elementUtils.getTypeElement(codePackageName + "." + topLevelType.getSimpleName()); + String fullName = typeElement.getQualifiedName().toString(); + return (clash == null) ? fullName.substring("java.lang.".length()) : fullName; + } + /** * Finds the top-level types for all the declared types (classes and interfaces) in the given * {@code Set}. @@ -280,7 +314,7 @@ private static Set ambiguousNames(Types typeUtils, Set types } return ambiguous; } - + /** * Returns true if casting to the given type will elicit an unchecked warning from the * compiler. Only generic types such as {@code List} produce such warnings. There will be diff --git a/value/src/main/java/com/google/auto/value/processor/autovalue.vm b/value/src/main/java/com/google/auto/value/processor/autovalue.vm index cd45855486..bbdbf2f374 100644 --- a/value/src/main/java/com/google/auto/value/processor/autovalue.vm +++ b/value/src/main/java/com/google/auto/value/processor/autovalue.vm @@ -78,7 +78,7 @@ $a ## Just throw NullPointerException with no message if it's null. ## The Object cast has no effect on the code but silences an ErrorProne warning. - ((Object) ${p}).getClass(); + ((`java.lang.Object`) ${p}).getClass(); #end #end @@ -106,7 +106,7 @@ $a #if ($toString) @Override - public String toString() { + public `java.lang.String` toString() { return "#if ($identifiers)$simpleClassName#end{" #foreach ($p in $props) @@ -125,7 +125,7 @@ $a #if ($equals) @Override - public boolean equals(Object o) { + public boolean equals(`java.lang.Object` o) { if (o == this) { return true; } @@ -255,7 +255,7 @@ $a ## Just throw NullPointerException with no message if it's null. ## The Object cast has no effect on the code but silences an ErrorProne warning. - ((Object) ${p}).getClass(); + ((`java.lang.Object`) ${p}).getClass(); #end #end @@ -383,7 +383,7 @@ $a #if (!$builderRequiredProperties.empty) #if ($identifiers) ## build a friendly message showing all missing properties - String missing = ""; + `java.lang.String` missing = ""; #foreach ($p in $builderRequiredProperties) diff --git a/value/src/test/java/com/google/auto/value/processor/CompilationTest.java b/value/src/test/java/com/google/auto/value/processor/CompilationTest.java index 8070026dfd..6adbdfef39 100644 --- a/value/src/test/java/com/google/auto/value/processor/CompilationTest.java +++ b/value/src/test/java/com/google/auto/value/processor/CompilationTest.java @@ -2761,6 +2761,70 @@ public void builderWithVarArgsDoesNotImportJavaUtilArrays() { .doesNotContain("java.util.Arrays"); } + /** + * Tests behaviour when the package containing an {@code @AutoValue} class also has classes with + * the same name as classes in {@code java.lang}. If you call a class {@code Object} you are + * asking for trouble, but you could innocently call a class {@code Compiler} without realizing + * there is a {@code java.lang.Compiler}. + * + *

The case where the class in question is mentioned in the {@code @AutoValue} class is the + * easy one, because then our logic can easily see that there is a clash and will use + * fully-qualified names. This is the case of the {@code Compiler} class below. The case where + * the class is not mentioned is harder. We have to realize that we can't elide the + * package name in {@code java.lang.Object} because there is also a {@code foo.bar.Object} in + * scope, and in fact it takes precedence. + */ + @Test + public void javaLangClash() { + JavaFileObject object = JavaFileObjects.forSourceLines( + "foo.bar.Object", + "package foo.bar;", + "", + "public class Object {}"); + JavaFileObject string = JavaFileObjects.forSourceLines( + "foo.bar.String", + "package foo.bar;", + "", + "public class String {}"); + JavaFileObject compiler = JavaFileObjects.forSourceLines( + "foo.bar.Compiler", + "package foo.bar;", + "", + "public class Compiler {}"); + JavaFileObject thread = JavaFileObjects.forSourceLines( + "foo.bar.Thread", + "package foo.bar;", + "", + "public class Thread {}"); + JavaFileObject test = JavaFileObjects.forSourceLines( + "foo.bar.Test", + "package foo.bar;", + "", + "import com.google.auto.value.AutoValue;", + "", + "@AutoValue", + "public abstract class Test {", + " public abstract java.lang.Compiler compiler();", + " public abstract java.lang.Thread.State state();", + " public static Builder builder() {", + " return new AutoValue_Test.Builder();", + " }", + "", + " @AutoValue.Builder", + " public abstract static class Builder {", + " public abstract Builder setCompiler(java.lang.Compiler x);", + " public abstract Builder setState(java.lang.Thread.State x);", + " public abstract Test build();", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoValueProcessor()) + .withOptions("-Xlint:-processing", "-implicit:none") + .compile(object, string, compiler, thread, test); + assertThat(compilation).succeededWithoutWarnings(); + } + private String sorted(String... imports) { return Stream.of(imports).sorted().collect(joining("\n")); }