From 56bf69900069f73ff5578a9aa79ed5de1e70e695 Mon Sep 17 00:00:00 2001 From: Leland Takamine Date: Tue, 14 May 2019 13:29:08 -0700 Subject: [PATCH] Update SuperficialValidation to validate supertypes and superinterfaces recursively. --- .../auto/common/SuperficialValidation.java | 61 ++++--- .../common/SuperficialValidationTest.java | 149 ++++++++++++++++++ 2 files changed, 191 insertions(+), 19 deletions(-) diff --git a/common/src/main/java/com/google/auto/common/SuperficialValidation.java b/common/src/main/java/com/google/auto/common/SuperficialValidation.java index a7b8ebcfa9..6319a9782f 100644 --- a/common/src/main/java/com/google/auto/common/SuperficialValidation.java +++ b/common/src/main/java/com/google/auto/common/SuperficialValidation.java @@ -15,8 +15,11 @@ */ package com.google.auto.common; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.AnnotationValue; import javax.lang.model.element.AnnotationValueVisitor; @@ -46,16 +49,29 @@ * @author Gregory Kick */ public final class SuperficialValidation { + + private final Set visited = new HashSet<>(); + + private SuperficialValidation() {} + public static boolean validateElements(Iterable elements) { + return new SuperficialValidation().validateElementsInternal(elements); + } + + public static boolean validateElement(Element element) { + return new SuperficialValidation().validateElementInternal(element); + } + + private boolean validateElementsInternal(Iterable elements) { for (Element element : elements) { - if (!validateElement(element)) { + if (!validateElementInternal(element)) { return false; } } return true; } - private static final ElementVisitor ELEMENT_VALIDATING_VISITOR = + private final ElementVisitor ELEMENT_VALIDATING_VISITOR = new AbstractElementVisitor6() { @Override public Boolean visitPackage(PackageElement e, Void p) { // don't validate enclosed elements because it will return types in the package @@ -63,10 +79,13 @@ public static boolean validateElements(Iterable elements) { } @Override public Boolean visitType(TypeElement e, Void p) { + TypeMirror superclass = e.getSuperclass(); return isValidBaseElement(e) - && validateElements(e.getTypeParameters()) + && validateElementsInternal(e.getTypeParameters()) && validateTypes(e.getInterfaces()) - && validateType(e.getSuperclass()); + && validateType(superclass) + && e.getInterfaces().stream().map(MoreTypes::asElement).allMatch(i -> validateElementInternal(i)) + && (superclass.getKind() == TypeKind.NONE || validateElementInternal(MoreTypes.asElement(superclass))); } @Override public Boolean visitVariable(VariableElement e, Void p) { @@ -79,8 +98,8 @@ && validateTypes(e.getInterfaces()) && (defaultValue == null || validateAnnotationValue(defaultValue, e.getReturnType())) && validateType(e.getReturnType()) && validateTypes(e.getThrownTypes()) - && validateElements(e.getTypeParameters()) - && validateElements(e.getParameters()); + && validateElementsInternal(e.getTypeParameters()) + && validateElementsInternal(e.getParameters()); } @Override public Boolean visitTypeParameter(TypeParameterElement e, Void p) { @@ -94,17 +113,21 @@ && validateElements(e.getTypeParameters()) } }; - public static boolean validateElement(Element element) { - return element.accept(ELEMENT_VALIDATING_VISITOR, null); + private boolean validateElementInternal(Element element) { + if (visited.add(element)) { + return element.accept(ELEMENT_VALIDATING_VISITOR, null); + } else { + return true; + } } - private static boolean isValidBaseElement(Element e) { + private boolean isValidBaseElement(Element e) { return validateType(e.asType()) && validateAnnotations(e.getAnnotationMirrors()) - && validateElements(e.getEnclosedElements()); + && validateElementsInternal(e.getEnclosedElements()); } - private static boolean validateTypes(Iterable types) { + private boolean validateTypes(Iterable types) { for (TypeMirror type : types) { if (!validateType(type)) { return false; @@ -118,7 +141,7 @@ private static boolean validateTypes(Iterable types) { * an issue. Javac turns the whole type parameter into an error type if it can't figure out the * bounds. */ - private static final TypeVisitor TYPE_VALIDATING_VISITOR = + private final TypeVisitor TYPE_VALIDATING_VISITOR = new SimpleTypeVisitor6() { @Override protected Boolean defaultAction(TypeMirror t, Void p) { @@ -163,11 +186,11 @@ && validateTypes(t.getThrownTypes()) } }; - private static boolean validateType(TypeMirror type) { + private boolean validateType(TypeMirror type) { return type.accept(TYPE_VALIDATING_VISITOR, null); } - private static boolean validateAnnotations( + private boolean validateAnnotations( Iterable annotationMirrors) { for (AnnotationMirror annotationMirror : annotationMirrors) { if (!validateAnnotation(annotationMirror)) { @@ -177,13 +200,13 @@ private static boolean validateAnnotations( return true; } - private static boolean validateAnnotation(AnnotationMirror annotationMirror) { + private boolean validateAnnotation(AnnotationMirror annotationMirror) { return validateType(annotationMirror.getAnnotationType()) && validateAnnotationValues(annotationMirror.getElementValues()); } @SuppressWarnings("unused") - private static boolean validateAnnotationValues( + private boolean validateAnnotationValues( Map valueMap) { for (Map.Entry valueEntry : valueMap.entrySet()) { @@ -195,7 +218,7 @@ private static boolean validateAnnotationValues( return true; } - private static final AnnotationValueVisitor VALUE_VALIDATING_VISITOR = + private final AnnotationValueVisitor VALUE_VALIDATING_VISITOR = new SimpleAnnotationValueVisitor6() { @Override protected Boolean defaultAction(Object o, TypeMirror expectedType) { return MoreTypes.isTypeOf(o.getClass(), expectedType); @@ -232,7 +255,7 @@ public Boolean visitArray(List values, TypeMirror exp @Override public Boolean visitEnumConstant(VariableElement enumConstant, TypeMirror expectedType) { return MoreTypes.equivalence().equivalent(enumConstant.asType(), expectedType) - && validateElement(enumConstant); + && validateElementInternal(enumConstant); } @Override public Boolean visitType(TypeMirror type, TypeMirror ignored) { @@ -276,7 +299,7 @@ public Boolean visitEnumConstant(VariableElement enumConstant, TypeMirror expect } }; - private static boolean validateAnnotationValue( + private boolean validateAnnotationValue( AnnotationValue annotationValue, TypeMirror expectedType) { return annotationValue.accept(VALUE_VALIDATING_VISITOR, expectedType); } diff --git a/common/src/test/java/com/google/auto/common/SuperficialValidationTest.java b/common/src/test/java/com/google/auto/common/SuperficialValidationTest.java index 15e54fff52..2cf9d2c551 100644 --- a/common/src/test/java/com/google/auto/common/SuperficialValidationTest.java +++ b/common/src/test/java/com/google/auto/common/SuperficialValidationTest.java @@ -19,7 +19,9 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.testing.compile.JavaSourceSubjectFactory.javaSource; +import static com.google.testing.compile.JavaSourcesSubjectFactory.javaSources; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.testing.compile.JavaFileObjects; import java.util.Set; @@ -199,6 +201,63 @@ public void handlesRecursiveType() { .compilesWithoutError(); } + @Test + public void handlesInnerSubclass() { + JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( + "test.TestClass", + "package test;", + "", + "abstract class TestClass {", + " class InnerTestClass extends TestClass {}", + "}"); + assertAbout(javaSource()) + .that(javaFileObject) + .processedWith(new AssertingProcessor() { + @Override void runAssertions() { + TypeElement testClassElement = + processingEnv.getElementUtils().getTypeElement("test.TestClass"); + assertThat(SuperficialValidation.validateElement(testClassElement)).isTrue(); + } + }) + .compilesWithoutError(); + } + + @Test + public void handlesRecursiveSuperinterface() { + JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( + "test.TestClass", + "package test;", + "", + "interface TestClass implements TestClass {}"); + assertAbout(javaSource()) + .that(javaFileObject) + .processedWith(new AssertingProcessor() { + @Override + void runAssertions() { + assertWithMessage("Should not reach annotation processing.").fail(); + } + }) + .failsToCompile(); + } + + @Test + public void handlesRecursiveSuperclass() { + JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( + "test.TestClass", + "package test;", + "", + "class TestClass extends TestClass {}"); + assertAbout(javaSource()) + .that(javaFileObject) + .processedWith(new AssertingProcessor() { + @Override + void runAssertions() { + assertWithMessage("Should not reach annotation processing.").fail(); + } + }) + .failsToCompile(); + } + @Test public void missingWildcardBound() { JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( @@ -276,6 +335,96 @@ void runAssertions() { .failsToCompile(); } + @Test + public void missingSuperclass() { + JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( + "test.TestClass", + "package test;", + "", + "class TestClass extends Missing {}"); + assertAbout(javaSource()) + .that(javaFileObject) + .processedWith(new AssertingProcessor() { + @Override + void runAssertions() { + TypeElement testClassElement = + processingEnv.getElementUtils().getTypeElement("test.TestClass"); + assertThat(SuperficialValidation.validateElement(testClassElement)).isFalse(); + } + }) + .failsToCompile(); + } + + @Test + public void missingSuperinterface() { + JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( + "test.TestClass", + "package test;", + "", + "class TestClass implements Missing {}"); + assertAbout(javaSource()) + .that(javaFileObject) + .processedWith(new AssertingProcessor() { + @Override + void runAssertions() { + TypeElement testClassElement = + processingEnv.getElementUtils().getTypeElement("test.TestClass"); + assertThat(SuperficialValidation.validateElement(testClassElement)).isFalse(); + } + }) + .failsToCompile(); + } + + @Test + public void missingGrandparentSuperclass() { + JavaFileObject parentJavaFileObject = JavaFileObjects.forSourceLines( + "test.Parent", + "package test;", + "", + "class Parent extends Missing {}"); + JavaFileObject testClassJavaFileObject = JavaFileObjects.forSourceLines( + "test.TestClass", + "package test;", + "", + "class TestClass extends Parent {}"); + assertAbout(javaSources()) + .that(ImmutableList.of(parentJavaFileObject, testClassJavaFileObject)) + .processedWith(new AssertingProcessor() { + @Override + void runAssertions() { + TypeElement testClassElement = + processingEnv.getElementUtils().getTypeElement("test.TestClass"); + assertThat(SuperficialValidation.validateElement(testClassElement)).isFalse(); + } + }) + .failsToCompile(); + } + + @Test + public void missingGrandparentSuperinterface() { + JavaFileObject parentJavaFileObject = JavaFileObjects.forSourceLines( + "test.Parent", + "package test;", + "", + "interface Parent extends Missing {}"); + JavaFileObject testClassJavaFileObject = JavaFileObjects.forSourceLines( + "test.TestClass", + "package test;", + "", + "class TestClass implements Parent {}"); + assertAbout(javaSources()) + .that(ImmutableList.of(parentJavaFileObject, testClassJavaFileObject)) + .processedWith(new AssertingProcessor() { + @Override + void runAssertions() { + TypeElement testClassElement = + processingEnv.getElementUtils().getTypeElement("test.TestClass"); + assertThat(SuperficialValidation.validateElement(testClassElement)).isFalse(); + } + }) + .failsToCompile(); + } + private abstract static class AssertingProcessor extends AbstractProcessor { @Override public Set getSupportedAnnotationTypes() {