Skip to content

Commit 1561e2c

Browse files
eamonnmcmanusronshapiro
eamonnmcmanus
authored and
ronshapiro
committed
Copy type annotations to the parameter of equals(Object).
If equals(Object) is being implemented because there is abstract redeclaration of it in the AutoValue class or an ancestor, and if that redeclaration has a type annotation such as @nullable on its parameter, then the generated implementation will also have that annotation. The same applies to AutoOneOf. We could go further and also copy annotations on the return types of equals/hashCode/toString, but for now there's no perceived need for that. Fixes #579. RELNOTES=In `@AutoValue`, copy `@Nullable` from `equals(@nullable Object)` to its implementation. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=188044994
1 parent 9ebbeb4 commit 1561e2c

File tree

10 files changed

+279
-19
lines changed

10 files changed

+279
-19
lines changed

‎value/src/it/functional/src/test/java/com/google/auto/value/AutoOneOfJava8Test.java‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
* Copyright (C) 2018 Google, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.auto.value;
17+
18+
import static com.google.common.truth.Truth.assertThat;
19+
20+
import java.lang.annotation.ElementType;
21+
import java.lang.annotation.Retention;
22+
import java.lang.annotation.RetentionPolicy;
23+
import java.lang.annotation.Target;
24+
import java.lang.reflect.AnnotatedType;
25+
import java.lang.reflect.Method;
26+
import org.junit.Test;
27+
import org.junit.runner.RunWith;
28+
import org.junit.runners.JUnit4;
29+
30+
/**
31+
* Tests for Java8-specific {@code @AutoOneOf} behaviour.
32+
*
33+
* @author [email protected] (Éamonn McManus)
34+
*/
35+
@RunWith(JUnit4.class)
36+
public class AutoOneOfJava8Test {
37+
@AutoOneOf(EqualsNullable.Kind.class)
38+
public abstract static class EqualsNullable {
39+
40+
@Target(ElementType.TYPE_USE)
41+
@Retention(RetentionPolicy.RUNTIME)
42+
public @interface Nullable {}
43+
44+
public enum Kind {THING}
45+
public abstract Kind kind();
46+
public abstract String thing();
47+
48+
public static EqualsNullable ofThing(String thing) {
49+
return AutoOneOf_AutoOneOfJava8Test_EqualsNullable.thing(thing);
50+
}
51+
52+
@Override
53+
public abstract boolean equals(@Nullable Object x);
54+
55+
@Override
56+
public abstract int hashCode();
57+
}
58+
59+
/**
60+
* Tests that a type annotation on the parameter of {@code equals(Object)} is copied into the
61+
* implementation class.
62+
*/
63+
@Test
64+
public void equalsNullable() throws ReflectiveOperationException {
65+
if (BugDetector.typeVisitorDropsAnnotations()) {
66+
System.err.println("TYPE VISITORS DO NOT SEE TYPE ANNOTATIONS, SKIPPING TEST");
67+
return;
68+
}
69+
EqualsNullable x = EqualsNullable.ofThing("foo");
70+
Class<? extends EqualsNullable> c = x.getClass();
71+
Method equals = c.getMethod("equals", Object.class);
72+
assertThat(equals.getDeclaringClass()).isNotSameAs(EqualsNullable.class);
73+
AnnotatedType parameterType = equals.getAnnotatedParameterTypes()[0];
74+
assertThat(parameterType.isAnnotationPresent(EqualsNullable.Nullable.class))
75+
.isTrue();
76+
}
77+
}

‎value/src/it/functional/src/test/java/com/google/auto/value/AutoValueJava8Test.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.lang.annotation.Retention;
3232
import java.lang.annotation.RetentionPolicy;
3333
import java.lang.annotation.Target;
34+
import java.lang.reflect.AnnotatedType;
3435
import java.lang.reflect.Constructor;
3536
import java.lang.reflect.Method;
3637
import java.util.Arrays;
@@ -512,4 +513,41 @@ public void testFunkyNullable() {
512513
FunkyNullable implicitNull = FunkyNullable.builder().build();
513514
assertThat(explicitNull).isEqualTo(implicitNull);
514515
}
516+
517+
@AutoValue
518+
abstract static class EqualsNullable {
519+
@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER})
520+
@Retention(RetentionPolicy.RUNTIME)
521+
@interface Nullable {}
522+
523+
abstract String foo();
524+
525+
static EqualsNullable create(String foo) {
526+
return new AutoValue_AutoValueJava8Test_EqualsNullable(foo);
527+
}
528+
529+
@Override
530+
public abstract boolean equals(@Nullable Object x);
531+
532+
@Override
533+
public abstract int hashCode();
534+
}
535+
536+
/**
537+
* Tests that a type annotation on the parameter of {@code equals(Object)} is copied into the
538+
* implementation class.
539+
*/
540+
@Test
541+
public void testEqualsNullable() throws ReflectiveOperationException {
542+
if (BugDetector.typeVisitorDropsAnnotations()) {
543+
System.err.println("TYPE VISITORS DO NOT SEE TYPE ANNOTATIONS, SKIPPING TEST");
544+
return;
545+
}
546+
EqualsNullable x = EqualsNullable.create("foo");
547+
Class<? extends EqualsNullable> implClass = x.getClass();
548+
Method equals = implClass.getDeclaredMethod("equals", Object.class);
549+
AnnotatedType[] parameterTypes = equals.getAnnotatedParameterTypes();
550+
assertThat(parameterTypes[0].isAnnotationPresent(EqualsNullable.Nullable.class))
551+
.isTrue();
552+
}
515553
}

‎value/src/it/functional/src/test/java/com/google/auto/value/BugDetector.java‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* Copyright (C) 2018 Google, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.auto.value;
17+
18+
import static com.google.common.truth.Truth.assertThat;
19+
import static com.google.testing.compile.CompilationSubject.assertThat;
20+
21+
import com.google.testing.compile.Compilation;
22+
import com.google.testing.compile.Compiler;
23+
import com.google.testing.compile.JavaFileObjects;
24+
import java.util.Collections;
25+
import java.util.List;
26+
import java.util.Set;
27+
import javax.annotation.processing.AbstractProcessor;
28+
import javax.annotation.processing.RoundEnvironment;
29+
import javax.annotation.processing.SupportedAnnotationTypes;
30+
import javax.annotation.processing.SupportedSourceVersion;
31+
import javax.lang.model.SourceVersion;
32+
import javax.lang.model.element.AnnotationMirror;
33+
import javax.lang.model.element.ExecutableElement;
34+
import javax.lang.model.element.TypeElement;
35+
import javax.lang.model.type.DeclaredType;
36+
import javax.lang.model.type.TypeMirror;
37+
import javax.lang.model.util.ElementFilter;
38+
import javax.lang.model.util.SimpleTypeVisitor8;
39+
import javax.tools.JavaFileObject;
40+
41+
/**
42+
* Detects javac bugs that might prevent tests from working.
43+
*
44+
* @author [email protected] (Éamonn McManus)
45+
*/
46+
final class BugDetector {
47+
private BugDetector() {}
48+
49+
/**
50+
* Returns true if {@link TypeMirror#accept} gives the unannotated type to the type visitor. It
51+
* should obviously receive the type that {@code accept} was called on, but in at least some
52+
* Java 8 versions it ends up being the unannotated one.
53+
*/
54+
// I have not been able to find a reference for this bug.
55+
static boolean typeVisitorDropsAnnotations() {
56+
JavaFileObject testClass = JavaFileObjects.forSourceLines(
57+
"com.example.Test",
58+
"package com.example;",
59+
"",
60+
"import java.lang.annotation.*;",
61+
"",
62+
"abstract class Test {",
63+
" @Target(ElementType.TYPE_USE)",
64+
" @interface Nullable {}",
65+
"",
66+
" @Override public abstract boolean equals(@Nullable Object x);",
67+
"}");
68+
BugDetectorProcessor bugDetectorProcessor = new BugDetectorProcessor();
69+
Compilation compilation =
70+
Compiler.javac().withProcessors(bugDetectorProcessor).compile(testClass);
71+
assertThat(compilation).succeeded();
72+
return bugDetectorProcessor.typeAnnotationsNotReturned;
73+
}
74+
75+
@SupportedAnnotationTypes("*")
76+
@SupportedSourceVersion(SourceVersion.RELEASE_8)
77+
private static class BugDetectorProcessor extends AbstractProcessor {
78+
volatile boolean typeAnnotationsNotReturned;
79+
80+
@Override
81+
public boolean process(
82+
Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
83+
if (!roundEnv.processingOver()) {
84+
TypeElement test = processingEnv.getElementUtils().getTypeElement("com.example.Test");
85+
ExecutableElement equals = ElementFilter.methodsIn(test.getEnclosedElements()).get(0);
86+
assertThat(equals.getSimpleName().toString()).isEqualTo("equals");
87+
TypeMirror parameterType = equals.getParameters().get(0).asType();
88+
List<AnnotationMirror> annotationsFromVisitor =
89+
parameterType.accept(new BugDetectorVisitor(), null);
90+
typeAnnotationsNotReturned = annotationsFromVisitor.isEmpty();
91+
}
92+
return false;
93+
}
94+
95+
private static class BugDetectorVisitor
96+
extends SimpleTypeVisitor8<List<AnnotationMirror>, Void> {
97+
@Override
98+
public List<AnnotationMirror> visitDeclared(DeclaredType t, Void p) {
99+
return Collections.unmodifiableList(t.getAnnotationMirrors());
100+
}
101+
}
102+
}
103+
}

‎value/src/main/java/com/google/auto/value/processor/AutoOneOfProcessor.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,12 @@ void processType(TypeElement autoOneOfType) {
110110
vars.generatedClass = TypeSimplifier.simpleNameOf(subclass);
111111
vars.types = processingEnv.getTypeUtils();
112112
vars.propertyToKind = propertyToKind;
113-
Set<ObjectMethod> methodsToGenerate = determineObjectMethodsToGenerate(methods);
114-
vars.toString = methodsToGenerate.contains(ObjectMethod.TO_STRING);
115-
vars.equals = methodsToGenerate.contains(ObjectMethod.EQUALS);
116-
vars.hashCode = methodsToGenerate.contains(ObjectMethod.HASH_CODE);
113+
Map<ObjectMethod, ExecutableElement> methodsToGenerate =
114+
determineObjectMethodsToGenerate(methods);
115+
vars.toString = methodsToGenerate.containsKey(ObjectMethod.TO_STRING);
116+
vars.equals = methodsToGenerate.containsKey(ObjectMethod.EQUALS);
117+
vars.hashCode = methodsToGenerate.containsKey(ObjectMethod.HASH_CODE);
118+
vars.equalsParameterType = equalsParameterType(methodsToGenerate);
117119
defineVarsForType(autoOneOfType, vars, propertyMethods, kindGetter);
118120

119121
String text = vars.toText();

‎value/src/main/java/com/google/auto/value/processor/AutoOneOfTemplateVars.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,13 @@ class AutoOneOfTemplateVars extends TemplateVars {
4141
/** Whether to generate a toString() method. */
4242
Boolean toString;
4343

44+
/**
45+
* A string representing the parameter type declaration of the equals(Object) method, including
46+
* any annotations. If {@link #equals} is false, this field is ignored (but it must still be
47+
* non-null).
48+
*/
49+
String equalsParameterType;
50+
4451
/** The type utilities returned by {@link ProcessingEnvironment#getTypeUtils()}. */
4552
Types types;
4653

‎value/src/main/java/com/google/auto/value/processor/AutoValueOrOneOfProcessor.java‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.google.auto.value.processor;
1717

1818
import static com.google.auto.common.AnnotationMirrors.getAnnotationValue;
19+
import static com.google.common.collect.Iterables.getOnlyElement;
1920
import static java.util.stream.Collectors.joining;
2021
import static java.util.stream.Collectors.toList;
2122

@@ -33,7 +34,7 @@
3334
import java.util.ArrayList;
3435
import java.util.Arrays;
3536
import java.util.Collection;
36-
import java.util.EnumSet;
37+
import java.util.EnumMap;
3738
import java.util.HashSet;
3839
import java.util.LinkedHashMap;
3940
import java.util.List;
@@ -411,9 +412,14 @@ static ObjectMethod objectMethodToOverride(ExecutableElement method) {
411412
}
412413
break;
413414
case 1:
414-
if (name.equals("equals")
415-
&& method.getParameters().get(0).asType().toString().equals("java.lang.Object")) {
416-
return ObjectMethod.EQUALS;
415+
if (name.equals("equals")) {
416+
TypeMirror param = getOnlyElement(method.getParameters()).asType();
417+
if (param.getKind().equals(TypeKind.DECLARED)) {
418+
TypeElement paramType = MoreTypes.asTypeElement(param);
419+
if (paramType.getQualifiedName().contentEquals("java.lang.Object")) {
420+
return ObjectMethod.EQUALS;
421+
}
422+
}
417423
}
418424
break;
419425
default:
@@ -541,22 +547,39 @@ private static String disambiguate(String name, Collection<String> existingNames
541547
}
542548

543549
/**
544-
* Given a list of all methods defined in or inherited by a class, returns a set indicating
545-
* which of equals, hashCode, and toString should be generated.
550+
* Given a list of all methods defined in or inherited by a class, returns a map indicating
551+
* which of equals, hashCode, and toString should be generated. Each value in the map is
552+
* the method that will be overridden by the generated method, which might be a method in
553+
* {@code Object} or an abstract method in the {@code @AutoValue} class or an ancestor.
546554
*/
547-
static Set<ObjectMethod> determineObjectMethodsToGenerate(Set<ExecutableElement> methods) {
548-
Set<ObjectMethod> methodsToGenerate = EnumSet.noneOf(ObjectMethod.class);
555+
static Map<ObjectMethod, ExecutableElement>
556+
determineObjectMethodsToGenerate(Set<ExecutableElement> methods) {
557+
Map<ObjectMethod, ExecutableElement> methodsToGenerate = new EnumMap<>(ObjectMethod.class);
549558
for (ExecutableElement method : methods) {
550559
ObjectMethod override = objectMethodToOverride(method);
551560
boolean canGenerate = method.getModifiers().contains(Modifier.ABSTRACT)
552561
|| isJavaLangObject((TypeElement) method.getEnclosingElement());
553562
if (!override.equals(ObjectMethod.NONE) && canGenerate) {
554-
methodsToGenerate.add(override);
563+
methodsToGenerate.put(override, method);
555564
}
556565
}
557566
return methodsToGenerate;
558567
}
559568

569+
/**
570+
* Returns the encoded parameter type of the {@code equals(Object)} method that is to be
571+
* generated, or an empty string if the method is not being generated. The parameter type
572+
* includes any type annotations, for example {@code @Nullable}.
573+
*/
574+
static String equalsParameterType(Map<ObjectMethod, ExecutableElement> methodsToGenerate) {
575+
ExecutableElement equals = methodsToGenerate.get(ObjectMethod.EQUALS);
576+
if (equals == null) {
577+
return ""; // this will not be referenced because no equals method will be generated
578+
}
579+
TypeMirror parameterType = equals.getParameters().get(0).asType();
580+
return TypeEncoder.encodeWithAnnotations(parameterType);
581+
}
582+
560583
/**
561584
* Returns the subset of all abstract methods in the given set of methods. A given method
562585
* signature is only mentioned once, even if it is inherited on more than one path.

‎value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import java.util.Collections;
4848
import java.util.HashSet;
4949
import java.util.List;
50+
import java.util.Map;
5051
import java.util.Optional;
5152
import java.util.OptionalInt;
5253
import java.util.ServiceConfigurationError;
@@ -222,10 +223,12 @@ void processType(TypeElement type) {
222223
vars.types = processingEnv.getTypeUtils();
223224
vars.identifiers =
224225
!processingEnv.getOptions().containsKey("com.google.auto.value.OmitIdentifiers");
225-
Set<ObjectMethod> methodsToGenerate = determineObjectMethodsToGenerate(methods);
226-
vars.toString = methodsToGenerate.contains(ObjectMethod.TO_STRING);
227-
vars.equals = methodsToGenerate.contains(ObjectMethod.EQUALS);
228-
vars.hashCode = methodsToGenerate.contains(ObjectMethod.HASH_CODE);
226+
Map<ObjectMethod, ExecutableElement> methodsToGenerate =
227+
determineObjectMethodsToGenerate(methods);
228+
vars.toString = methodsToGenerate.containsKey(ObjectMethod.TO_STRING);
229+
vars.hashCode = methodsToGenerate.containsKey(ObjectMethod.HASH_CODE);
230+
vars.equals = methodsToGenerate.containsKey(ObjectMethod.EQUALS);
231+
vars.equalsParameterType = equalsParameterType(methodsToGenerate);
229232
defineVarsForType(type, vars, toBuilderMethods, propertyMethods, builder);
230233

231234
// Only copy annotations from a class if it has @AutoValue.CopyAnnotations.

‎value/src/main/java/com/google/auto/value/processor/AutoValueTemplateVars.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ class AutoValueTemplateVars extends TemplateVars {
4545
/** Whether to generate a toString() method. */
4646
Boolean toString;
4747

48+
/**
49+
* A string representing the parameter type declaration of the equals(Object) method, including
50+
* any annotations. If {@link #equals} is false, this field is ignored (but it must still be
51+
* non-null).
52+
*/
53+
String equalsParameterType;
54+
4855
/**
4956
* Whether to include identifiers in strings in the generated code. If false, exception messages
5057
* will not mention properties by name, and {@code toString()} will include neither property

‎value/src/main/java/com/google/auto/value/processor/autooneof.vm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ final class $generatedClass {
112112
#if ($equals)
113113

114114
@Override
115-
public boolean equals(Object x) {
115+
public boolean equals($equalsParameterType x) {
116116
if (x instanceof $origClass) {
117117
$origClass$wildcardTypes that = ($origClass$wildcardTypes) x;
118118
return this.${kindGetter}() == that.${kindGetter}()

‎value/src/main/java/com/google/auto/value/processor/autovalue.vm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ $a
125125
#if ($equals)
126126

127127
@Override
128-
public boolean equals(`java.lang.Object` o) {
128+
public boolean equals($equalsParameterType o) {
129129
if (o == this) {
130130
return true;
131131
}

0 commit comments

Comments
 (0)