Skip to content

Commit

Permalink
Copy type annotations to the parameter of equals(Object).
Browse files Browse the repository at this point in the history
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
  • Loading branch information
eamonnmcmanus authored and ronshapiro committed Mar 7, 2018
1 parent 9ebbeb4 commit 1561e2c
Show file tree
Hide file tree
Showing 10 changed files with 279 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright (C) 2018 Google, Inc.
*
* 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.
*/
package com.google.auto.value;

import static com.google.common.truth.Truth.assertThat;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.AnnotatedType;
import java.lang.reflect.Method;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/**
* Tests for Java8-specific {@code @AutoOneOf} behaviour.
*
* @author [email protected] (Éamonn McManus)
*/
@RunWith(JUnit4.class)
public class AutoOneOfJava8Test {
@AutoOneOf(EqualsNullable.Kind.class)
public abstract static class EqualsNullable {

@Target(ElementType.TYPE_USE)
@Retention(RetentionPolicy.RUNTIME)
public @interface Nullable {}

public enum Kind {THING}
public abstract Kind kind();
public abstract String thing();

public static EqualsNullable ofThing(String thing) {
return AutoOneOf_AutoOneOfJava8Test_EqualsNullable.thing(thing);
}

@Override
public abstract boolean equals(@Nullable Object x);

@Override
public abstract int hashCode();
}

/**
* Tests that a type annotation on the parameter of {@code equals(Object)} is copied into the
* implementation class.
*/
@Test
public void equalsNullable() throws ReflectiveOperationException {
if (BugDetector.typeVisitorDropsAnnotations()) {
System.err.println("TYPE VISITORS DO NOT SEE TYPE ANNOTATIONS, SKIPPING TEST");
return;
}
EqualsNullable x = EqualsNullable.ofThing("foo");
Class<? extends EqualsNullable> c = x.getClass();
Method equals = c.getMethod("equals", Object.class);
assertThat(equals.getDeclaringClass()).isNotSameAs(EqualsNullable.class);
AnnotatedType parameterType = equals.getAnnotatedParameterTypes()[0];
assertThat(parameterType.isAnnotationPresent(EqualsNullable.Nullable.class))
.isTrue();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.AnnotatedType;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.util.Arrays;
Expand Down Expand Up @@ -512,4 +513,41 @@ public void testFunkyNullable() {
FunkyNullable implicitNull = FunkyNullable.builder().build();
assertThat(explicitNull).isEqualTo(implicitNull);
}

@AutoValue
abstract static class EqualsNullable {
@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER})
@Retention(RetentionPolicy.RUNTIME)
@interface Nullable {}

abstract String foo();

static EqualsNullable create(String foo) {
return new AutoValue_AutoValueJava8Test_EqualsNullable(foo);
}

@Override
public abstract boolean equals(@Nullable Object x);

@Override
public abstract int hashCode();
}

/**
* Tests that a type annotation on the parameter of {@code equals(Object)} is copied into the
* implementation class.
*/
@Test
public void testEqualsNullable() throws ReflectiveOperationException {
if (BugDetector.typeVisitorDropsAnnotations()) {
System.err.println("TYPE VISITORS DO NOT SEE TYPE ANNOTATIONS, SKIPPING TEST");
return;
}
EqualsNullable x = EqualsNullable.create("foo");
Class<? extends EqualsNullable> implClass = x.getClass();
Method equals = implClass.getDeclaredMethod("equals", Object.class);
AnnotatedType[] parameterTypes = equals.getAnnotatedParameterTypes();
assertThat(parameterTypes[0].isAnnotationPresent(EqualsNullable.Nullable.class))
.isTrue();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Copyright (C) 2018 Google, Inc.
*
* 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.
*/
package com.google.auto.value;

import static com.google.common.truth.Truth.assertThat;
import static com.google.testing.compile.CompilationSubject.assertThat;

import com.google.testing.compile.Compilation;
import com.google.testing.compile.Compiler;
import com.google.testing.compile.JavaFileObjects;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import javax.annotation.processing.AbstractProcessor;
import javax.annotation.processing.RoundEnvironment;
import javax.annotation.processing.SupportedAnnotationTypes;
import javax.annotation.processing.SupportedSourceVersion;
import javax.lang.model.SourceVersion;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.ElementFilter;
import javax.lang.model.util.SimpleTypeVisitor8;
import javax.tools.JavaFileObject;

/**
* Detects javac bugs that might prevent tests from working.
*
* @author [email protected] (Éamonn McManus)
*/
final class BugDetector {
private BugDetector() {}

/**
* Returns true if {@link TypeMirror#accept} gives the unannotated type to the type visitor. It
* should obviously receive the type that {@code accept} was called on, but in at least some
* Java 8 versions it ends up being the unannotated one.
*/
// I have not been able to find a reference for this bug.
static boolean typeVisitorDropsAnnotations() {
JavaFileObject testClass = JavaFileObjects.forSourceLines(
"com.example.Test",
"package com.example;",
"",
"import java.lang.annotation.*;",
"",
"abstract class Test {",
" @Target(ElementType.TYPE_USE)",
" @interface Nullable {}",
"",
" @Override public abstract boolean equals(@Nullable Object x);",
"}");
BugDetectorProcessor bugDetectorProcessor = new BugDetectorProcessor();
Compilation compilation =
Compiler.javac().withProcessors(bugDetectorProcessor).compile(testClass);
assertThat(compilation).succeeded();
return bugDetectorProcessor.typeAnnotationsNotReturned;
}

@SupportedAnnotationTypes("*")
@SupportedSourceVersion(SourceVersion.RELEASE_8)
private static class BugDetectorProcessor extends AbstractProcessor {
volatile boolean typeAnnotationsNotReturned;

@Override
public boolean process(
Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
if (!roundEnv.processingOver()) {
TypeElement test = processingEnv.getElementUtils().getTypeElement("com.example.Test");
ExecutableElement equals = ElementFilter.methodsIn(test.getEnclosedElements()).get(0);
assertThat(equals.getSimpleName().toString()).isEqualTo("equals");
TypeMirror parameterType = equals.getParameters().get(0).asType();
List<AnnotationMirror> annotationsFromVisitor =
parameterType.accept(new BugDetectorVisitor(), null);
typeAnnotationsNotReturned = annotationsFromVisitor.isEmpty();
}
return false;
}

private static class BugDetectorVisitor
extends SimpleTypeVisitor8<List<AnnotationMirror>, Void> {
@Override
public List<AnnotationMirror> visitDeclared(DeclaredType t, Void p) {
return Collections.unmodifiableList(t.getAnnotationMirrors());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,12 @@ void processType(TypeElement autoOneOfType) {
vars.generatedClass = TypeSimplifier.simpleNameOf(subclass);
vars.types = processingEnv.getTypeUtils();
vars.propertyToKind = propertyToKind;
Set<ObjectMethod> methodsToGenerate = determineObjectMethodsToGenerate(methods);
vars.toString = methodsToGenerate.contains(ObjectMethod.TO_STRING);
vars.equals = methodsToGenerate.contains(ObjectMethod.EQUALS);
vars.hashCode = methodsToGenerate.contains(ObjectMethod.HASH_CODE);
Map<ObjectMethod, ExecutableElement> methodsToGenerate =
determineObjectMethodsToGenerate(methods);
vars.toString = methodsToGenerate.containsKey(ObjectMethod.TO_STRING);
vars.equals = methodsToGenerate.containsKey(ObjectMethod.EQUALS);
vars.hashCode = methodsToGenerate.containsKey(ObjectMethod.HASH_CODE);
vars.equalsParameterType = equalsParameterType(methodsToGenerate);
defineVarsForType(autoOneOfType, vars, propertyMethods, kindGetter);

String text = vars.toText();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ class AutoOneOfTemplateVars extends TemplateVars {
/** Whether to generate a toString() method. */
Boolean toString;

/**
* A string representing the parameter type declaration of the equals(Object) method, including
* any annotations. If {@link #equals} is false, this field is ignored (but it must still be
* non-null).
*/
String equalsParameterType;

/** The type utilities returned by {@link ProcessingEnvironment#getTypeUtils()}. */
Types types;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.auto.value.processor;

import static com.google.auto.common.AnnotationMirrors.getAnnotationValue;
import static com.google.common.collect.Iterables.getOnlyElement;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;

Expand All @@ -33,7 +34,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.EnumSet;
import java.util.EnumMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
Expand Down Expand Up @@ -411,9 +412,14 @@ static ObjectMethod objectMethodToOverride(ExecutableElement method) {
}
break;
case 1:
if (name.equals("equals")
&& method.getParameters().get(0).asType().toString().equals("java.lang.Object")) {
return ObjectMethod.EQUALS;
if (name.equals("equals")) {
TypeMirror param = getOnlyElement(method.getParameters()).asType();
if (param.getKind().equals(TypeKind.DECLARED)) {
TypeElement paramType = MoreTypes.asTypeElement(param);
if (paramType.getQualifiedName().contentEquals("java.lang.Object")) {
return ObjectMethod.EQUALS;
}
}
}
break;
default:
Expand Down Expand Up @@ -541,22 +547,39 @@ private static String disambiguate(String name, Collection<String> existingNames
}

/**
* Given a list of all methods defined in or inherited by a class, returns a set indicating
* which of equals, hashCode, and toString should be generated.
* Given a list of all methods defined in or inherited by a class, returns a map indicating
* which of equals, hashCode, and toString should be generated. Each value in the map is
* the method that will be overridden by the generated method, which might be a method in
* {@code Object} or an abstract method in the {@code @AutoValue} class or an ancestor.
*/
static Set<ObjectMethod> determineObjectMethodsToGenerate(Set<ExecutableElement> methods) {
Set<ObjectMethod> methodsToGenerate = EnumSet.noneOf(ObjectMethod.class);
static Map<ObjectMethod, ExecutableElement>
determineObjectMethodsToGenerate(Set<ExecutableElement> methods) {
Map<ObjectMethod, ExecutableElement> methodsToGenerate = new EnumMap<>(ObjectMethod.class);
for (ExecutableElement method : methods) {
ObjectMethod override = objectMethodToOverride(method);
boolean canGenerate = method.getModifiers().contains(Modifier.ABSTRACT)
|| isJavaLangObject((TypeElement) method.getEnclosingElement());
if (!override.equals(ObjectMethod.NONE) && canGenerate) {
methodsToGenerate.add(override);
methodsToGenerate.put(override, method);
}
}
return methodsToGenerate;
}

/**
* Returns the encoded parameter type of the {@code equals(Object)} method that is to be
* generated, or an empty string if the method is not being generated. The parameter type
* includes any type annotations, for example {@code @Nullable}.
*/
static String equalsParameterType(Map<ObjectMethod, ExecutableElement> methodsToGenerate) {
ExecutableElement equals = methodsToGenerate.get(ObjectMethod.EQUALS);
if (equals == null) {
return ""; // this will not be referenced because no equals method will be generated
}
TypeMirror parameterType = equals.getParameters().get(0).asType();
return TypeEncoder.encodeWithAnnotations(parameterType);
}

/**
* Returns the subset of all abstract methods in the given set of methods. A given method
* signature is only mentioned once, even if it is inherited on more than one path.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.OptionalInt;
import java.util.ServiceConfigurationError;
Expand Down Expand Up @@ -222,10 +223,12 @@ void processType(TypeElement type) {
vars.types = processingEnv.getTypeUtils();
vars.identifiers =
!processingEnv.getOptions().containsKey("com.google.auto.value.OmitIdentifiers");
Set<ObjectMethod> methodsToGenerate = determineObjectMethodsToGenerate(methods);
vars.toString = methodsToGenerate.contains(ObjectMethod.TO_STRING);
vars.equals = methodsToGenerate.contains(ObjectMethod.EQUALS);
vars.hashCode = methodsToGenerate.contains(ObjectMethod.HASH_CODE);
Map<ObjectMethod, ExecutableElement> methodsToGenerate =
determineObjectMethodsToGenerate(methods);
vars.toString = methodsToGenerate.containsKey(ObjectMethod.TO_STRING);
vars.hashCode = methodsToGenerate.containsKey(ObjectMethod.HASH_CODE);
vars.equals = methodsToGenerate.containsKey(ObjectMethod.EQUALS);
vars.equalsParameterType = equalsParameterType(methodsToGenerate);
defineVarsForType(type, vars, toBuilderMethods, propertyMethods, builder);

// Only copy annotations from a class if it has @AutoValue.CopyAnnotations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ class AutoValueTemplateVars extends TemplateVars {
/** Whether to generate a toString() method. */
Boolean toString;

/**
* A string representing the parameter type declaration of the equals(Object) method, including
* any annotations. If {@link #equals} is false, this field is ignored (but it must still be
* non-null).
*/
String equalsParameterType;

/**
* Whether to include identifiers in strings in the generated code. If false, exception messages
* will not mention properties by name, and {@code toString()} will include neither property
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ final class $generatedClass {
#if ($equals)

@Override
public boolean equals(Object x) {
public boolean equals($equalsParameterType x) {
if (x instanceof $origClass) {
$origClass$wildcardTypes that = ($origClass$wildcardTypes) x;
return this.${kindGetter}() == that.${kindGetter}()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ $a
#if ($equals)

@Override
public boolean equals(`java.lang.Object` o) {
public boolean equals($equalsParameterType o) {
if (o == this) {
return true;
}
Expand Down

0 comments on commit 1561e2c

Please sign in to comment.