Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recursive supertype and superinterface validation #733

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+191 −19
Open

Recursive supertype and superinterface validation #733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 42 additions & 19 deletions common/src/main/java/com/google/auto/common/SuperficialValidation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -46,27 +49,43 @@
* @author Gregory Kick
*/
public final class SuperficialValidation {

private final Set<Element> visited = new HashSet<>();

private SuperficialValidation() {}

public static boolean validateElements(Iterable<? extends Element> elements) {
return new SuperficialValidation().validateElementsInternal(elements);
}

public static boolean validateElement(Element element) {
return new SuperficialValidation().validateElementInternal(element);
}

private boolean validateElementsInternal(Iterable<? extends Element> elements) {
for (Element element : elements) {
if (!validateElement(element)) {
if (!validateElementInternal(element)) {
return false;
}
}
return true;
}

private static final ElementVisitor<Boolean, Void> ELEMENT_VALIDATING_VISITOR =
private final ElementVisitor<Boolean, Void> ELEMENT_VALIDATING_VISITOR =
new AbstractElementVisitor6<Boolean, Void>() {
@Override public Boolean visitPackage(PackageElement e, Void p) {
// don't validate enclosed elements because it will return types in the package
return validateAnnotations(e.getAnnotationMirrors());
}

@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) {
Expand All @@ -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) {
Expand All @@ -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<? extends TypeMirror> types) {
private boolean validateTypes(Iterable<? extends TypeMirror> types) {
for (TypeMirror type : types) {
if (!validateType(type)) {
return false;
Expand All @@ -118,7 +141,7 @@ private static boolean validateTypes(Iterable<? extends TypeMirror> 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<Boolean, Void> TYPE_VALIDATING_VISITOR =
private final TypeVisitor<Boolean, Void> TYPE_VALIDATING_VISITOR =
new SimpleTypeVisitor6<Boolean, Void>() {
@Override
protected Boolean defaultAction(TypeMirror t, Void p) {
Expand Down Expand Up @@ -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<? extends AnnotationMirror> annotationMirrors) {
for (AnnotationMirror annotationMirror : annotationMirrors) {
if (!validateAnnotation(annotationMirror)) {
Expand All @@ -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<? extends ExecutableElement, ? extends AnnotationValue> valueMap) {
for (Map.Entry<? extends ExecutableElement, ? extends AnnotationValue> valueEntry :
valueMap.entrySet()) {
Expand All @@ -195,7 +218,7 @@ private static boolean validateAnnotationValues(
return true;
}

private static final AnnotationValueVisitor<Boolean, TypeMirror> VALUE_VALIDATING_VISITOR =
private final AnnotationValueVisitor<Boolean, TypeMirror> VALUE_VALIDATING_VISITOR =
new SimpleAnnotationValueVisitor6<Boolean, TypeMirror>() {
@Override protected Boolean defaultAction(Object o, TypeMirror expectedType) {
return MoreTypes.isTypeOf(o.getClass(), expectedType);
Expand Down Expand Up @@ -232,7 +255,7 @@ public Boolean visitArray(List<? extends AnnotationValue> 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) {
Expand Down Expand Up @@ -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);
}
Expand Down
149 changes: 149 additions & 0 deletions common/src/test/java/com/google/auto/common/SuperficialValidationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {}");
ronshapiro marked this conversation as resolved.
Show resolved Hide resolved
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(
Expand Down Expand Up @@ -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<String> getSupportedAnnotationTypes() {
Expand Down