From bf27cabfc366b87f8e98d8242e96acf8e3eea08b Mon Sep 17 00:00:00 2001 From: stabai Date: Mon, 25 Mar 2019 10:51:39 -0700 Subject: [PATCH 1/5] Support @CopyAnnotations in classes generated by MemoizeExtension. Note: Several methods are copied from the com.google.auto.value.processor that should be moved to a common location. RELNOTES=Support @CopyAnnotations in classes generated by MemoizeExtension. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=240174112 --- .../memoized/processor/MemoizeExtension.java | 168 +++++++++++++++++- .../extension/memoized/MemoizedTest.java | 43 +++++ 2 files changed, 209 insertions(+), 2 deletions(-) diff --git a/value/src/main/java/com/google/auto/value/extension/memoized/processor/MemoizeExtension.java b/value/src/main/java/com/google/auto/value/extension/memoized/processor/MemoizeExtension.java index d9ccf1fcac..9f2f287a92 100644 --- a/value/src/main/java/com/google/auto/value/extension/memoized/processor/MemoizeExtension.java +++ b/value/src/main/java/com/google/auto/value/extension/memoized/processor/MemoizeExtension.java @@ -15,17 +15,24 @@ */ package com.google.auto.value.extension.memoized.processor; +import static com.google.auto.common.AnnotationMirrors.getAnnotationValue; import static com.google.auto.common.GeneratedAnnotationSpecs.generatedAnnotationSpec; +import static com.google.auto.common.MoreElements.getPackage; +import static com.google.auto.common.MoreElements.isAnnotationPresent; import static com.google.auto.value.extension.memoized.processor.ClassNames.MEMOIZED_NAME; import static com.google.auto.value.extension.memoized.processor.MemoizedValidator.getAnnotationMirror; import static com.google.common.base.Predicates.equalTo; import static com.google.common.base.Predicates.not; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Iterables.filter; import static com.google.common.collect.Iterables.getOnlyElement; +import static com.google.common.collect.Sets.union; import static com.squareup.javapoet.MethodSpec.constructorBuilder; import static com.squareup.javapoet.MethodSpec.methodBuilder; import static com.squareup.javapoet.TypeSpec.classBuilder; import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; import static javax.lang.model.element.Modifier.ABSTRACT; import static javax.lang.model.element.Modifier.FINAL; import static javax.lang.model.element.Modifier.PRIVATE; @@ -38,8 +45,11 @@ import static javax.tools.Diagnostic.Kind.ERROR; import com.google.auto.common.MoreElements; +import com.google.auto.common.MoreTypes; +import com.google.auto.common.Visibility; import com.google.auto.service.AutoService; import com.google.auto.value.extension.AutoValueExtension; +import com.google.common.base.Equivalence.Wrapper; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -53,16 +63,21 @@ import com.squareup.javapoet.TypeName; import com.squareup.javapoet.TypeSpec; import com.squareup.javapoet.TypeVariableName; +import java.lang.annotation.Inherited; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import javax.annotation.processing.Messager; import javax.annotation.processing.ProcessingEnvironment; import javax.lang.model.SourceVersion; import javax.lang.model.element.AnnotationMirror; +import javax.lang.model.element.AnnotationValue; +import javax.lang.model.element.Element; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.Modifier; +import javax.lang.model.element.QualifiedNameable; import javax.lang.model.element.TypeElement; import javax.lang.model.element.TypeParameterElement; import javax.lang.model.type.TypeMirror; @@ -70,12 +85,20 @@ import javax.lang.model.util.Types; import javax.tools.Diagnostic.Kind; -/** An extension that implements the {@link Memoized} contract. */ +/** + * An extension that implements the {@link com.google.auto.value.extension.memoized.Memoized} + * contract. + */ @AutoService(AutoValueExtension.class) public final class MemoizeExtension extends AutoValueExtension { private static final ImmutableSet DO_NOT_PULL_DOWN_ANNOTATIONS = ImmutableSet.of(Override.class.getCanonicalName(), MEMOIZED_NAME); + // TODO(b/122509249): Move code copied from com.google.auto.value.processor to auto-common. + private static final String AUTO_VALUE_PACKAGE_NAME = "com.google.auto.value."; + private static final String AUTO_VALUE_NAME = AUTO_VALUE_PACKAGE_NAME + "AutoValue"; + private static final String COPY_ANNOTATIONS_NAME = AUTO_VALUE_NAME + ".CopyAnnotations"; + private static final ClassName LAZY_INIT = ClassName.get("com.google.errorprone.annotations.concurrent", "LazyInit"); @@ -136,6 +159,7 @@ String generate() { TypeSpec.Builder generated = classBuilder(className) .superclass(superType()) + .addAnnotations(copiedClassAnnotations(context.autoValueClass())) .addTypeVariables(typeVariableNames()) .addModifiers(isFinal ? FINAL : ABSTRACT) .addMethod(constructor()); @@ -223,7 +247,147 @@ private MethodSpec equalsWithHashCodeCheck() { } /** - * Determines the required fields and overriding method for a {@link Memoized @Memoized} method. + * True if the given class name is in the com.google.auto.value package or a subpackage. False + * if the class name contains {@code Test}, since many AutoValue tests under + * com.google.auto.value define their own annotations. + */ + // TODO(b/122509249): Move code copied from com.google.auto.value.processor to auto-common. + private boolean isInAutoValuePackage(String className) { + return className.startsWith(AUTO_VALUE_PACKAGE_NAME) && !className.contains("Test"); + } + + /** + * Returns the fully-qualified name of an annotation-mirror, e.g. + * "com.google.auto.value.AutoValue". + */ + // TODO(b/122509249): Move code copied from com.google.auto.value.processor to auto-common. + private static String getAnnotationFqName(AnnotationMirror annotation) { + return ((QualifiedNameable) annotation.getAnnotationType().asElement()) + .getQualifiedName() + .toString(); + } + + // TODO(b/122509249): Move code copied from com.google.auto.value.processor to auto-common. + private boolean annotationVisibleFrom(AnnotationMirror annotation, Element from) { + Element annotationElement = annotation.getAnnotationType().asElement(); + Visibility visibility = Visibility.effectiveVisibilityOfElement(annotationElement); + switch (visibility) { + case PUBLIC: + return true; + case PROTECTED: + // If the annotation is protected, it must be inside another class, call it C. If our + // @AutoValue class is Foo then, for the annotation to be visible, either Foo must be in + // the same package as C or Foo must be a subclass of C. If the annotation is visible from + // Foo then it is also visible from our generated subclass AutoValue_Foo. + // The protected case only applies to method annotations. An annotation on the + // AutoValue_Foo class itself can't be protected, even if AutoValue_Foo ultimately + // inherits from the class that defines the annotation. The JLS says "Access is permitted + // only within the body of a subclass": + // https://docs.oracle.com/javase/specs/jls/se8/html/jls-6.html#jls-6.6.2.1 + // AutoValue_Foo is a top-level class, so an annotation on it cannot be in the body of a + // subclass of anything. + return getPackage(annotationElement).equals(getPackage(from)) + || types.isSubtype(from.asType(), annotationElement.getEnclosingElement().asType()); + case DEFAULT: + return getPackage(annotationElement).equals(getPackage(from)); + default: + return false; + } + } + + /** Implements the semantics of {@code AutoValue.CopyAnnotations}; see its javadoc. */ + // TODO(b/122509249): Move code copied from com.google.auto.value.processor to auto-common. + private ImmutableList annotationsToCopy( + Element autoValueType, Element typeOrMethod, Set excludedAnnotations) { + ImmutableList.Builder result = ImmutableList.builder(); + for (AnnotationMirror annotation : typeOrMethod.getAnnotationMirrors()) { + String annotationFqName = getAnnotationFqName(annotation); + // To be included, the annotation should not be in com.google.auto.value, + // and it should not be in the excludedAnnotations set. + if (!isInAutoValuePackage(annotationFqName) + && !excludedAnnotations.contains(annotationFqName) + && annotationVisibleFrom(annotation, autoValueType)) { + result.add(annotation); + } + } + + return result.build(); + } + + /** Implements the semantics of {@code AutoValue.CopyAnnotations}; see its javadoc. */ + // TODO(b/122509249): Move code copied from com.google.auto.value.processor to auto-common. + private ImmutableList copyAnnotations( + Element autoValueType, Element typeOrMethod, Set excludedAnnotations) { + ImmutableList annotationsToCopy = + annotationsToCopy(autoValueType, typeOrMethod, excludedAnnotations); + return annotationsToCopy.stream().map(AnnotationSpec::get).collect(toImmutableList()); + } + + // TODO(b/122509249): Move code copied from com.google.auto.value.processor to auto-common. + private static boolean hasAnnotationMirror(Element element, String annotationName) { + return getAnnotationMirror(element, annotationName).isPresent(); + } + + /** + * Returns the contents of the {@code AutoValue.CopyAnnotations.exclude} element, as a set of + * {@code TypeMirror} where each type is an annotation type. + */ + // TODO(b/122509249): Move code copied from com.google.auto.value.processor to auto-common. + private ImmutableSet getExcludedAnnotationTypes(Element element) { + Optional maybeAnnotation = + getAnnotationMirror(element, COPY_ANNOTATIONS_NAME); + if (!maybeAnnotation.isPresent()) { + return ImmutableSet.of(); + } + + @SuppressWarnings("unchecked") + List excludedClasses = + (List) getAnnotationValue(maybeAnnotation.get(), "exclude").getValue(); + return excludedClasses.stream() + .map( + annotationValue -> + MoreTypes.equivalence().wrap((TypeMirror) annotationValue.getValue())) + // TODO(b/122509249): Move TypeMirrorSet to common package instead of doing this. + .distinct() + .map(Wrapper::get) + .collect(toImmutableSet()); + } + + /** + * Returns the contents of the {@code AutoValue.CopyAnnotations.exclude} element, as a set of + * strings that are fully-qualified class names. + */ + // TODO(b/122509249): Move code copied from com.google.auto.value.processor to auto-common. + private Set getExcludedAnnotationClassNames(Element element) { + return getExcludedAnnotationTypes(element).stream() + .map(MoreTypes::asTypeElement) + .map(typeElement -> typeElement.getQualifiedName().toString()) + .collect(toSet()); + } + + // TODO(b/122509249): Move code copied from com.google.auto.value.processor to auto-common. + private static Set getAnnotationsMarkedWithInherited(Element element) { + return element.getAnnotationMirrors().stream() + .filter(a -> isAnnotationPresent(a.getAnnotationType().asElement(), Inherited.class)) + .map(Generator::getAnnotationFqName) + .collect(toSet()); + } + + private ImmutableList copiedClassAnnotations(TypeElement type) { + // Only copy annotations from a class if it has @AutoValue.CopyAnnotations. + if (hasAnnotationMirror(type, COPY_ANNOTATIONS_NAME)) { + Set excludedAnnotations = + union(getExcludedAnnotationClassNames(type), getAnnotationsMarkedWithInherited(type)); + + return copyAnnotations(type, type, excludedAnnotations); + } else { + return ImmutableList.of(); + } + } + + /** + * Determines the required fields and overriding method for a {@link + * com.google.auto.value.extension.memoized.Memoized @Memoized} method. */ private final class MethodOverrider { private final ExecutableElement method; diff --git a/value/src/test/java/com/google/auto/value/extension/memoized/MemoizedTest.java b/value/src/test/java/com/google/auto/value/extension/memoized/MemoizedTest.java index bc4895e8b6..57ec8c7c4e 100644 --- a/value/src/test/java/com/google/auto/value/extension/memoized/MemoizedTest.java +++ b/value/src/test/java/com/google/auto/value/extension/memoized/MemoizedTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.fail; import com.google.auto.value.AutoValue; +import com.google.auto.value.AutoValue.CopyAnnotations; import com.google.auto.value.extension.memoized.MemoizedTest.HashCodeEqualsOptimization.EqualsCounter; import com.google.common.collect.ImmutableList; import java.lang.reflect.AnnotatedType; @@ -52,6 +53,29 @@ boolean getMemoizedNative0() { } } + @AutoValue + @CopyAnnotations + @javax.annotation.Nullable + abstract static class ValueWithCopyAnnotations { + abstract boolean getNative(); + + @Memoized + boolean getMemoizedNative() { + return getNative(); + } + } + + @AutoValue + @javax.annotation.Nullable + abstract static class ValueWithoutCopyAnnotations { + abstract boolean getNative(); + + @Memoized + boolean getMemoizedNative() { + return getNative(); + } + } + @AutoValue abstract static class Value { private int primitiveCount; @@ -320,6 +344,25 @@ public void keywords() { assertThat(value.getMemoizedNative0()).isFalse(); } + @Test + public void copyAnnotations() { + ValueWithCopyAnnotations valueWithCopyAnnotations = + new AutoValue_MemoizedTest_ValueWithCopyAnnotations(true); + ValueWithoutCopyAnnotations valueWithoutCopyAnnotations = + new AutoValue_MemoizedTest_ValueWithoutCopyAnnotations(true); + + assertThat( + valueWithCopyAnnotations + .getClass() + .isAnnotationPresent(javax.annotation.Nullable.class)) + .isTrue(); + assertThat( + valueWithoutCopyAnnotations + .getClass() + .isAnnotationPresent(javax.annotation.Nullable.class)) + .isFalse(); + } + @Test public void nullableHasAnnotation() throws ReflectiveOperationException { Method nullable = AutoValue_MemoizedTest_Value.class.getDeclaredMethod("nullable"); From cf61cff62b6f822604d09620f6997097d5ba57e2 Mon Sep 17 00:00:00 2001 From: ronshapiro Date: Tue, 26 Mar 2019 20:17:37 -0700 Subject: [PATCH 2/5] Move the META-INF/gradle resources into auto/service/processor. I missed this when first doing the migration ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=240483232 --- .../resources/META-INF/gradle/incremental.annotation.processors | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename service/{ => processor}/src/main/resources/META-INF/gradle/incremental.annotation.processors (100%) diff --git a/service/src/main/resources/META-INF/gradle/incremental.annotation.processors b/service/processor/src/main/resources/META-INF/gradle/incremental.annotation.processors similarity index 100% rename from service/src/main/resources/META-INF/gradle/incremental.annotation.processors rename to service/processor/src/main/resources/META-INF/gradle/incremental.annotation.processors From a81a9e253f28ee7480671dde3f2bf53589e19458 Mon Sep 17 00:00:00 2001 From: nhcohen Date: Thu, 28 Mar 2019 06:49:47 -0700 Subject: [PATCH 3/5] Add sentence to AutoValue user guide to explain where the order of the constructor parameters comes from. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=240764093 --- value/userguide/index.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/value/userguide/index.md b/value/userguide/index.md index b147b12e14..0b72e14168 100644 --- a/value/userguide/index.md +++ b/value/userguide/index.md @@ -65,6 +65,9 @@ abstract class Animal { } ``` +The constructor parameters correspond, in order, to the abstract accessor +methods. + Note that in real life, some classes and methods would presumably be public and have Javadoc. We're leaving these off in the User Guide only to keep the examples short and simple. From f17d298a901683c0d1b0a05fa093a5936c7abcc5 Mon Sep 17 00:00:00 2001 From: emcmanus Date: Thu, 28 Mar 2019 14:57:05 -0700 Subject: [PATCH 4/5] Expand the exceptions covered by the workaround for a JDK8 jar bug. Fixes https://github.com/google/auto/issues/715. RELNOTES=Expand the exceptions covered by the workaround for a JDK8 jar bug. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=240859625 --- .../auto/value/processor/TemplateVars.java | 5 +++-- .../auto/value/processor/TemplateVarsTest.java | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/value/src/main/java/com/google/auto/value/processor/TemplateVars.java b/value/src/main/java/com/google/auto/value/processor/TemplateVars.java index a92f8bcaf0..7bb26a1ec3 100644 --- a/value/src/main/java/com/google/auto/value/processor/TemplateVars.java +++ b/value/src/main/java/com/google/auto/value/processor/TemplateVars.java @@ -119,8 +119,9 @@ static Template parsedTemplateForResource(String resourceName) { return Template.parseFrom(resourceName, TemplateVars::readerFromResource); } catch (UnsupportedEncodingException e) { throw new AssertionError(e); - } catch (IOException | NullPointerException e) { - // https://github.com/google/auto/pull/439 says that we can also get NullPointerException. + } catch (IOException | NullPointerException | IllegalStateException e) { + // https://github.com/google/auto/pull/439 says that we can get NullPointerException. + // https://github.com/google/auto/issues/715 says that we can get IllegalStateException return retryParseAfterException(resourceName, e); } } diff --git a/value/src/test/java/com/google/auto/value/processor/TemplateVarsTest.java b/value/src/test/java/com/google/auto/value/processor/TemplateVarsTest.java index b30fee44ca..85f5accc6b 100644 --- a/value/src/test/java/com/google/auto/value/processor/TemplateVarsTest.java +++ b/value/src/test/java/com/google/auto/value/processor/TemplateVarsTest.java @@ -183,14 +183,20 @@ public void testBrokenInputStream_NullPointerException() throws Exception { doTestBrokenInputStream(new NullPointerException("BrokenInputStream")); } + @Test + public void testBrokenInputStream_IllegalStateException() throws Exception { + doTestBrokenInputStream(new IllegalStateException("BrokenInputStream")); + } + // This is a complicated test that tries to simulates the failures that are worked around in // Template.parsedTemplateForResource. Those failures means that the InputStream returned by - // ClassLoader.getResourceAsStream sometimes throws IOException or NullPointerException while it - // is being read. To simulate that, we make a second ClassLoader with the same configuration as - // the one that runs this test, and we override getResourceAsStream so that it wraps the returned - // InputStream in a BrokenInputStream, which throws an exception after a certain number of - // characters. We check that that exception was indeed seen, and that we did indeed try to read - // the resource we're interested in, and that we succeeded in loading a Template nevertheless. + // ClassLoader.getResourceAsStream sometimes throws IOException or NullPointerException or + // IllegalStateException while it is being read. To simulate that, we make a second ClassLoader + // with the same configuration as the one that runs this test, and we override getResourceAsStream + // so that it wraps the returned InputStream in a BrokenInputStream, which throws an exception + // after a certain number of characters. We check that that exception was indeed seen, and that + // we did indeed try to read the resource we're interested in, and that we succeeded in loading a + // Template nevertheless. private void doTestBrokenInputStream(Exception exception) throws Exception { URLClassLoader shadowLoader = new ShadowLoader(getClass().getClassLoader(), exception); Runnable brokenInputStreamTest = From 976d6c56eb1fb8b7c0b3b304e7ec2ba62b74955a Mon Sep 17 00:00:00 2001 From: ronshapiro Date: Fri, 29 Mar 2019 07:44:36 -0700 Subject: [PATCH 5/5] Set the nested AutoService pom's to have the same parent (auto-service-aggregator). This allows mvn versions:set to set each of them at the same time. It also allows them to be released in tandem. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=240976869 --- service/annotations/pom.xml | 6 +++--- service/processor/pom.xml | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/service/annotations/pom.xml b/service/annotations/pom.xml index 5cf91b3e31..36ef5de42d 100644 --- a/service/annotations/pom.xml +++ b/service/annotations/pom.xml @@ -19,9 +19,9 @@ 4.0.0 - com.google.auto - auto-parent - 7 + com.google.auto.service + auto-service-aggregator + HEAD-SNAPSHOT com.google.auto.service diff --git a/service/processor/pom.xml b/service/processor/pom.xml index 029072eb56..7f381bc58e 100644 --- a/service/processor/pom.xml +++ b/service/processor/pom.xml @@ -19,9 +19,9 @@ 4.0.0 - com.google.auto - auto-parent - 7 + com.google.auto.service + auto-service-aggregator + HEAD-SNAPSHOT com.google.auto.service