Rework type resolution

- JavaTypeDefinition now lazily resolves generics, meaning less
overhead and avoiding access order problems (ie: class Foo extends <A
extends B, B>). These means you can no longer request the generics list
all at once... but you probably shouldn't anyway
 - I took the chance to move most type resolution code dealing with
Java's Type to the JavaTypeDefinition. This makes things a little
clearer and tidier
 - There are some minor API changes, but since 5.8.0 is recently out I
doubt these have any impact. Plus, there was no easy way to just patch
away the open issues in the old greedy algorithm
This commit is contained in:
Juan Martín Sotuyo Dodero
2017-06-29 20:16:23 -03:00
parent ecaa8cb24d
commit 3bd69b15f1
7 changed files with 159 additions and 266 deletions

View File

@ -28,7 +28,7 @@ public abstract class AbstractJavaAccessTypeNode extends AbstractJavaAccessNode
@Override
public void setType(Class<?> type) {
typeDefinition = JavaTypeDefinition.build(type);
typeDefinition = JavaTypeDefinition.forClass(type);
}
@Override

View File

@ -34,7 +34,7 @@ public abstract class AbstractJavaTypeNode extends AbstractJavaNode implements T
@Override
public void setType(Class<?> type) {
typeDefinition = JavaTypeDefinition.build(type);
typeDefinition = JavaTypeDefinition.forClass(type);
}
@Override

View File

@ -6,10 +6,6 @@ package net.sourceforge.pmd.lang.java.typeresolution;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.lang.reflect.WildcardType;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@ -81,7 +77,6 @@ import net.sourceforge.pmd.lang.java.ast.TypeNode;
import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition;
import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinitionBuilder;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
import net.sourceforge.pmd.lang.symboltable.Scope;
@ -156,12 +151,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
private List<String> importedOnDemand;
private int anonymousClassCounter = 0;
/**
* Contains Class -> JavaTypeDefinitions map for raw Class types. Also helps to avoid infinite recursion
* when determining default upper bounds.
*/
private Map<Class<?>, JavaTypeDefinition> classToDefaultUpperBounds = new HashMap<>();
public ClassTypeResolver() {
this(ClassTypeResolver.class.getClassLoader());
}
@ -251,15 +240,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
ASTTypeArguments typeArguments = node.getFirstChildOfType(ASTTypeArguments.class);
if (typeArguments != null) {
JavaTypeDefinitionBuilder builder = JavaTypeDefinition.builder(node.getType());
for (int index = 0; index < typeArguments.jjtGetNumChildren(); ++index) {
builder.addTypeArg(((TypeNode) typeArguments.jjtGetChild(index)).getTypeDefinition());
final JavaTypeDefinition[] boundGenerics = new JavaTypeDefinition[typeArguments.jjtGetNumChildren()];
for (int i = 0; i < typeArguments.jjtGetNumChildren(); ++i) {
boundGenerics[i] = ((TypeNode) typeArguments.jjtGetChild(i)).getTypeDefinition();
}
node.setTypeDefinition(builder.build());
} else if (isGeneric(node.getType()) && node.getTypeDefinition().getGenericArgs().size() == 0) {
node.setTypeDefinition(getDefaultUpperBounds(null, node.getType()));
node.setTypeDefinition(JavaTypeDefinition.forClass(node.getType(), boundGenerics));
}
return data;
@ -297,8 +283,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
&& node.getNameDeclaration().getNode() instanceof TypeNode) {
// Carry over the type from the declaration
Class<?> nodeType = ((TypeNode) node.getNameDeclaration().getNode()).getType();
// generic classes and class with generic super types could have the wrong type assigned here
if (nodeType != null && !isGeneric(nodeType) && !isGeneric(nodeType.getSuperclass())) {
// FIXME : generic classes and class with generic super types could have the wrong type assigned here
if (nodeType != null) {
node.setType(nodeType);
}
}
@ -341,16 +327,16 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
*/
private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class<?>
accessingClass) {
while (typeToSearch != null) {
while (typeToSearch != null && typeToSearch.getType() != Object.class) {
try {
Field field = typeToSearch.getType().getDeclaredField(fieldImage);
final Field field = typeToSearch.getType().getDeclaredField(fieldImage);
if (isMemberVisibleFromClass(typeToSearch.getType(), field.getModifiers(), accessingClass)) {
return getNextTypeDefinition(typeToSearch, field.getGenericType());
return typeToSearch.resolveTypeDefinition(field.getGenericType());
}
} catch (NoSuchFieldException e) { /* swallow */ }
// transform the type into it's supertype
typeToSearch = getNextTypeDefinition(typeToSearch, typeToSearch.getType().getGenericSuperclass());
typeToSearch = typeToSearch.resolveTypeDefinition(typeToSearch.getType().getGenericSuperclass());
}
return null;
@ -386,7 +372,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
if (typeNode.jjtGetChild(0) instanceof ASTReferenceType) {
return ((TypeNode) typeNode.jjtGetChild(0)).getTypeDefinition();
} else { // primitive type
return JavaTypeDefinition.build(typeNode.getType());
return JavaTypeDefinition.forClass(typeNode.getType());
}
}
}
@ -416,133 +402,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
return null;
}
/**
* Given a type def. and a Type, resolves the type into a JavaTypeDefinition. Takes into account
* simple Classes, TypeVariables, ParameterizedTypes and WildCards types. Can resolve nested Generic
* type arguments.
*
* @param context The JavaTypeDefinition in which the {@code genericType} was declared.
* @param genericType The Type to resolve.
* @return JavaTypeDefinition of the {@code genericType}.
*/
private JavaTypeDefinition getNextTypeDefinition(JavaTypeDefinition context, Type genericType) {
return getNextTypeDefinition(context, genericType, null);
}
private JavaTypeDefinition getNextTypeDefinition(JavaTypeDefinition context, Type genericType,
JavaTypeDefinitionBuilder buildTypeInAdvance) {
if (genericType == null) {
return null;
}
if (genericType instanceof Class) { // Raw types take this branch as well
return getDefaultUpperBounds(context, (Class) genericType);
} else if (genericType instanceof ParameterizedType) {
ParameterizedType parameterizedType = (ParameterizedType) genericType;
JavaTypeDefinitionBuilder typeDef = JavaTypeDefinition.builder((Class) parameterizedType.getRawType());
if (buildTypeInAdvance != null) {
buildTypeInAdvance.addTypeArg(typeDef.build());
}
// recursively determine each type argument's type def.
for (Type type : parameterizedType.getActualTypeArguments()) {
typeDef.addTypeArg(getNextTypeDefinition(context, type));
}
return typeDef.build();
} else if (genericType instanceof TypeVariable) {
int ordinal = getTypeParameterOrdinal(context.getType(), ((TypeVariable) genericType).getName());
if (ordinal != -1) {
return context.getGenericArgs().get(ordinal);
}
} else if (genericType instanceof WildcardType) {
Type[] wildcardUpperBounds = ((WildcardType) genericType).getUpperBounds();
if (wildcardUpperBounds.length != 0) { // upper bound wildcard
return getNextTypeDefinition(context, wildcardUpperBounds[0]);
} else { // lower bound wildcard
return JavaTypeDefinition.build(Object.class);
}
}
return null;
}
/**
* Returns the ordinal of the type parameter with the name {@code parameterName} in {@code clazz}.
*
* @param clazz The Class with the type parameters.
* @param parameterName The name of the type parameter.
* @return The ordinal of the type parameter.
*/
private int getTypeParameterOrdinal(Class<?> clazz, String parameterName) {
TypeVariable[] classTypeParameters = clazz.getTypeParameters();
for (int index = 0; index < classTypeParameters.length; ++index) {
if (classTypeParameters[index].getName().equals(parameterName)) {
return index;
}
}
return -1;
}
/**
* Returns true if the class is generic.
*
* @param clazz The Class to examine.
* @return True if the Class is generic.
*/
private boolean isGeneric(Class<?> clazz) {
if (clazz != null) {
return clazz.getTypeParameters().length != 0;
}
return false;
}
/**
* Given a Class, returns the type def. for when the Class stands without type arguments, meaning it
* is a raw type. Determines the generic types by looking at the upper bounds of it's generic parameters.
*
* @param context Synthetic parameter for recursion, pass {@code null}.
* @param clazzWithDefBounds The raw Class type.
* @return The type def. of the raw Class.
*/
private JavaTypeDefinition getDefaultUpperBounds(JavaTypeDefinition context, Class<?> clazzWithDefBounds) {
JavaTypeDefinitionBuilder typeDef = JavaTypeDefinition.builder(clazzWithDefBounds);
// helps avoid infinite recursion with Something<.... E extends Something (<- same raw type)... >
if (classToDefaultUpperBounds.containsKey(clazzWithDefBounds)) {
return classToDefaultUpperBounds.get(clazzWithDefBounds);
} else {
classToDefaultUpperBounds.put(clazzWithDefBounds, typeDef.build());
}
if (isGeneric(clazzWithDefBounds)) {
// Recursion, outer call should pass in null.
// Recursive calls will get the first JavaTypeDefinition to be able to resolve cases like
// ... < T extends Something ... E extends Other<T> ... >
if (context == null) {
context = typeDef.build();
}
for (TypeVariable parameter : clazzWithDefBounds.getTypeParameters()) {
// TODO: fix self reference "< ... E extends Something<E> ... >"
JavaTypeDefinition typeDefOfParameter = getNextTypeDefinition(context, parameter.getBounds()[0],
typeDef);
// if it isn't 0, then it has already been added
if (typeDefOfParameter.getGenericArgs().size() == 0) {
typeDef.addTypeArg(getNextTypeDefinition(context, parameter.getBounds()[0]));
}
}
}
return typeDef.build();
}
/**
* Given a class, the modifiers of on of it's member and the class that is trying to access that member,
* returns true is the member is accessible from the accessingClass Class.
@ -790,7 +649,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
if (currentChild.jjtGetLastToken().toString().equals("this")) {
if (previousChild != null) { // Qualified 'this' expression
currentChild.setType(previousChild.getType());
currentChild.setTypeDefinition(previousChild.getTypeDefinition());
} else { // simple 'this' expression
ASTClassOrInterfaceDeclaration typeDeclaration
= currentChild.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class);
@ -817,8 +676,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
&& currentChild.getImage() != null) {
currentChild.setTypeDefinition(getFieldType(previousChild.getTypeDefinition(),
currentChild.getImage(),
accessingClass));
currentChild.getImage(), accessingClass));
}
}
@ -889,7 +747,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
if (extendsList != null) {
return ((TypeNode) extendsList.jjtGetChild(0)).getTypeDefinition();
} else {
return JavaTypeDefinition.build(Object.class);
return JavaTypeDefinition.forClass(Object.class);
}
// anonymous class declaration
@ -939,13 +797,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
if (node.jjtGetParent() instanceof ASTClassOrInterfaceDeclaration) {
TypeNode parent = (TypeNode) node.jjtGetParent();
JavaTypeDefinitionBuilder builder = JavaTypeDefinition.builder(parent.getType());
for (int childIndex = 0; childIndex < node.jjtGetNumChildren(); ++childIndex) {
builder.addTypeArg(((TypeNode) node.jjtGetChild(childIndex)).getTypeDefinition());
final JavaTypeDefinition[] boundGenerics = new JavaTypeDefinition[node.jjtGetNumChildren()];
for (int i = 0; i < node.jjtGetNumChildren(); ++i) {
boundGenerics[i] = ((TypeNode) node.jjtGetChild(i)).getTypeDefinition();
}
parent.setTypeDefinition(builder.build());
parent.setTypeDefinition(JavaTypeDefinition.forClass(parent.getType(), boundGenerics));
}
return data;

View File

@ -4,71 +4,161 @@
package net.sourceforge.pmd.lang.java.typeresolution.typedefinition;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.lang.reflect.WildcardType;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class JavaTypeDefinition implements TypeDefinition {
private final Class<?> clazz;
private List<JavaTypeDefinition> genericArgs;
// contains TypeDefs where only the clazz field is used
private static Map<Class<?>, JavaTypeDefinition> onlyClassTypeDef = new HashMap<>();
private static final Map<Class<?>, JavaTypeDefinition> CLASS_TYPE_DEF_CACHE = new HashMap<>();
private final Class<?> clazz;
private final List<JavaTypeDefinition> genericArgs;
private final boolean isGeneric;
private JavaTypeDefinition(final Class<?> clazz) {
this.clazz = clazz;
final TypeVariable<?>[] typeParameters;
if (clazz.isAnonymousClass()) {
typeParameters = resolveTypeDefinition(clazz.getGenericSuperclass()).clazz.getTypeParameters();
} else {
typeParameters = clazz.getTypeParameters();
}
isGeneric = typeParameters.length != 0;
if (isGeneric) {
// Generics will be lazily loaded
this.genericArgs = new ArrayList<JavaTypeDefinition>(typeParameters.length);
} else {
this.genericArgs = Collections.emptyList();
}
}
public static JavaTypeDefinition forClass(final Class<?> clazz) {
if (clazz == null) {
return null;
}
final JavaTypeDefinition typeDef = CLASS_TYPE_DEF_CACHE.get(clazz);
if (typeDef != null) {
return typeDef;
}
final JavaTypeDefinition newDef = new JavaTypeDefinition(clazz);
// We can only cache types without generics, since their values are context-based
if (!newDef.isGeneric) {
CLASS_TYPE_DEF_CACHE.put(clazz, newDef);
}
return newDef;
}
public static JavaTypeDefinition forClass(final Class<?> clazz, final JavaTypeDefinition... boundGenerics) {
// With generics there is no cache
final JavaTypeDefinition typeDef = forClass(clazz);
if (typeDef == null) {
return null;
}
for (final JavaTypeDefinition generic : boundGenerics) {
typeDef.genericArgs.add(generic);
}
return typeDef;
}
@Override
public Class<?> getType() {
return clazz;
}
public List<JavaTypeDefinition> getGenericArgs() {
if (genericArgs == null) {
genericArgs = Collections.unmodifiableList(new ArrayList<JavaTypeDefinition>());
}
return genericArgs;
public boolean isGeneric() {
return !genericArgs.isEmpty();
}
private JavaTypeDefinition(Class<?> clazz, List<JavaTypeDefinition> genericArgs) {
this.clazz = clazz;
if (genericArgs != null) {
this.genericArgs = Collections.unmodifiableList(genericArgs);
}
}
// builder part of the class
public static JavaTypeDefinition build(Class<?> clazz) {
if (onlyClassTypeDef.containsKey(clazz)) {
return onlyClassTypeDef.get(clazz);
}
JavaTypeDefinition typeDef = new JavaTypeDefinition(clazz, null);
onlyClassTypeDef.put(clazz, typeDef);
return typeDef;
}
/**
* @param genericArgs This package private method expects that the genericArgs list has not been leaked,
* meaning the other references have been discarded to ensure immutability.
*/
/* default */ static JavaTypeDefinition build(Class<?> clazz, List<JavaTypeDefinition> genericArgs) {
if (genericArgs == null) {
return build(clazz);
public JavaTypeDefinition getGenericType(final String parameterName) {
final TypeVariable<?>[] typeParameters = clazz.getTypeParameters();
for (int i = 0; i < typeParameters.length; i++) {
if (typeParameters[i].getName().equals(parameterName)) {
return getGenericType(i);
}
}
return new JavaTypeDefinition(clazz, genericArgs);
throw new IllegalArgumentException("No generic parameter by name " + parameterName
+ " on class " + clazz.getSimpleName());
}
public static JavaTypeDefinitionBuilder builder(Class<?> clazz) {
return new JavaTypeDefinitionBuilder().setType(clazz);
public JavaTypeDefinition getGenericType(final int index) {
// Check if it has been lazily initialized first
if (genericArgs.size() > index) {
final JavaTypeDefinition cachedDefinition = genericArgs.get(index);
if (cachedDefinition != null) {
return cachedDefinition;
}
}
// Force the list to have enough elements
for (int i = genericArgs.size(); i <= index; i++) {
genericArgs.add(null);
}
/*
* Set a default to circuit-brake any recursions (ie: raw types with no generic info)
* Object.class is a right answer in those scenarios
*/
genericArgs.set(index, forClass(Object.class));
final TypeVariable<?> typeVariable = clazz.getTypeParameters()[index];
final JavaTypeDefinition typeDefinition = resolveTypeDefinition(typeVariable.getBounds()[0]);
// cache result
genericArgs.set(index, typeDefinition);
return typeDefinition;
}
public JavaTypeDefinition resolveTypeDefinition(final Type type) {
if (type == null) {
// Without more info, this is all we can tell...
return forClass(Object.class);
}
public static JavaTypeDefinitionBuilder builder() {
return new JavaTypeDefinitionBuilder();
if (type instanceof Class) { // Raw types take this branch as well
return forClass((Class<?>) type);
} else if (type instanceof ParameterizedType) {
final ParameterizedType parameterizedType = (ParameterizedType) type;
// recursively determine each type argument's type def.
final Type[] typeArguments = parameterizedType.getActualTypeArguments();
final JavaTypeDefinition[] genericBounds = new JavaTypeDefinition[typeArguments.length];
for (int i = 0; i < typeArguments.length; i++) {
genericBounds[i] = resolveTypeDefinition(typeArguments[i]);
}
// TODO : is this cast safe?
return forClass((Class<?>) parameterizedType.getRawType(), genericBounds);
} else if (type instanceof TypeVariable) {
// return forClass(Object.class);
return getGenericType(((TypeVariable<?>) type).getName());
} else if (type instanceof WildcardType) {
final Type[] wildcardUpperBounds = ((WildcardType) type).getUpperBounds();
if (wildcardUpperBounds.length != 0) { // upper bound wildcard
return resolveTypeDefinition(wildcardUpperBounds[0]);
} else { // lower bound wildcard
return forClass(Object.class);
}
}
// TODO : Shall we throw here?
return forClass(Object.class);
}
}

View File

@ -1,39 +0,0 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.typeresolution.typedefinition;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
public class JavaTypeDefinitionBuilder {
private Class<?> clazz = null;
private List<JavaTypeDefinition> genericArgs = new ArrayList<>();
/* default */ JavaTypeDefinitionBuilder() {}
public JavaTypeDefinitionBuilder addTypeArg(JavaTypeDefinition arg) {
genericArgs.add(arg);
return this;
}
public List<JavaTypeDefinition> getTypeArgs() {
return Collections.unmodifiableList(genericArgs);
}
public JavaTypeDefinitionBuilder getTypeArg(int index) {
genericArgs.get(index);
return this;
}
public JavaTypeDefinitionBuilder setType(Class<?> clazz) {
this.clazz = clazz;
return this;
}
public JavaTypeDefinition build() {
return JavaTypeDefinition.build(clazz, genericArgs);
}
}

View File

@ -4,8 +4,6 @@
package net.sourceforge.pmd.lang.java.typeresolution.typedefinition;
import java.util.List;
public interface TypeDefinition {
/**
* Get the raw Class type of the definition.
@ -13,11 +11,4 @@ public interface TypeDefinition {
* @return Raw Class type.
*/
Class<?> getType();
/**
* Get the list of type arguments for this TypeDefinition.
*
* @return An ordered and immutable list of type arguments.
*/
List<? extends TypeDefinition> getGenericArgs();
}

View File

@ -6,7 +6,7 @@ package net.sourceforge.pmd.typeresolution;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertSame;
import java.io.IOException;
import java.io.InputStream;
@ -46,7 +46,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.AbstractJavaTypeNode;
import net.sourceforge.pmd.lang.java.ast.TypeNode;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.java.typeresolution.ClassTypeResolver;
import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition;
@ -1067,9 +1066,6 @@ public class ClassTypeResolverTest {
int index = 0;
JavaTypeDefinition typeDef;
// this.genericField.first = "";
assertEquals(String.class, expressions.get(index).getType());
assertChildTypeArgsEqualTo(expressions.get(index), 1, String.class, Double.class);
@ -1114,17 +1110,15 @@ public class ClassTypeResolverTest {
assertEquals("All expressions not tested", index, expressions.size());
}
private Class getChildType(Node node, int childIndex) {
private Class<?> getChildType(Node node, int childIndex) {
return ((TypeNode) node.jjtGetChild(childIndex)).getType();
}
private void assertChildTypeArgsEqualTo(Node node, int childIndex, Class... classes) {
private void assertChildTypeArgsEqualTo(Node node, int childIndex, Class<?>... classes) {
JavaTypeDefinition typeDef = ((TypeNode) node.jjtGetChild(childIndex)).getTypeDefinition();
assertTrue(typeDef.getGenericArgs().size() == classes.length);
for (int index = 0; index < classes.length; ++index) {
assertTrue(typeDef.getGenericArgs().get(index).getType() == classes[index]);
assertSame(classes[index], typeDef.getGenericType(index).getType());
}
}