Skip to content

Commit

Permalink
Ensure that @autovalue classes have correct code even if redeclaratio…
Browse files Browse the repository at this point in the history
…ns of Object or String are in scope.

Fixes #403.

RELNOTES=Handle redeclared Object or String in @autovalue classes.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=182927083
  • Loading branch information
eamonnmcmanus authored and ronshapiro committed Jan 24, 2018
1 parent a3b5ba2 commit 6a41d3a
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ String rewrite() {
Set<TypeMirror> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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<TypeMirror> types, TypeMirror base) {
TypeSimplifier(
Elements elementUtils,
Types typeUtils,
String packageName,
Set<TypeMirror> types,
TypeMirror base) {
Set<TypeMirror> typesPlusBase = new TypeMirrorSet(types);
if (base != null) {
typesPlusBase.add(base);
}
Set<TypeMirror> topLevelTypes = topLevelTypes(typeUtils, typesPlusBase);
Set<TypeMirror> defined = nonPrivateDeclaredTypes(typeUtils, base);
this.imports = findImports(typeUtils, packageName, topLevelTypes, defined);
this.imports = findImports(elementUtils, typeUtils, packageName, topLevelTypes, defined);
}

/**
Expand Down Expand Up @@ -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.
Expand All @@ -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<String, Spelling> findImports(
Types typeUtils, String packageName, Set<TypeMirror> referenced, Set<TypeMirror> defined) {
Elements elementUtils,
Types typeUtils,
String codePackageName,
Set<TypeMirror> referenced,
Set<TypeMirror> defined) {
Map<String, Spelling> imports = new HashMap<>();
Set<TypeMirror> typesInScope = new TypeMirrorSet();
typesInScope.addAll(referenced);
Expand All @@ -208,7 +220,10 @@ private static Map<String, Spelling> 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 {
Expand All @@ -220,6 +235,25 @@ private static Map<String, Spelling> 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<TypeMirror>}.
Expand Down Expand Up @@ -280,7 +314,7 @@ private static Set<String> ambiguousNames(Types typeUtils, Set<TypeMirror> 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<String>} produce such warnings. There will be
Expand Down
10 changes: 5 additions & 5 deletions value/src/main/java/com/google/auto/value/processor/autovalue.vm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
*
* <p>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 <i>not</i> 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"));
}
Expand Down

0 comments on commit 6a41d3a

Please sign in to comment.