Skip to content

Commit

Permalink
Defer processing in @AutoValue classes if any abstract method has a…
Browse files Browse the repository at this point in the history
…n undefined return type or parameter type. This avoids problems in certain cases where other annotation processors will generate the currently-undefined type.

Fixes #847.

RELNOTES=Fixed a problem when an `@AutoValue` class references types that are generated by other annotation processors.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=312172526
  • Loading branch information
eamonnmcmanus authored and cgdecker committed May 20, 2020
1 parent 365d3f6 commit 2797d38
Show file tree
Hide file tree
Showing 9 changed files with 297 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.auto.common.MoreElements;
import com.google.auto.common.MoreTypes;
import com.google.auto.service.AutoService;
import com.google.auto.value.processor.MissingTypes.MissingTypeException;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
Expand All @@ -43,7 +44,6 @@
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import net.ltgt.gradle.incap.IncrementalAnnotationProcessor;
import net.ltgt.gradle.incap.IncrementalAnnotationProcessorType;
Expand Down Expand Up @@ -135,12 +135,18 @@ private DeclaredType mirrorForKindType(TypeElement autoOneOfType) {
AnnotationValue kindValue =
AnnotationMirrors.getAnnotationValue(oneOfAnnotation.get(), "value");
Object value = kindValue.getValue();
if (value instanceof TypeMirror && ((TypeMirror) value).getKind().equals(TypeKind.DECLARED)) {
return MoreTypes.asDeclared((TypeMirror) value);
} else {
// This is presumably because the referenced type doesn't exist.
throw new MissingTypeException();
if (value instanceof TypeMirror) {
TypeMirror kindType = (TypeMirror) value;
switch (kindType.getKind()) {
case DECLARED:
return MoreTypes.asDeclared(kindType);
case ERROR:
throw new MissingTypeException(MoreTypes.asError(kindType));
default:
break;
}
}
throw new MissingTypeException(null);
}

private ImmutableMap<String, String> propertyToKindMap(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.google.auto.common.MoreElements;
import com.google.auto.common.MoreTypes;
import com.google.auto.common.Visibility;
import com.google.auto.value.processor.MissingTypes.MissingTypeException;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -125,6 +126,15 @@ final Elements elementUtils() {
return processingEnv.getElementUtils();
}

/**
* Qualified names of {@code @AutoValue} or {@code AutoOneOf} classes that we attempted to process
* but had to abandon because we needed other types that they referenced and those other types
* were missing. This is used by tests.
*/
final ImmutableList<String> deferredTypeNames() {
return ImmutableList.copyOf(deferredTypeNames);
}

@Override
public final SourceVersion getSupportedSourceVersion() {
return SourceVersion.latestSupported();
Expand Down Expand Up @@ -693,14 +703,17 @@ static String equalsParameterType(Map<ObjectMethod, ExecutableElement> methodsTo

/**
* 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.
* signature is only mentioned once, even if it is inherited on more than one path. If any of the
* abstract methods has a return type or parameter type that is not currently defined then this
* method will throw an exception that will cause us to defer processing of the current class
* until a later annotation-processing round.
*/
static ImmutableSet<ExecutableElement> abstractMethodsIn(
ImmutableSet<ExecutableElement> methods) {
static ImmutableSet<ExecutableElement> abstractMethodsIn(Iterable<ExecutableElement> methods) {
Set<Name> noArgMethods = new HashSet<>();
ImmutableSet.Builder<ExecutableElement> abstracts = ImmutableSet.builder();
for (ExecutableElement method : methods) {
if (method.getModifiers().contains(Modifier.ABSTRACT)) {
MissingTypes.deferIfMissingTypesIn(method);
boolean hasArgs = !method.getParameters().isEmpty();
if (hasArgs || noArgMethods.add(method.getSimpleName())) {
// If an abstract method with the same signature is inherited on more than one path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.ElementFilter;
import javax.lang.model.util.Types;

/**
Expand Down Expand Up @@ -257,8 +256,7 @@ void defineVars(
if (!optionalClassifier.isPresent()) {
return;
}
for (ExecutableElement method :
ElementFilter.methodsIn(builderTypeElement.getEnclosedElements())) {
for (ExecutableElement method : methodsIn(builderTypeElement.getEnclosedElements())) {
if (method.getSimpleName().contentEquals("builder")
&& method.getModifiers().contains(Modifier.STATIC)
&& method.getAnnotationMirrors().isEmpty()) {
Expand Down Expand Up @@ -480,14 +478,20 @@ private static boolean sameTypeParameters(TypeElement a, TypeElement b) {
return true;
}

// Return a set of all abstract methods in the given TypeElement or inherited from ancestors.
private Set<ExecutableElement> abstractMethods(TypeElement typeElement) {
/**
* Returns a set of all abstract methods in the given TypeElement or inherited from ancestors. If
* any of the abstract methods has a return type or parameter type that is not currently defined
* then this method will throw an exception that will cause us to defer processing of the current
* class until a later annotation-processing round.
*/
private ImmutableSet<ExecutableElement> abstractMethods(TypeElement typeElement) {
Set<ExecutableElement> methods =
getLocalAndInheritedMethods(
typeElement, processingEnv.getTypeUtils(), processingEnv.getElementUtils());
ImmutableSet.Builder<ExecutableElement> abstractMethods = ImmutableSet.builder();
for (ExecutableElement method : methods) {
if (method.getModifiers().contains(Modifier.ABSTRACT)) {
MissingTypes.deferIfMissingTypesIn(method);
abstractMethods.add(method);
}
}
Expand Down

This file was deleted.

139 changes: 139 additions & 0 deletions value/src/main/java/com/google/auto/value/processor/MissingTypes.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
* Copyright 2014 Google LLC
*
* 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.processor;

import java.util.List;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.ArrayType;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.ErrorType;
import javax.lang.model.type.IntersectionType;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.type.TypeVariable;
import javax.lang.model.type.WildcardType;
import javax.lang.model.util.SimpleTypeVisitor8;

/**
* Handling of undefined types. When we see an undefined type, it might genuinely be undefined, or
* it might be a type whose source code will be generated later on as part of the same compilation.
* If we encounter an undefined type in a place where we need to know the type, we throw {@link
* MissingTypeException}. We then catch that and defer processing for the current class until the
* next annotation-processing "round". If the missing class has been generated in the meanwhile, we
* may now be able to complete processing. After a round has completed without generating any new
* source code, if there are still missing types then we report an error.
*
* @author emcmanus@google.com (Éamonn McManus)
*/
final class MissingTypes {
private MissingTypes() {}

/**
* Exception thrown in the specific case where processing of a class was abandoned because it
* required types that the class references to be present and they were not. This case is handled
* specially because it is possible that those types might be generated later during annotation
* processing, so we should reattempt the processing of the class in a later annotation processing
* round.
*/
@SuppressWarnings("serial")
static class MissingTypeException extends RuntimeException {
MissingTypeException(ErrorType missingType) {
// Although it is not specified as such, in practice ErrorType.toString() is the type name
// that appeared in the source code. Showing it here can help in debugging issues with
// deferral.
super(missingType == null ? null : missingType.toString());
}
}

/**
* Check that the return type and parameter types of the given method are all defined, and arrange
* to defer processing until the next round if not.
*
* @throws MissingTypeException if the return type or a parameter type of the given method is
* undefined
*/
static void deferIfMissingTypesIn(ExecutableElement method) {
MISSING_TYPE_VISITOR.check(method.getReturnType());
for (VariableElement param : method.getParameters()) {
MISSING_TYPE_VISITOR.check(param.asType());
}
}

private static final MissingTypeVisitor MISSING_TYPE_VISITOR = new MissingTypeVisitor();

private static class MissingTypeVisitor extends SimpleTypeVisitor8<Void, TypeMirrorSet> {
// Avoid infinite recursion for a type like `Enum<E extends Enum<E>>` by remembering types that
// we have already seen on this visit. Recursion has to go through a declared type, such as Enum
// in this example, so in principle it should be enough to check only in visitDeclared. However
// Eclipse has a quirk where the second E in `Enum<E extends Enum<E>>` is not the same as the
// first, and if you ask for its bounds you will get another `Enum<E>` with a third E. So we
// also check in visitTypeVariable. TypeMirrorSet does consider that all these E variables are
// the same so infinite recursion is avoided.
void check(TypeMirror type) {
type.accept(this, new TypeMirrorSet());
}

@Override
public Void visitError(ErrorType t, TypeMirrorSet visiting) {
throw new MissingTypeException(t);
}

@Override
public Void visitArray(ArrayType t, TypeMirrorSet visiting) {
return t.getComponentType().accept(this, visiting);
}

@Override
public Void visitDeclared(DeclaredType t, TypeMirrorSet visiting) {
if (visiting.add(t)) {
visitAll(t.getTypeArguments(), visiting);
}
return null;
}

@Override
public Void visitTypeVariable(TypeVariable t, TypeMirrorSet visiting) {
if (visiting.add(t)) {
t.getLowerBound().accept(this, visiting);
t.getUpperBound().accept(this, visiting);
}
return null;
}

@Override
public Void visitWildcard(WildcardType t, TypeMirrorSet visiting) {
if (t.getSuperBound() != null) {
t.getSuperBound().accept(this, visiting);
}
if (t.getExtendsBound() != null) {
t.getExtendsBound().accept(this, visiting);
}
return null;
}

@Override
public Void visitIntersection(IntersectionType t, TypeMirrorSet visiting) {
return visitAll(t.getBounds(), visiting);
}

private Void visitAll(List<? extends TypeMirror> types, TypeMirrorSet visiting) {
for (TypeMirror type : types) {
type.accept(this, visiting);
}
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.google.auto.common.MoreElements;
import com.google.auto.common.MoreTypes;
import com.google.auto.value.processor.MissingTypes.MissingTypeException;
import com.google.common.collect.ImmutableSet;
import java.util.List;
import java.util.OptionalInt;
Expand Down Expand Up @@ -290,7 +291,7 @@ public StringBuilder visitWildcard(WildcardType type, StringBuilder sb) {

@Override
public StringBuilder visitError(ErrorType t, StringBuilder p) {
throw new MissingTypeException();
throw new MissingTypeException(t);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import static javax.lang.model.element.Modifier.PRIVATE;

import com.google.auto.common.MoreElements;
import com.google.auto.common.MoreTypes;
import com.google.auto.value.processor.MissingTypes.MissingTypeException;
import com.google.common.collect.ImmutableSortedSet;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -298,7 +300,7 @@ private static Set<String> ambiguousNames(Types typeUtils, Set<TypeMirror> types
Map<String, Name> simpleNamesToQualifiedNames = new HashMap<>();
for (TypeMirror type : types) {
if (type.getKind() == TypeKind.ERROR) {
throw new MissingTypeException();
throw new MissingTypeException(MoreTypes.asError(type));
}
String simpleName = typeUtils.asElement(type).getSimpleName().toString();
/*
Expand Down
Loading

0 comments on commit 2797d38

Please sign in to comment.