Skip to content

Commit

Permalink
Fix a bug in AutoAnnotation when using Collections to pass values to …
Browse files Browse the repository at this point in the history
…generic array parameters.

Without this AutoAnnotation would generate code that constructed arrays of
generic types which would result in errors from javac like:
  "Cannot create a generic array of Class<? extends Annotation>"

Also, change the generated code to not presize the array when calling
Collection.toArray, for hotspot vms this is likely a pessimization per
http://shipilev.net/blog/2016/arrays-wisdom-ancients/ and I doubt
@AutoAnnotation is used much outside of hotspot vms.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=127234753
  • Loading branch information
lukesandberg authored and eamonnmcmanus committed Aug 30, 2016
1 parent 5ec14eb commit 82041d0
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import javax.lang.model.type.PrimitiveType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.type.WildcardType;
import javax.lang.model.util.ElementFilter;
import javax.lang.model.util.Types;
import javax.tools.Diagnostic;
Expand Down Expand Up @@ -486,6 +487,34 @@ public TypeKind getKind() {
return getTypeMirror().getKind();
}

public boolean isArrayOfClassWithBounds() {
if (getTypeMirror().getKind() != TypeKind.ARRAY) {
return false;
}
TypeMirror componentType = ((ArrayType) getTypeMirror()).getComponentType();
if (componentType.getKind() != TypeKind.DECLARED) {
return false;
}
DeclaredType declared = (DeclaredType) componentType;
if (!((TypeElement) processingEnv.getTypeUtils().asElement(componentType))
.getQualifiedName()
.contentEquals("java.lang.Class")) {
return false;
}
if (declared.getTypeArguments().size() != 1) {
return false;
}
TypeMirror parameter = declared.getTypeArguments().get(0);
if (parameter.getKind() != TypeKind.WILDCARD) {
return true; // for Class<Foo>
}
WildcardType wildcard = (WildcardType) parameter;
// In theory, we should check if getExtendsBound() != Object, since '?' is equivalent to
// '? extends Object', but, experimentally, neither javac or ecj will sets getExtendsBound()
// to 'Object', so there isn't a point in checking.
return wildcard.getSuperBound() != null || wildcard.getExtendsBound() != null;
}

public String getDefaultValue() {
AnnotationValue defaultValue = method.getDefaultValue();
if (defaultValue == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,15 @@ final class $className implements $annotationName {

this.$p = ${members[$p].typeMirror.componentType}ArrayFromCollection($p);

#elseif ($members[$p].arrayOfClassWithBounds)

@SuppressWarnings({"unchecked", "rawtypes"})
${members[$p].componentType}[] ${p}$ = ${p}.toArray(new Class[0]);
this.$p = ${p}$;

#else

this.$p = ${p}.toArray(new ${members[$p].componentType}[${p}.size()]);
this.$p = ${p}.toArray(new ${members[$p].componentType}[0]);

#end
#else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ public void testCollectionsForArrays() {
" if (enums == null) {",
" throw new NullPointerException(\"Null enums\");",
" }",
" this.enums = enums.toArray(new MyEnum[enums.size()];",
" this.enums = enums.toArray(new MyEnum[0];",
" }",
"",
" @Override public Class<? extends MyAnnotation> annotationType() {",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,4 +323,38 @@ public void testAnnotationValuedDefaultsNotSupportedYet() {
+ "'optionalAnnotation'")
.in(testSource).onLine(7);
}

public void testAnnotationMemberNameConflictWithGeneratedLocal() {
JavaFileObject annotationSource = JavaFileObjects.forSourceLines(
"com.example.TestAnnotation",
"package com.example;",
"",
"import java.lang.annotation.Annotation;",
"",
"public @interface TestAnnotation {",
" Class<? extends Annotation>[] value();",
" int value$();",
"}");
JavaFileObject testSource = JavaFileObjects.forSourceLines(
"com.foo.Test",
"package com.foo;",
"",
"import java.lang.annotation.Annotation;",
"import java.util.Collection;",
"",
"import com.example.TestAnnotation;",
"import com.google.auto.value.AutoAnnotation;",
"",
"class Test {",
" @AutoAnnotation static TestAnnotation newTestAnnotation(",
" Collection<Class<? extends Annotation>> value, int value$) {",
" return new AutoAnnotation_Test_newTestAnnotation(value, value$);",
" }",
"}");
assert_().about(javaSources())
.that(ImmutableList.of(annotationSource, testSource))
.processedWith(new AutoAnnotationProcessor())
.failsToCompile()
.withErrorContaining("variable value$ is already defined in constructor");
}
}

0 comments on commit 82041d0

Please sign in to comment.