From d17fb883bbe67f1b0755b790264c6b2fa8cf9e35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Thu, 8 Jun 2017 18:54:39 +0200 Subject: [PATCH 01/20] Java: add support for some generic field access --- .../pmd/lang/java/ast/ASTTypeArgument.java | 2 +- .../pmd/lang/java/ast/ASTTypeBound.java | 2 +- .../pmd/lang/java/ast/ASTTypeParameter.java | 2 +- .../typeresolution/ClassTypeResolver.java | 236 ++++++++++++++++-- .../typeresolution/ClassTypeResolverTest.java | 95 +++++-- .../testdata/GenericFieldAccess.java | 59 +++++ .../testdata/dummytypes/GenericClass.java | 6 +- 7 files changed, 355 insertions(+), 47 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeArgument.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeArgument.java index e9139d14cc..7b0ffb8ffd 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeArgument.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeArgument.java @@ -5,7 +5,7 @@ package net.sourceforge.pmd.lang.java.ast; -public class ASTTypeArgument extends AbstractJavaNode { +public class ASTTypeArgument extends AbstractJavaTypeNode { public ASTTypeArgument(int id) { super(id); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeBound.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeBound.java index 4cfba7ab58..3c0a5bf845 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeBound.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeBound.java @@ -5,7 +5,7 @@ package net.sourceforge.pmd.lang.java.ast; -public class ASTTypeBound extends AbstractJavaNode { +public class ASTTypeBound extends AbstractJavaTypeNode { public ASTTypeBound(int id) { super(id); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeParameter.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeParameter.java index 4c412b7adc..6cd0ba9b37 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeParameter.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTypeParameter.java @@ -5,7 +5,7 @@ package net.sourceforge.pmd.lang.java.ast; -public class ASTTypeParameter extends AbstractJavaNode { +public class ASTTypeParameter extends AbstractJavaTypeNode { public ASTTypeParameter(int id) { super(id); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 2fac5fcc7e..f401c31e94 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -5,6 +5,9 @@ package net.sourceforge.pmd.lang.java.typeresolution; import java.lang.reflect.Field; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.lang.reflect.TypeVariable; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -29,6 +32,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression; import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression; import net.sourceforge.pmd.lang.java.ast.ASTConditionalOrExpression; +import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression; import net.sourceforge.pmd.lang.java.ast.ASTExclusiveOrExpression; @@ -39,6 +43,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTInclusiveOrExpression; import net.sourceforge.pmd.lang.java.ast.ASTInstanceOfExpression; import net.sourceforge.pmd.lang.java.ast.ASTLiteral; import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMultiplicativeExpression; import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTNormalAnnotation; @@ -57,7 +62,12 @@ import net.sourceforge.pmd.lang.java.ast.ASTShiftExpression; import net.sourceforge.pmd.lang.java.ast.ASTSingleMemberAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; import net.sourceforge.pmd.lang.java.ast.ASTType; +import net.sourceforge.pmd.lang.java.ast.ASTTypeArgument; +import net.sourceforge.pmd.lang.java.ast.ASTTypeArguments; +import net.sourceforge.pmd.lang.java.ast.ASTTypeBound; import net.sourceforge.pmd.lang.java.ast.ASTTypeDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTTypeParameter; +import net.sourceforge.pmd.lang.java.ast.ASTTypeParameters; import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpressionNotPlusMinus; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator; @@ -203,7 +213,10 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { @Override public Object visit(ASTClassOrInterfaceType node, Object data) { + super.visit(node, data); + String typeName = node.getImage(); + // branch deals with anonymous classes if (node.jjtGetParent().hasDescendantOfType(ASTClassOrInterfaceBody.class)) { anonymousClassCounter++; AbstractNode parent = node.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class); @@ -236,50 +249,147 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { @Override public Object visit(ASTName node, Object data) { - /* + + if (node.getNameDeclaration() != null + && node.getNameDeclaration().getNode() instanceof TypeNode) { + // Carry over the type from the declaration + Class nodeType = ((TypeNode) node.getNameDeclaration().getNode()).getType(); + if (!isGeneric(nodeType)) { + node.setType(nodeType); + } + } + + /* * Only doing this for nodes where getNameDeclaration is null this means * it's not a named node, i.e. Static reference or Annotation Doing this * for memory - TODO: Investigate if there is a valid memory concern or * not */ - if (node.getNameDeclaration() == null) { - // Skip these scenarios as there is no type to populate in these - // cases: - // 1) Parent is a PackageDeclaration, which is not a type - // 2) Parent is a ImportDeclaration, this is handled elsewhere. - if (!(node.jjtGetParent() instanceof ASTPackageDeclaration - || node.jjtGetParent() instanceof ASTImportDeclaration)) { - String[] dotSplitImage = node.getImage().split("\\."); + // Skip these scenarios as there is no type to populate in these + // cases: + // 1) Parent is a PackageDeclaration, which is not a type + // 2) Parent is a ImportDeclaration, this is handled elsewhere. + if (node.getType() == null + && !(node.jjtGetParent() instanceof ASTPackageDeclaration + || node.jjtGetParent() instanceof ASTImportDeclaration)) { - if (dotSplitImage.length == 1) { - populateType(node, dotSplitImage[0]); - } + String[] dotSplitImage = node.getImage().split("\\."); - if (node.getType() == null) { - Class previousNameType = getVariableNameType(node.getScope(), dotSplitImage[0]); + if (dotSplitImage.length == 1) { + populateType(node, dotSplitImage[0]); + } - for (int i = 1; i < dotSplitImage.length; ++i) { - if (previousNameType == null) { - break; - } + if (node.getType() == null) { + Class previousNameType = searchScopeForVariableType(node.getScope(), dotSplitImage[0]); - previousNameType = getFieldFromClass(previousNameType, dotSplitImage[i]).getType(); + List genericList = getTypeArgumentsFromASTType(searchScopeForVariableDeclaration(node.getScope(), dotSplitImage[0]) + .getDeclaratorId().getTypeNode()); + + for (int i = 1; i < dotSplitImage.length; ++i) { + if (previousNameType == null) { + break; } - node.setType(previousNameType); + // TODO: raw types not covered + Field field = getFieldFromClass(previousNameType, dotSplitImage[i]); + previousNameType = getGenericFieldType(genericList, field); + genericList = getGenericFieldTypeArguments(genericList, field); } - } - } else { - // Carry over the type from the declaration - if (node.getNameDeclaration().getNode() instanceof TypeNode) { - node.setType(((TypeNode) node.getNameDeclaration().getNode()).getType()); + + node.setType(previousNameType); } } + return super.visit(node, data); } - private Class getVariableNameType(Scope scope, String image) { + private List getGenericFieldTypeArguments(List previousTypeArguments, Field field) { + List newTypeArguments = new ArrayList<>(); + + if(field.getGenericType() instanceof ParameterizedType) { + Type[] typeArgs = ((ParameterizedType)field.getGenericType()).getActualTypeArguments(); + for(Type type : typeArgs) { + if(type instanceof TypeVariable) { + int ordinal = getTypeParameterOrdinal(field.getDeclaringClass(), ((TypeVariable) type).getName()); + if(ordinal != -1) { + newTypeArguments.add(previousTypeArguments.get(ordinal)); + } + } else { + newTypeArguments.add((Class)type); + } + } + } + + return newTypeArguments; + } + + private Class getGenericFieldType(List typeArguments, Field field) { + Type fieldType = field.getGenericType(); + + if(fieldType instanceof Class) { + return (Class)fieldType; + } else if (fieldType instanceof ParameterizedType) { + return (Class)((ParameterizedType) fieldType).getRawType(); + } else if (fieldType instanceof TypeVariable) { + int ordinal = getTypeParameterOrdinal(field.getDeclaringClass(), ((TypeVariable) fieldType).getName()); + if(ordinal != -1) { + return typeArguments.get(ordinal); + } + } + + return null; + } + + private List getTypeArgumentsFromASTType(ASTType node) { + List typeList = new ArrayList<>(); + + if (node.jjtGetChild(0) instanceof ASTReferenceType) { + + ASTTypeArguments typeArgs = ((ASTClassOrInterfaceType) node.jjtGetChild(0).jjtGetChild(0)) + .getFirstChildOfType(ASTTypeArguments.class); + + if (typeArgs != null) { + // TODO: does not cover type arguments which are generic class + for (int index = 0; index < typeArgs.jjtGetNumChildren(); ++index) { + typeList.add(((TypeNode) typeArgs.jjtGetChild(index)).getType()); + } + } + } + + return typeList; + } + + 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; + } + + private boolean isGeneric(Class clazz) { + return clazz.getTypeParameters().length != 0; + } + + private VariableNameDeclaration searchScopeForVariableDeclaration(Scope scope, String image) { + for (/* empty */; scope != null; scope = scope.getParent()) { + for (Map.Entry> entry + : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { + if (entry.getKey().getImage().equals(image)) { + return entry.getKey(); + } + } + } + + return null; + } + + private Class searchScopeForVariableType(Scope scope, String image) { for (/* empty */; scope != null; scope = scope.getParent()) { for (Map.Entry> entry : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { @@ -570,6 +680,44 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return data; } + @Override + public Object visit(ASTTypeArguments node, Object data) { + super.visit(node, data); + return data; + } + + @Override + public Object visit(ASTTypeArgument node, Object data) { + super.visit(node, data); + rollupTypeUnary(node); + return data; + } + + @Override + public Object visit(ASTTypeParameters node, Object data) { + super.visit(node, data); + return data; + } + + @Override + public Object visit(ASTTypeParameter node, Object data) { + super.visit(node, data); + rollupTypeUnary(node); + + if (node.getType() == null) { + node.setType(Object.class); + } + + return data; + } + + @Override + public Object visit(ASTTypeBound node, Object data) { + super.visit(node, data); + rollupTypeUnary(node); + return data; + } + @Override public Object visit(ASTNullLiteral node, Object data) { // No explicit type @@ -793,11 +941,47 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // ignored } } + + // try generics + // TODO: generic declarations can shadow type declarations ... :( + // TODO: ? and ? super is not covered + if (myType == null) { + ASTTypeParameter parameter = getTypeParameterDeclaration(node, className); + if (parameter != null) { + myType = parameter.getType(); + } + } + if (myType != null) { node.setType(myType); } } + private ASTTypeParameter getTypeParameterDeclaration(Node startNode, String image) { + for (Node parent = startNode.jjtGetParent(); parent != null; parent = parent.jjtGetParent()) { + ASTTypeParameters typeParameters = null; + + if (parent instanceof ASTTypeParameters) { // if type parameter defined in the same < > + typeParameters = (ASTTypeParameters) parent; + } else if (parent instanceof ASTConstructorDeclaration + || parent instanceof ASTMethodDeclaration + || parent instanceof ASTClassOrInterfaceDeclaration) { + typeParameters = parent.getFirstChildOfType(ASTTypeParameters.class); + } + + if (typeParameters != null) { + for (int index = 0; index < typeParameters.jjtGetNumChildren(); ++index) { + String imageToCompareTo = typeParameters.jjtGetChild(index).getImage(); + if (imageToCompareTo != null && imageToCompareTo.equals(image)) { + return (ASTTypeParameter) typeParameters.jjtGetChild(index); + } + } + } + } + + return null; + } + /** * Check whether the supplied class name exists. */ diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index 2374459c6b..fd83920b51 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -14,6 +14,7 @@ import java.util.ArrayList; import java.util.List; import java.util.StringTokenizer; +import net.sourceforge.pmd.typeresolution.testdata.GenericFieldAccess; import org.apache.commons.io.IOUtils; import org.jaxen.JaxenException; import org.junit.Assert; @@ -65,7 +66,6 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.SuperClassA2; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.SuperClassB; - public class ClassTypeResolverTest { @Test @@ -81,7 +81,7 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = parseAndTypeResolveForClass15(ArrayListFound.class); assertEquals(ArrayListFound.class, acu.getFirstDescendantOfType(ASTTypeDeclaration.class).getType()); assertEquals(ArrayListFound.class, - acu.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getType()); + acu.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getType()); ASTImportDeclaration id = acu.getFirstDescendantOfType(ASTImportDeclaration.class); assertEquals("java.util", id.getPackage().getName()); assertEquals(ArrayList.class, id.getType()); @@ -117,12 +117,12 @@ public class ClassTypeResolverTest { ASTTypeDeclaration typeDeclaration = (ASTTypeDeclaration) acu.jjtGetChild(1); assertEquals(ExtraTopLevelClass.class, typeDeclaration.getType()); assertEquals(ExtraTopLevelClass.class, - typeDeclaration.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getType()); + typeDeclaration.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getType()); // Second class typeDeclaration = (ASTTypeDeclaration) acu.jjtGetChild(2); assertEquals(theExtraTopLevelClass, typeDeclaration.getType()); assertEquals(theExtraTopLevelClass, - typeDeclaration.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getType()); + typeDeclaration.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getType()); } @Test @@ -137,7 +137,7 @@ public class ClassTypeResolverTest { assertEquals(InnerClass.class, outerClassDeclaration.getType()); // Inner class assertEquals(theInnerClass, - outerClassDeclaration.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getType()); + outerClassDeclaration.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getType()); // Method parameter as inner class ASTFormalParameter formalParameter = typeDeclaration.getFirstDescendantOfType(ASTFormalParameter.class); assertEquals(theInnerClass, formalParameter.getTypeNode().getType()); @@ -152,8 +152,9 @@ public class ClassTypeResolverTest { @Test public void testInnerClassNotCompiled() throws Exception { Node acu = parseAndTypeResolveForString("public class TestInnerClass {\n" + " public void foo() {\n" - + " Statement statement = new Statement();\n" + " }\n" + " static class Statement {\n" - + " }\n" + "}", "1.8"); + + " Statement statement = new Statement();\n" + " " + + "}\n" + " static class Statement {\n" + + " }\n" + "}", "1.8"); ASTClassOrInterfaceType statement = acu.getFirstDescendantOfType(ASTClassOrInterfaceType.class); Assert.assertTrue(statement.isReferenceToClassSameCompilationUnit()); } @@ -171,7 +172,7 @@ public class ClassTypeResolverTest { assertEquals(AnonymousInnerClass.class, outerClassDeclaration.getType()); // Anonymous Inner class assertEquals(theAnonymousInnerClass, - outerClassDeclaration.getFirstDescendantOfType(ASTAllocationExpression.class).getType()); + outerClassDeclaration.getFirstDescendantOfType(ASTAllocationExpression.class).getType()); } @Test @@ -329,7 +330,8 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = parseAndTypeResolveForClass15(Promotion.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'unaryNumericPromotion']]//Expression[UnaryExpression]"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'unaryNumericPromotion']]//Expression[UnaryExpression]"), ASTExpression.class); int index = 0; @@ -350,7 +352,8 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = parseAndTypeResolveForClass15(Promotion.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'binaryNumericPromotion']]//Expression[AdditiveExpression]"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'binaryNumericPromotion']]//Expression[AdditiveExpression]"), ASTExpression.class); int index = 0; @@ -487,15 +490,18 @@ public class ClassTypeResolverTest { TypeNode.class)); expressions.addAll(convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'unaryNumericOperators']]//PostfixExpression"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'unaryNumericOperators']]//PostfixExpression"), TypeNode.class)); expressions.addAll(convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'unaryNumericOperators']]//PreIncrementExpression"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'unaryNumericOperators']]//PreIncrementExpression"), TypeNode.class)); expressions.addAll(convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'unaryNumericOperators']]//PreDecrementExpression"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'unaryNumericOperators']]//PreDecrementExpression"), TypeNode.class)); int index = 0; @@ -545,7 +551,8 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = parseAndTypeResolveForClass15(Operators.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'assignmentOperators']]//StatementExpression"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'assignmentOperators']]//StatementExpression"), ASTStatementExpression.class); int index = 0; @@ -649,12 +656,13 @@ public class ClassTypeResolverTest { assertEquals(SuperClassA.class, expressions.get(index++).getType()); assertEquals(SuperClassA.class, expressions.get(index++).getType()); assertEquals(SuperClassA.class, expressions.get(index++).getType()); - assertEquals(SuperExpression.class, ((TypeNode) expressions.get(index).jjtGetParent().jjtGetChild(0)).getType()); + assertEquals(SuperExpression.class, ((TypeNode) expressions.get(index).jjtGetParent().jjtGetChild(0)).getType + ()); assertEquals(SuperClassA.class, ((TypeNode) expressions.get(index++).jjtGetParent().jjtGetChild(1)).getType()); assertEquals(SuperExpression.class, expressions.get(index++).getType()); assertEquals(SuperExpression.class, expressions.get(index++).getType()); - + // Make sure we got them all assertEquals("All expressions not tested", index, expressions.size()); @@ -749,6 +757,61 @@ public class ClassTypeResolverTest { assertEquals("All expressions not tested", index, expressions.size()); } + @Test + public void testGenericFieldAccess() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(GenericFieldAccess.class); + + List expressions = convertList( + acu.findChildNodesWithXPath("//StatementExpression/PrimaryExpression"), + AbstractJavaTypeNode.class); + + + int index = 0; + + // generic.first = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 0)); + + // generic.second = new Double(0); + assertEquals(Double.class, expressions.get(index).getType()); + assertEquals(Double.class, getChildType(expressions.get(index++), 0)); + + // param.generic.first = new Character('c'); + assertEquals(Character.class, expressions.get(index).getType()); + assertEquals(Character.class, getChildType(expressions.get(index++), 0)); + + // local.generic.second = new Float(0); + assertEquals(Float.class, expressions.get(index).getType()); + assertEquals(Float.class, getChildType(expressions.get(index++), 0)); + + // generic.generic.generic.generic.first = new Double(0); + assertEquals(Double.class, expressions.get(index).getType()); + assertEquals(Double.class, getChildType(expressions.get(index++), 0)); + + // param.first = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + + // local.second = new Long(0); + assertEquals(Long.class, expressions.get(index).getType()); + assertEquals(Long.class, getChildType(expressions.get(index++), 0)); + + // classGeneric = null; // Double + assertEquals(Double.class, expressions.get(index).getType()); + assertEquals(Double.class, getChildType(expressions.get(index++), 0)); + + // localGeneric = null; // Character + assertEquals(Character.class, expressions.get(index).getType()); + assertEquals(Character.class, getChildType(expressions.get(index++), 0)); + + // localGeneric = null; // Number + assertEquals(Number.class, expressions.get(index).getType()); + assertEquals(Number.class, getChildType(expressions.get(index++), 0)); + + // Make sure we got them all + assertEquals("All expressions not tested", index, expressions.size()); + } + private Class getSiblingType(Node node, int siblingIndex) { Node parent = node.jjtGetParent(); if (parent.jjtGetNumChildren() > siblingIndex) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java new file mode 100644 index 0000000000..1e927fda1d --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java @@ -0,0 +1,59 @@ +package net.sourceforge.pmd.typeresolution.testdata; + +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; + + +/* + * TODO: static field access not tested + * TODO: upper bounds: add extends, super, wild card and what not cases + * TODO: sources: nested + * TODO: diamond + * + * Add fields dependent on type arguments from class, method, have super, extends, wild card and what not + * + * TODO: cover (first) one being local or param or field + * TODO: generic from class, method, consturctor, type argument + * + */ + +public class GenericFieldAccess { + public GenericClass generic; + public GenericFieldAccess field; + public S classGeneric; + + public void foo(GenericClass param) { + GenericClass local = null; + M localGeneric = null; + + // access a generic field through member field + // Primary[Prefix[Name[generic.first]]] + generic.first = ""; + generic.second = new Double(0); + + // access a generic field whose type depends on indirect type arguments + // Primary[Prefix[Name[generic.generic.first]]] + param.generic.first = new Character('c'); + local.generic.second = new Float(0); + generic.generic.generic.generic.first = new Double(0); + + + // access a generic field through a local or a parameter + // Primary[Prefix[Name[param.first]]] + param.first = new Integer(0); + local.second = new Long(0); + + // access type dependant on class/method type arguments + // Primary[Prefix[Name[classGeneric]]] + classGeneric = null; // Double + localGeneric = null; // Character + + } + + public GenericFieldAccess() { + C localGeneric = null; + + // access type dependant on constructor type arguments + // Primary[Prefix[Name[localGeneric]]] + localGeneric = null; // Number + } +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass.java index 7f2e480764..6f43c42dc2 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass.java @@ -5,6 +5,8 @@ package net.sourceforge.pmd.typeresolution.testdata.dummytypes; -public class GenericClass { - public T a; +public class GenericClass { + public T first; + public S second; + public GenericClass generic; } From 7c04542861e51c752a86755738e2c51cd0e2c663 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Sat, 10 Jun 2017 15:45:13 +0200 Subject: [PATCH 02/20] Java: resolve type of field dependant on generic type argument --- .../typeresolution/ClassTypeResolver.java | 109 ++++++++++-------- .../typeresolution/ClassTypeResolverTest.java | 4 + .../testdata/GenericFieldAccess.java | 5 + 3 files changed, 71 insertions(+), 47 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index f401c31e94..033b33931f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -247,6 +247,15 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return super.visit(node, data); } + private static class ClassWrapper { + public final Class clazz; + public List genericArgs = null; + + public ClassWrapper(Class clazz) { + this.clazz = clazz; + } + } + @Override public Object visit(ASTName node, Object data) { @@ -276,15 +285,16 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { String[] dotSplitImage = node.getImage().split("\\."); + // cases like EnclosingScope.something ... if (dotSplitImage.length == 1) { populateType(node, dotSplitImage[0]); } if (node.getType() == null) { - Class previousNameType = searchScopeForVariableType(node.getScope(), dotSplitImage[0]); - - List genericList = getTypeArgumentsFromASTType(searchScopeForVariableDeclaration(node.getScope(), dotSplitImage[0]) - .getDeclaratorId().getTypeNode()); + ClassWrapper previousNameType = getClassWrapperFromTypeNode( + searchScopeForVariableType(node.getScope(), dotSplitImage[0]), // get first name's class + searchScopeForVariableDeclaration(node.getScope(), dotSplitImage[0]) // get TypeNode + .getDeclaratorId().getTypeNode()); for (int i = 1; i < dotSplitImage.length; ++i) { if (previousNameType == null) { @@ -292,72 +302,77 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } // TODO: raw types not covered - Field field = getFieldFromClass(previousNameType, dotSplitImage[i]); - previousNameType = getGenericFieldType(genericList, field); - genericList = getGenericFieldTypeArguments(genericList, field); + Field field = searchClassForField(previousNameType.clazz, dotSplitImage[i]); + previousNameType = getNextClassWrapper(previousNameType, + field.getGenericType(), + field.getDeclaringClass()); } - node.setType(previousNameType); + node.setType(previousNameType.clazz); } } return super.visit(node, data); } - private List getGenericFieldTypeArguments(List previousTypeArguments, Field field) { - List newTypeArguments = new ArrayList<>(); + private ClassWrapper getNextClassWrapper(ClassWrapper previousWrapper, Type genericType, Class declaringClass) { + if (genericType instanceof Class) { + return new ClassWrapper((Class) genericType); + } else if (genericType instanceof ParameterizedType) { - if(field.getGenericType() instanceof ParameterizedType) { - Type[] typeArgs = ((ParameterizedType)field.getGenericType()).getActualTypeArguments(); - for(Type type : typeArgs) { - if(type instanceof TypeVariable) { - int ordinal = getTypeParameterOrdinal(field.getDeclaringClass(), ((TypeVariable) type).getName()); - if(ordinal != -1) { - newTypeArguments.add(previousTypeArguments.get(ordinal)); - } - } else { - newTypeArguments.add((Class)type); - } + ParameterizedType parameterizedType = (ParameterizedType) genericType; + ClassWrapper wrapper = new ClassWrapper((Class) parameterizedType.getRawType()); + wrapper.genericArgs = new ArrayList<>(); + + for (Type type : parameterizedType.getActualTypeArguments()) { + wrapper.genericArgs.add(getNextClassWrapper(previousWrapper, type, declaringClass)); } - } - return newTypeArguments; - } - - private Class getGenericFieldType(List typeArguments, Field field) { - Type fieldType = field.getGenericType(); - - if(fieldType instanceof Class) { - return (Class)fieldType; - } else if (fieldType instanceof ParameterizedType) { - return (Class)((ParameterizedType) fieldType).getRawType(); - } else if (fieldType instanceof TypeVariable) { - int ordinal = getTypeParameterOrdinal(field.getDeclaringClass(), ((TypeVariable) fieldType).getName()); - if(ordinal != -1) { - return typeArguments.get(ordinal); + return wrapper; + } else if (genericType instanceof TypeVariable) { + int ordinal = getTypeParameterOrdinal(declaringClass, ((TypeVariable) genericType).getName()); + if (ordinal != -1) { + return previousWrapper.genericArgs.get(ordinal); } } return null; } - private List getTypeArgumentsFromASTType(ASTType node) { - List typeList = new ArrayList<>(); + private ClassWrapper getClassWrapperFromTypeNode(Class variableType, TypeNode node) { + ClassWrapper classWrapper = new ClassWrapper(variableType); - if (node.jjtGetChild(0) instanceof ASTReferenceType) { + // this is really just a hack, so that down in the loop we can conveniently do some recursion + // this method should only have ASTType passed to it + if (node instanceof ASTType) { + node = (TypeNode) node.jjtGetChild(0); + } - ASTTypeArguments typeArgs = ((ASTClassOrInterfaceType) node.jjtGetChild(0).jjtGetChild(0)) - .getFirstChildOfType(ASTTypeArguments.class); + if (node instanceof ASTReferenceType) { + + ASTTypeArguments typeArgs = node.jjtGetChild(0).getFirstChildOfType(ASTTypeArguments.class); if (typeArgs != null) { + classWrapper.genericArgs = new ArrayList<>(); + + // TODO: raw types not supported // TODO: does not cover type arguments which are generic class for (int index = 0; index < typeArgs.jjtGetNumChildren(); ++index) { - typeList.add(((TypeNode) typeArgs.jjtGetChild(index)).getType()); + ASTTypeArgument typeArgumentNode = (ASTTypeArgument) typeArgs.jjtGetChild(index); + Class typeArgumentClass = typeArgumentNode.getType(); + + if (isGeneric(typeArgumentClass)) { + classWrapper.genericArgs.add( + getClassWrapperFromTypeNode(typeArgumentClass, + (TypeNode) typeArgumentNode.jjtGetChild(0))); + } else { + classWrapper.genericArgs.add(new ClassWrapper(typeArgumentClass)); + } } } } - return typeList; + return classWrapper; } private int getTypeParameterOrdinal(Class clazz, String parameterName) { @@ -401,8 +416,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // Nested class' inherited fields shadow enclosing variables if (scope instanceof ClassScope) { try { - Field inheritedField = getFieldFromClass(((ClassScope) scope).getClassDeclaration().getType(), - image); + Field inheritedField = searchClassForField(((ClassScope) scope).getClassDeclaration().getType(), + image); if (inheritedField != null) { return inheritedField.getType(); @@ -637,7 +652,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } } else if (previousChild != null && previousChild.getType() != null && currentChild.getImage() != null) { - Field field = getFieldFromClass(previousChild.getType(), currentChild.getImage()); + Field field = searchClassForField(previousChild.getType(), currentChild.getImage()); if (field != null) { currentChild.setType(field.getType()); } @@ -656,7 +671,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return data; } - private Field getFieldFromClass(Class clazz, String fieldName) { + private Field searchClassForField(Class clazz, String fieldName) { for ( /* empty */; clazz != null; clazz = clazz.getSuperclass()) { try { return clazz.getDeclaredField(fieldName); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index fd83920b51..f1e187b0c2 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -768,6 +768,10 @@ public class ClassTypeResolverTest { int index = 0; + //genericTypeArg.second.second = new Double(0); + assertEquals(Double.class, expressions.get(index).getType()); + assertEquals(Double.class, getChildType(expressions.get(index++), 0)); + // generic.first = ""; assertEquals(String.class, expressions.get(index).getType()); assertEquals(String.class, getChildType(expressions.get(index++), 0)); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java index 1e927fda1d..936a7ab36c 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java @@ -18,6 +18,7 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; public class GenericFieldAccess { public GenericClass generic; + public GenericClass> genericTypeArg; public GenericFieldAccess field; public S classGeneric; @@ -25,6 +26,10 @@ public class GenericFieldAccess { GenericClass local = null; M localGeneric = null; + // access a generic field whose type depends on a generic type argument + // Primary[Prefix[Name[genericTypeArg.second.second]]] + genericTypeArg.second.second = new Double(0); + // access a generic field through member field // Primary[Prefix[Name[generic.first]]] generic.first = ""; From 2be5b987bede3a2d90c5f9c65761c74b1773eae0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Sat, 10 Jun 2017 22:39:52 +0200 Subject: [PATCH 03/20] Java: resolve type of inherited generic fields --- .../typeresolution/ClassTypeResolver.java | 157 +++++++++--------- .../typeresolution/ClassTypeResolverTest.java | 28 +++- .../testdata/GenericFieldAccess.java | 36 +++- .../testdata/dummytypes/GenericClass2.java | 10 ++ 4 files changed, 147 insertions(+), 84 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 033b33931f..c6f57b7a9b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -37,6 +37,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression; import net.sourceforge.pmd.lang.java.ast.ASTExclusiveOrExpression; import net.sourceforge.pmd.lang.java.ast.ASTExpression; +import net.sourceforge.pmd.lang.java.ast.ASTExtendsList; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTInclusiveOrExpression; @@ -291,10 +292,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } if (node.getType() == null) { - ClassWrapper previousNameType = getClassWrapperFromTypeNode( - searchScopeForVariableType(node.getScope(), dotSplitImage[0]), // get first name's class - searchScopeForVariableDeclaration(node.getScope(), dotSplitImage[0]) // get TypeNode - .getDeclaratorId().getTypeNode()); + ClassWrapper previousNameType = + getClassWrapperOfVariableFromScope(node.getScope(), dotSplitImage[0]); for (int i = 1; i < dotSplitImage.length; ++i) { if (previousNameType == null) { @@ -303,9 +302,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // TODO: raw types not covered Field field = searchClassForField(previousNameType.clazz, dotSplitImage[i]); - previousNameType = getNextClassWrapper(previousNameType, - field.getGenericType(), - field.getDeclaringClass()); + previousNameType = getNextClassWrapper(previousNameType, field.getGenericType()); } node.setType(previousNameType.clazz); @@ -315,7 +312,65 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return super.visit(node, data); } - private ClassWrapper getNextClassWrapper(ClassWrapper previousWrapper, Type genericType, Class declaringClass) { + private ClassWrapper getClassWrapperOfVariableFromScope(Scope scope, String image) { + for (/* empty */; scope != null; scope = scope.getParent()) { + for (Map.Entry> entry + : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { + if (entry.getKey().getImage().equals(image)) { + ASTType typeNode = entry.getKey().getDeclaratorId().getTypeNode(); + + if(typeNode.jjtGetChild(0) instanceof ASTReferenceType) { + return getClassWrapperOfASTNode((ASTClassOrInterfaceType)typeNode.jjtGetChild(0).jjtGetChild(0)); + } else { // primitive type + return new ClassWrapper(typeNode.getType()); + } + } + } + + // Nested class' inherited fields shadow enclosing variables + if (scope instanceof ClassScope) { + try { + ClassScope classScope = (ClassScope) scope; + + Field inheritedField = searchClassForField(classScope.getClassDeclaration().getType(), + image); + + if (inheritedField != null) { + ClassWrapper superClass = getClassWrapperOfASTNode( + (ASTClassOrInterfaceType) classScope.getClassDeclaration().getNode() + .getFirstChildOfType(ASTExtendsList.class).jjtGetChild(0) + ); + + return getClassOfInheritedField(superClass, image); + } + } catch (ClassCastException e) { + // if there is an anonymous class, getClassDeclaration().getType() will throw + // TODO: maybe there is a better way to handle this, maybe this hides bugs + } + } + } + + return null; + } + + private ClassWrapper getClassOfInheritedField(ClassWrapper inheritedClass, String fieldImage) { + while(true) { + try { + Field field = inheritedClass.clazz.getDeclaredField(fieldImage); + return getNextClassWrapper(inheritedClass, field.getGenericType()); + } catch (NoSuchFieldException e) { /* swallow */ } + + Type genericSuperClass = inheritedClass.clazz.getGenericSuperclass(); + + if(genericSuperClass == null) { + return null; + } + + inheritedClass = getNextClassWrapper(inheritedClass, inheritedClass.clazz.getGenericSuperclass()); + } + } + + private ClassWrapper getNextClassWrapper(ClassWrapper previousWrapper, Type genericType) { if (genericType instanceof Class) { return new ClassWrapper((Class) genericType); } else if (genericType instanceof ParameterizedType) { @@ -325,12 +380,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { wrapper.genericArgs = new ArrayList<>(); for (Type type : parameterizedType.getActualTypeArguments()) { - wrapper.genericArgs.add(getNextClassWrapper(previousWrapper, type, declaringClass)); + wrapper.genericArgs.add(getNextClassWrapper(previousWrapper, type)); } return wrapper; } else if (genericType instanceof TypeVariable) { - int ordinal = getTypeParameterOrdinal(declaringClass, ((TypeVariable) genericType).getName()); + int ordinal = getTypeParameterOrdinal(previousWrapper.clazz, ((TypeVariable) genericType).getName()); if (ordinal != -1) { return previousWrapper.genericArgs.get(ordinal); } @@ -339,37 +394,29 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return null; } - private ClassWrapper getClassWrapperFromTypeNode(Class variableType, TypeNode node) { - ClassWrapper classWrapper = new ClassWrapper(variableType); + private ClassWrapper getClassWrapperOfASTNode(ASTClassOrInterfaceType node) { + ClassWrapper classWrapper = new ClassWrapper(node.getType()); - // this is really just a hack, so that down in the loop we can conveniently do some recursion - // this method should only have ASTType passed to it - if (node instanceof ASTType) { - node = (TypeNode) node.jjtGetChild(0); - } + ASTTypeArguments typeArgs = node.getFirstChildOfType(ASTTypeArguments.class); - if (node instanceof ASTReferenceType) { + if (typeArgs != null) { + classWrapper.genericArgs = new ArrayList<>(); - ASTTypeArguments typeArgs = node.jjtGetChild(0).getFirstChildOfType(ASTTypeArguments.class); + for (int index = 0; index < typeArgs.jjtGetNumChildren(); ++index) { + ASTTypeArgument typeArgumentNode = (ASTTypeArgument) typeArgs.jjtGetChild(index); + Class typeArgumentClass = typeArgumentNode.getType(); - if (typeArgs != null) { - classWrapper.genericArgs = new ArrayList<>(); - - // TODO: raw types not supported - // TODO: does not cover type arguments which are generic class - for (int index = 0; index < typeArgs.jjtGetNumChildren(); ++index) { - ASTTypeArgument typeArgumentNode = (ASTTypeArgument) typeArgs.jjtGetChild(index); - Class typeArgumentClass = typeArgumentNode.getType(); - - if (isGeneric(typeArgumentClass)) { - classWrapper.genericArgs.add( - getClassWrapperFromTypeNode(typeArgumentClass, - (TypeNode) typeArgumentNode.jjtGetChild(0))); - } else { - classWrapper.genericArgs.add(new ClassWrapper(typeArgumentClass)); - } + if (isGeneric(typeArgumentClass)) { + classWrapper.genericArgs.add( + getClassWrapperOfASTNode((ASTClassOrInterfaceType) + typeArgumentNode.jjtGetChild(0).jjtGetChild(0))); + } else { + classWrapper.genericArgs.add(new ClassWrapper(typeArgumentClass)); } } + + } else if (isGeneric(node.getType())) { // raw declaration of a generic class + //TODO: raw types } return classWrapper; @@ -391,46 +438,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return clazz.getTypeParameters().length != 0; } - private VariableNameDeclaration searchScopeForVariableDeclaration(Scope scope, String image) { - for (/* empty */; scope != null; scope = scope.getParent()) { - for (Map.Entry> entry - : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { - if (entry.getKey().getImage().equals(image)) { - return entry.getKey(); - } - } - } - - return null; - } - - private Class searchScopeForVariableType(Scope scope, String image) { - for (/* empty */; scope != null; scope = scope.getParent()) { - for (Map.Entry> entry - : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { - if (entry.getKey().getImage().equals(image)) { - return entry.getKey().getType(); - } - } - - // Nested class' inherited fields shadow enclosing variables - if (scope instanceof ClassScope) { - try { - Field inheritedField = searchClassForField(((ClassScope) scope).getClassDeclaration().getType(), - image); - - if (inheritedField != null) { - return inheritedField.getType(); - } - } catch (ClassCastException e) { - // if there is an anonymous class, getClassDeclaration().getType() will throw - // TODO: maybe there is a better way to handle this, maybe this hides bugs - } - } - } - - return null; - } @Override public Object visit(ASTFieldDeclaration node, Object data) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index f1e187b0c2..5f291000c7 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -768,15 +768,37 @@ public class ClassTypeResolverTest { int index = 0; + // fieldA = new Long(0); + assertEquals(Long.class, expressions.get(index).getType()); + assertEquals(Long.class, getChildType(expressions.get(index++), 0)); + + // fieldB.generic.second = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 0)); + + /*// rawGeneric.first = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + + // rawGeneric.second = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 0)); + + // rawGeneric.rawGeneric.first = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + */ + + //genericTypeArg.second.second = new Double(0); assertEquals(Double.class, expressions.get(index).getType()); assertEquals(Double.class, getChildType(expressions.get(index++), 0)); - // generic.first = ""; + // genericField.first = ""; assertEquals(String.class, expressions.get(index).getType()); assertEquals(String.class, getChildType(expressions.get(index++), 0)); - // generic.second = new Double(0); + // genericField.second = new Double(0); assertEquals(Double.class, expressions.get(index).getType()); assertEquals(Double.class, getChildType(expressions.get(index++), 0)); @@ -788,7 +810,7 @@ public class ClassTypeResolverTest { assertEquals(Float.class, expressions.get(index).getType()); assertEquals(Float.class, getChildType(expressions.get(index++), 0)); - // generic.generic.generic.generic.first = new Double(0); + // genericField.generic.generic.generic.first = new Double(0); assertEquals(Double.class, expressions.get(index).getType()); assertEquals(Double.class, getChildType(expressions.get(index++), 0)); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java index 936a7ab36c..8c5e7a3f8e 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java @@ -1,6 +1,7 @@ package net.sourceforge.pmd.typeresolution.testdata; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass2; /* @@ -16,9 +17,18 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; * */ -public class GenericFieldAccess { - public GenericClass generic; +class GenericSuperClassB { + public S fieldB; +} + +class GenericSuperClassA extends GenericSuperClassB> { + public T fieldA; +} + +public class GenericFieldAccess extends GenericSuperClassA { + public GenericClass genericField; public GenericClass> genericTypeArg; + public GenericClass2 rawGeneric; public GenericFieldAccess field; public S classGeneric; @@ -26,20 +36,34 @@ public class GenericFieldAccess { GenericClass local = null; M localGeneric = null; + + // test inherited generic + // Primary[Prefix[Name[generic.first]]] + fieldA = new Long(0); + fieldB.generic.second = ""; + + + // test raw types + // Primary[Prefix[Name[rawGeneric.first]]] + //rawGeneric.first = new Integer(0); + //rawGeneric.second = ""; + //rawGeneric.rawGeneric.first = new Integer(0); + + // access a generic field whose type depends on a generic type argument // Primary[Prefix[Name[genericTypeArg.second.second]]] genericTypeArg.second.second = new Double(0); // access a generic field through member field - // Primary[Prefix[Name[generic.first]]] - generic.first = ""; - generic.second = new Double(0); + // Primary[Prefix[Name[genericField.first]]] + genericField.first = ""; + genericField.second = new Double(0); // access a generic field whose type depends on indirect type arguments // Primary[Prefix[Name[generic.generic.first]]] param.generic.first = new Character('c'); local.generic.second = new Float(0); - generic.generic.generic.generic.first = new Double(0); + genericField.generic.generic.generic.first = new Double(0); // access a generic field through a local or a parameter diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java new file mode 100644 index 0000000000..73436114a1 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java @@ -0,0 +1,10 @@ +package net.sourceforge.pmd.typeresolution.testdata.dummytypes; + +public class GenericClass2> { + public A first; + public B second; + public C third; + public D fourth; + public GenericClass rawGeneric; +} From f4e6a1716c7869f965c597672deffd51c7f80f83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Sun, 11 Jun 2017 15:29:11 +0200 Subject: [PATCH 04/20] Java: resolve raw generic types --- .../typeresolution/ClassTypeResolver.java | 54 ++++++++++++++----- .../typeresolution/ClassTypeResolverTest.java | 17 ++++-- .../typeresolution/testdata/FieldAccess.java | 2 + .../testdata/GenericFieldAccess.java | 24 ++++----- .../testdata/dummytypes/GenericClass2.java | 9 +++- 5 files changed, 75 insertions(+), 31 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index c6f57b7a9b..f246b3eec7 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -300,7 +300,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { break; } - // TODO: raw types not covered Field field = searchClassForField(previousNameType.clazz, dotSplitImage[i]); previousNameType = getNextClassWrapper(previousNameType, field.getGenericType()); } @@ -319,8 +318,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (entry.getKey().getImage().equals(image)) { ASTType typeNode = entry.getKey().getDeclaratorId().getTypeNode(); - if(typeNode.jjtGetChild(0) instanceof ASTReferenceType) { - return getClassWrapperOfASTNode((ASTClassOrInterfaceType)typeNode.jjtGetChild(0).jjtGetChild(0)); + if (typeNode.jjtGetChild(0) instanceof ASTReferenceType) { + return getClassWrapperOfASTNode((ASTClassOrInterfaceType) typeNode.jjtGetChild(0).jjtGetChild + (0)); } else { // primitive type return new ClassWrapper(typeNode.getType()); } @@ -354,7 +354,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } private ClassWrapper getClassOfInheritedField(ClassWrapper inheritedClass, String fieldImage) { - while(true) { + while (true) { try { Field field = inheritedClass.clazz.getDeclaredField(fieldImage); return getNextClassWrapper(inheritedClass, field.getGenericType()); @@ -362,7 +362,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { Type genericSuperClass = inheritedClass.clazz.getGenericSuperclass(); - if(genericSuperClass == null) { + if (genericSuperClass == null) { return null; } @@ -372,7 +372,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { private ClassWrapper getNextClassWrapper(ClassWrapper previousWrapper, Type genericType) { if (genericType instanceof Class) { - return new ClassWrapper((Class) genericType); + return getDefaultUpperBounds(previousWrapper, (Class)genericType); } else if (genericType instanceof ParameterizedType) { ParameterizedType parameterizedType = (ParameterizedType) genericType; @@ -395,11 +395,11 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } private ClassWrapper getClassWrapperOfASTNode(ASTClassOrInterfaceType node) { - ClassWrapper classWrapper = new ClassWrapper(node.getType()); ASTTypeArguments typeArgs = node.getFirstChildOfType(ASTTypeArguments.class); if (typeArgs != null) { + ClassWrapper classWrapper = new ClassWrapper(node.getType()); classWrapper.genericArgs = new ArrayList<>(); for (int index = 0; index < typeArgs.jjtGetNumChildren(); ++index) { @@ -409,17 +409,47 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (isGeneric(typeArgumentClass)) { classWrapper.genericArgs.add( getClassWrapperOfASTNode((ASTClassOrInterfaceType) - typeArgumentNode.jjtGetChild(0).jjtGetChild(0))); + typeArgumentNode.jjtGetChild(0).jjtGetChild(0))); } else { classWrapper.genericArgs.add(new ClassWrapper(typeArgumentClass)); } } + return classWrapper; } else if (isGeneric(node.getType())) { // raw declaration of a generic class - //TODO: raw types + return getDefaultUpperBounds(null, node.getType()); } - return classWrapper; + return new ClassWrapper(node.getType()); + } + + // this exists to avoid infinite recursion in some cases + private Map defaultUpperBounds = new HashMap<>(); + + private ClassWrapper getDefaultUpperBounds(ClassWrapper original, Class clazzToFill) { + if(defaultUpperBounds.containsKey(clazzToFill)) { + return defaultUpperBounds.get(clazzToFill); + } + + ClassWrapper wrapper = new ClassWrapper(clazzToFill); + + if(original == null) { + original = wrapper; + } + + wrapper.genericArgs = new ArrayList<>(); + + defaultUpperBounds.put(clazzToFill, wrapper); + + for (TypeVariable parameter : clazzToFill.getTypeParameters()) { + Type upperBound = parameter.getBounds()[0]; + + // TODO: fix self reference "< ... E extends Something ... >" + + wrapper.genericArgs.add(getNextClassWrapper(original, upperBound)); + } + + return wrapper; } private int getTypeParameterOrdinal(Class clazz, String parameterName) { @@ -626,8 +656,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { public Object visit(ASTPrimaryExpression primaryNode, Object data) { super.visit(primaryNode, data); - Class primaryNodeType = null; + Class primaryNodeType = null; AbstractJavaTypeNode previousChild = null; + ClassWrapper previousClassWraper = null; for (int childIndex = 0; childIndex < primaryNode.jjtGetNumChildren(); ++childIndex) { AbstractJavaTypeNode currentChild = (AbstractJavaTypeNode) primaryNode.jjtGetChild(childIndex); @@ -645,7 +676,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { currentChild.setType(typeDeclaration.getType()); } } - // Last token, because if 'super' is a Suffix, it'll have tokens '.' and 'super' } else if (currentChild.jjtGetLastToken().toString().equals("super")) { if (previousChild != null) { // Qualified 'super' expression diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index 5f291000c7..c5b74d6429 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -776,18 +776,27 @@ public class ClassTypeResolverTest { assertEquals(String.class, expressions.get(index).getType()); assertEquals(String.class, getChildType(expressions.get(index++), 0)); - /*// rawGeneric.first = new Integer(0); + + // rawGeneric.first = new Integer(0); assertEquals(Integer.class, expressions.get(index).getType()); assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); - // rawGeneric.second = ""; + // rawGeneric.second = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + + // rawGeneric.third = new Object(); + assertEquals(Object.class, expressions.get(index).getType()); + assertEquals(Object.class, getChildType(expressions.get(index++), 0)); + + // rawGeneric.fourth.second = ""; assertEquals(String.class, expressions.get(index).getType()); assertEquals(String.class, getChildType(expressions.get(index++), 0)); - // rawGeneric.rawGeneric.first = new Integer(0); + // rawGeneric.rawGeneric.second = new Integer(0); assertEquals(Integer.class, expressions.get(index).getType()); assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); - */ + //genericTypeArg.second.second = new Double(0); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java index 325e9d0ef6..1440fe56d1 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java @@ -14,6 +14,8 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.SuperClassB; /* * Note: inherited fields of a nested class shadow outer scope variables * Note: only if they are accessible! + * + * TODO: test static field access, array types */ public class FieldAccess extends SuperClassA { public int field; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java index 8c5e7a3f8e..d218c8b553 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java @@ -5,16 +5,15 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass2; /* - * TODO: static field access not tested - * TODO: upper bounds: add extends, super, wild card and what not cases - * TODO: sources: nested - * TODO: diamond + * TODO: test [generic static field, nested classes, shadowing, inherited raw types] + * TODO: add super, wild card and what not cases + * TODO: add not Name[...] cases + * TODO: add primitives, parameterized arrays + * TODO: diamond, type parmeter declarations can shadow Class declarations + * * * Add fields dependent on type arguments from class, method, have super, extends, wild card and what not * - * TODO: cover (first) one being local or param or field - * TODO: generic from class, method, consturctor, type argument - * */ class GenericSuperClassB { @@ -36,18 +35,18 @@ public class GenericFieldAccess extends GenericSuperClassA< GenericClass local = null; M localGeneric = null; - // test inherited generic // Primary[Prefix[Name[generic.first]]] fieldA = new Long(0); fieldB.generic.second = ""; - // test raw types // Primary[Prefix[Name[rawGeneric.first]]] - //rawGeneric.first = new Integer(0); - //rawGeneric.second = ""; - //rawGeneric.rawGeneric.first = new Integer(0); + rawGeneric.first = new Integer(0); + rawGeneric.second = new Integer(0); + rawGeneric.third = new Object(); + rawGeneric.fourth.second = ""; + rawGeneric.rawGeneric.second = new Integer(0); // access a generic field whose type depends on a generic type argument @@ -75,7 +74,6 @@ public class GenericFieldAccess extends GenericSuperClassA< // Primary[Prefix[Name[classGeneric]]] classGeneric = null; // Double localGeneric = null; // Character - } public GenericFieldAccess() { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java index 73436114a1..f9d3c6aab4 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java @@ -1,10 +1,15 @@ package net.sourceforge.pmd.typeresolution.testdata.dummytypes; public class GenericClass2> { + S extends String, + D extends GenericClass, + //, E extends GenericClass, + F extends GenericClass2> { public A first; public B second; public C third; public D fourth; - public GenericClass rawGeneric; + //public E fifth; // recursion + public F sixth; // recursion + public GenericClass2 rawGeneric; } From 18bca7459acb9465ad6cb16d5fda579f125156b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Sun, 11 Jun 2017 17:29:11 +0200 Subject: [PATCH 05/20] Java: resolve type or wildcards and lower bounds --- .../typeresolution/ClassTypeResolver.java | 51 +++++++++++++++---- .../typeresolution/ClassTypeResolverTest.java | 41 +++++++++++++++ .../testdata/GenericFieldAccess.java | 34 +++++++++++-- .../testdata/dummytypes/StaticFields.java | 10 ++++ 4 files changed, 121 insertions(+), 15 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index f246b3eec7..a747ae259b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -8,6 +8,7 @@ import java.lang.reflect.Field; 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; @@ -17,6 +18,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import net.sourceforge.pmd.lang.ast.AbstractNode; +import net.sourceforge.pmd.lang.ast.GenericToken; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTAdditiveExpression; import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; @@ -75,6 +77,7 @@ 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.JavaParserVisitorAdapter; +import net.sourceforge.pmd.lang.java.ast.Token; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.symboltable.ClassScope; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; @@ -304,7 +307,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { previousNameType = getNextClassWrapper(previousNameType, field.getGenericType()); } - node.setType(previousNameType.clazz); + if(previousNameType != null) { + node.setType(previousNameType.clazz); + } } } @@ -350,6 +355,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } } + return null; } @@ -389,6 +395,13 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (ordinal != -1) { return previousWrapper.genericArgs.get(ordinal); } + } else if (genericType instanceof WildcardType) { + Type[] wildcardUpperBounds = ((WildcardType)genericType).getUpperBounds(); + if(wildcardUpperBounds.length != 0) { // upper bound wildcard + return getNextClassWrapper(previousWrapper, wildcardUpperBounds[0]); + } else { // lower bound wildcard + return new ClassWrapper(Object.class); + } } return null; @@ -433,20 +446,22 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { ClassWrapper wrapper = new ClassWrapper(clazzToFill); - if(original == null) { - original = wrapper; - } - - wrapper.genericArgs = new ArrayList<>(); - defaultUpperBounds.put(clazzToFill, wrapper); - for (TypeVariable parameter : clazzToFill.getTypeParameters()) { - Type upperBound = parameter.getBounds()[0]; + if(isGeneric(clazzToFill)) { + if(original == null) { + original = wrapper; + } - // TODO: fix self reference "< ... E extends Something ... >" + wrapper.genericArgs = new ArrayList<>(); - wrapper.genericArgs.add(getNextClassWrapper(original, upperBound)); + for (TypeVariable parameter : clazzToFill.getTypeParameters()) { + Type upperBound = parameter.getBounds()[0]; + + // TODO: fix self reference "< ... E extends Something ... >" + + wrapper.genericArgs.add(getNextClassWrapper(original, upperBound)); + } } return wrapper; @@ -742,6 +757,19 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { public Object visit(ASTTypeArgument node, Object data) { super.visit(node, data); rollupTypeUnary(node); + + if(node.getType() == null) { + // ? extends Something + if(node.jjtGetFirstToken() instanceof Token + && ((Token) node.jjtGetFirstToken()).next.image.equals("extends")) { + + populateType(node, node.jjtGetLastToken().toString()); + + } else { // ? or ? super Something + node.setType(Object.class); + } + } + return data; } @@ -996,6 +1024,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // try generics // TODO: generic declarations can shadow type declarations ... :( + // TODO: incorrect if type parameter upper bound is generic // TODO: ? and ? super is not covered if (myType == null) { ASTTypeParameter parameter = getTypeParameterDeclaration(node, className); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index c5b74d6429..d6e6ab7bee 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -15,6 +15,7 @@ import java.util.List; import java.util.StringTokenizer; import net.sourceforge.pmd.typeresolution.testdata.GenericFieldAccess; +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.StaticFields; import org.apache.commons.io.IOUtils; import org.jaxen.JaxenException; import org.junit.Assert; @@ -768,6 +769,46 @@ public class ClassTypeResolverTest { int index = 0; + // superGeneric.first = ""; // Object + assertEquals(Object.class, expressions.get(index).getType()); + assertEquals(Object.class, getChildType(expressions.get(index++), 0)); + + // superGeneric.second = null; // Object + assertEquals(Object.class, expressions.get(index).getType()); + assertEquals(Object.class, getChildType(expressions.get(index++), 0)); + + // inheritedSuperGeneric.first = ""; // Object + assertEquals(Object.class, expressions.get(index).getType()); + assertEquals(Object.class, getChildType(expressions.get(index++), 0)); + + // inheritedSuperGeneric.second = null; // Object + assertEquals(Object.class, expressions.get(index).getType()); + assertEquals(Object.class, getChildType(expressions.get(index++), 0)); + + // upperBound.first = null; // Number + assertEquals(Number.class, expressions.get(index).getType()); + assertEquals(Number.class, getChildType(expressions.get(index++), 0)); + + // inheritedUpperBound.first = null; // String + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 0)); + + + + // parameterGeneric.first = ""; + //assertEquals(String.class, expressions.get(index).getType()); + //assertEquals(String.class, getChildType(expressions.get(index++), 0)); + + + // instanceFields.generic.first = ""; + //assertEquals(String.class, expressions.get(index).getType()); + //assertEquals(String.class, getChildType(expressions.get(index++), 0)); + + // staticGeneric.first = new Long(0); + //assertEquals(Long.class, expressions.get(index).getType()); + //assertEquals(Long.class, getChildType(expressions.get(index++), 0)); + + // fieldA = new Long(0); assertEquals(Long.class, expressions.get(index).getType()); assertEquals(Long.class, getChildType(expressions.get(index++), 0)); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java index d218c8b553..8a2cd130d8 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java @@ -2,11 +2,12 @@ package net.sourceforge.pmd.typeresolution.testdata; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass2; +import static net.sourceforge.pmd.typeresolution.testdata.dummytypes.StaticFields.instanceFields; +import static net.sourceforge.pmd.typeresolution.testdata.dummytypes.StaticFields.staticGeneric; /* - * TODO: test [generic static field, nested classes, shadowing, inherited raw types] - * TODO: add super, wild card and what not cases + * TODO: test [nested classes, shadowing] * TODO: add not Name[...] cases * TODO: add primitives, parameterized arrays * TODO: diamond, type parmeter declarations can shadow Class declarations @@ -22,19 +23,44 @@ class GenericSuperClassB { class GenericSuperClassA extends GenericSuperClassB> { public T fieldA; + public GenericClass inheritedSuperGeneric; + public GenericClass inheritedUpperBound; } -public class GenericFieldAccess extends GenericSuperClassA { +public class GenericFieldAccess, S extends Double> extends GenericSuperClassA { public GenericClass genericField; public GenericClass> genericTypeArg; public GenericClass2 rawGeneric; public GenericFieldAccess field; public S classGeneric; + public T parameterGeneric; + public GenericClass superGeneric; + public GenericClass upperBound; - public void foo(GenericClass param) { + public void astNameCases(GenericClass param) { GenericClass local = null; M localGeneric = null; + + // test ?, ? super Something, ? extends Something + // Primary[Prefix[Name[superGeneric.first]]] + superGeneric.first = ""; // Object + superGeneric.second = null; // Object + inheritedSuperGeneric.first = ""; // Object + inheritedSuperGeneric.second = null; // Object + + upperBound.first = null; // Number + inheritedUpperBound.first = null; // String + + // test type parameters extending generic types + // parameterGeneric.first = ""; + + // test static imports + // Primary[Prefix[Name[instanceFields.generic.first]]] + //instanceFields.generic.first = ""; + //staticGeneric.first = new Long(0); + + // test inherited generic // Primary[Prefix[Name[generic.first]]] fieldA = new Long(0); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java new file mode 100644 index 0000000000..bc3baaa9fb --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java @@ -0,0 +1,10 @@ +package net.sourceforge.pmd.typeresolution.testdata.dummytypes; + +public class StaticFields { + public static StaticFields instanceFields; + public static int staticPrimitive; + public static GenericClass staticGeneric; + + public long primitive; + public GenericClass generic; +} From fccbf294acf7460e66143641b32bbb0b4f424cf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Mon, 12 Jun 2017 15:36:26 +0200 Subject: [PATCH 06/20] Java: add TypeWrapper to TypeNode interface --- .../java/ast/AbstractJavaAccessTypeNode.java | 27 +++++-- .../lang/java/ast/AbstractJavaTypeNode.java | 29 ++++++-- .../pmd/lang/java/ast/TypeNode.java | 24 +++++-- .../typeresolution/ClassTypeResolver.java | 70 ++++++++----------- .../lang/java/typeresolution/TypeWrapper.java | 28 ++++++++ 5 files changed, 125 insertions(+), 53 deletions(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java index cb19cbbc36..a9a25fa667 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java @@ -4,9 +4,10 @@ package net.sourceforge.pmd.lang.java.ast; -public abstract class AbstractJavaAccessTypeNode extends AbstractJavaAccessNode implements TypeNode { +import net.sourceforge.pmd.lang.java.typeresolution.TypeWrapper; - private Class type; +public abstract class AbstractJavaAccessTypeNode extends AbstractJavaAccessNode implements TypeNode { + private TypeWrapper typeWrapper; public AbstractJavaAccessTypeNode(int i) { super(i); @@ -18,11 +19,29 @@ public abstract class AbstractJavaAccessTypeNode extends AbstractJavaAccessNode @Override public Class getType() { - return type; + if (typeWrapper != null) { + return typeWrapper.getClazz(); + } + + return null; } @Override public void setType(Class type) { - this.type = type; + if (typeWrapper == null) { + typeWrapper = new TypeWrapper(type); + } else { + typeWrapper.setClazz(type); + } + } + + @Override + public TypeWrapper getTypeWrapper() { + return typeWrapper; + } + + @Override + public void setTypeWrapper(TypeWrapper typeWrapper) { + this.typeWrapper = typeWrapper; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java index 039780fcf6..9528d9e502 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java @@ -4,15 +4,16 @@ package net.sourceforge.pmd.lang.java.ast; +import net.sourceforge.pmd.lang.java.typeresolution.TypeWrapper; + /** * An extension of the SimpleJavaNode which implements the TypeNode interface. - * + * * @see AbstractJavaNode * @see TypeNode */ public abstract class AbstractJavaTypeNode extends AbstractJavaNode implements TypeNode { - - private Class type; + private TypeWrapper typeWrapper; public AbstractJavaTypeNode(int i) { super(i); @@ -24,11 +25,29 @@ public abstract class AbstractJavaTypeNode extends AbstractJavaNode implements T @Override public Class getType() { - return type; + if (typeWrapper != null) { + return typeWrapper.getClazz(); + } + + return null; } @Override public void setType(Class type) { - this.type = type; + if (typeWrapper == null) { + typeWrapper = new TypeWrapper(type); + } else { + typeWrapper.setClazz(type); + } + } + + @Override + public TypeWrapper getTypeWrapper() { + return typeWrapper; + } + + @Override + public void setTypeWrapper(TypeWrapper typeWrapper) { + this.typeWrapper = typeWrapper; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java index 5ed05bb2a4..9a480c76a1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java @@ -5,6 +5,7 @@ package net.sourceforge.pmd.lang.java.ast; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.typeresolution.TypeWrapper; /** * This interface allows a Java Class to be associated with a node. @@ -13,16 +14,31 @@ public interface TypeNode extends Node { /** * Get the Java Class associated with this node. - * + * * @return The Java Class, may return null. */ Class getType(); + /** + * Get the TypeWrapper associated with this node. The Class object + * contained in the TypeWrapper will always be equal to that which + * is returned by getType(). + * + * @return The TypeWrapper, may return null + */ + TypeWrapper getTypeWrapper(); + + /** + * Set the TypeWrapper associated with this node. + * + * @param type A TypeWrapper object + */ + void setTypeWrapper(TypeWrapper type); + /** * Set the Java Class associated with this node. - * - * @param type - * A Java Class + * + * @param type A Java Class */ void setType(Class type); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index a747ae259b..d7d2078c85 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -18,7 +18,6 @@ import java.util.logging.Level; import java.util.logging.Logger; import net.sourceforge.pmd.lang.ast.AbstractNode; -import net.sourceforge.pmd.lang.ast.GenericToken; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTAdditiveExpression; import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; @@ -251,15 +250,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return super.visit(node, data); } - private static class ClassWrapper { - public final Class clazz; - public List genericArgs = null; - - public ClassWrapper(Class clazz) { - this.clazz = clazz; - } - } - @Override public Object visit(ASTName node, Object data) { @@ -295,7 +285,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } if (node.getType() == null) { - ClassWrapper previousNameType = + TypeWrapper previousNameType = getClassWrapperOfVariableFromScope(node.getScope(), dotSplitImage[0]); for (int i = 1; i < dotSplitImage.length; ++i) { @@ -303,12 +293,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { break; } - Field field = searchClassForField(previousNameType.clazz, dotSplitImage[i]); + Field field = searchClassForField(previousNameType.getClazz(), dotSplitImage[i]); previousNameType = getNextClassWrapper(previousNameType, field.getGenericType()); } if(previousNameType != null) { - node.setType(previousNameType.clazz); + node.setType(previousNameType.getClazz()); } } } @@ -316,7 +306,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return super.visit(node, data); } - private ClassWrapper getClassWrapperOfVariableFromScope(Scope scope, String image) { + private TypeWrapper getClassWrapperOfVariableFromScope(Scope scope, String image) { for (/* empty */; scope != null; scope = scope.getParent()) { for (Map.Entry> entry : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { @@ -327,7 +317,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return getClassWrapperOfASTNode((ASTClassOrInterfaceType) typeNode.jjtGetChild(0).jjtGetChild (0)); } else { // primitive type - return new ClassWrapper(typeNode.getType()); + return new TypeWrapper(typeNode.getType()); } } } @@ -341,7 +331,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { image); if (inheritedField != null) { - ClassWrapper superClass = getClassWrapperOfASTNode( + TypeWrapper superClass = getClassWrapperOfASTNode( (ASTClassOrInterfaceType) classScope.getClassDeclaration().getNode() .getFirstChildOfType(ASTExtendsList.class).jjtGetChild(0) ); @@ -359,92 +349,92 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return null; } - private ClassWrapper getClassOfInheritedField(ClassWrapper inheritedClass, String fieldImage) { + private TypeWrapper getClassOfInheritedField(TypeWrapper inheritedClass, String fieldImage) { while (true) { try { - Field field = inheritedClass.clazz.getDeclaredField(fieldImage); + Field field = inheritedClass.getClazz().getDeclaredField(fieldImage); return getNextClassWrapper(inheritedClass, field.getGenericType()); } catch (NoSuchFieldException e) { /* swallow */ } - Type genericSuperClass = inheritedClass.clazz.getGenericSuperclass(); + Type genericSuperClass = inheritedClass.getClazz().getGenericSuperclass(); if (genericSuperClass == null) { return null; } - inheritedClass = getNextClassWrapper(inheritedClass, inheritedClass.clazz.getGenericSuperclass()); + inheritedClass = getNextClassWrapper(inheritedClass, inheritedClass.getClazz().getGenericSuperclass()); } } - private ClassWrapper getNextClassWrapper(ClassWrapper previousWrapper, Type genericType) { + private TypeWrapper getNextClassWrapper(TypeWrapper previousWrapper, Type genericType) { if (genericType instanceof Class) { return getDefaultUpperBounds(previousWrapper, (Class)genericType); } else if (genericType instanceof ParameterizedType) { ParameterizedType parameterizedType = (ParameterizedType) genericType; - ClassWrapper wrapper = new ClassWrapper((Class) parameterizedType.getRawType()); - wrapper.genericArgs = new ArrayList<>(); + TypeWrapper wrapper = new TypeWrapper((Class) parameterizedType.getRawType()); + wrapper.setGenericArgs(new ArrayList()); for (Type type : parameterizedType.getActualTypeArguments()) { - wrapper.genericArgs.add(getNextClassWrapper(previousWrapper, type)); + wrapper.getGenericArgs().add(getNextClassWrapper(previousWrapper, type)); } return wrapper; } else if (genericType instanceof TypeVariable) { - int ordinal = getTypeParameterOrdinal(previousWrapper.clazz, ((TypeVariable) genericType).getName()); + int ordinal = getTypeParameterOrdinal(previousWrapper.getClazz(), ((TypeVariable) genericType).getName()); if (ordinal != -1) { - return previousWrapper.genericArgs.get(ordinal); + return previousWrapper.getGenericArgs().get(ordinal); } } else if (genericType instanceof WildcardType) { Type[] wildcardUpperBounds = ((WildcardType)genericType).getUpperBounds(); if(wildcardUpperBounds.length != 0) { // upper bound wildcard return getNextClassWrapper(previousWrapper, wildcardUpperBounds[0]); } else { // lower bound wildcard - return new ClassWrapper(Object.class); + return new TypeWrapper(Object.class); } } return null; } - private ClassWrapper getClassWrapperOfASTNode(ASTClassOrInterfaceType node) { + private TypeWrapper getClassWrapperOfASTNode(ASTClassOrInterfaceType node) { ASTTypeArguments typeArgs = node.getFirstChildOfType(ASTTypeArguments.class); if (typeArgs != null) { - ClassWrapper classWrapper = new ClassWrapper(node.getType()); - classWrapper.genericArgs = new ArrayList<>(); + TypeWrapper typeWrapper = new TypeWrapper(node.getType()); + typeWrapper.setGenericArgs(new ArrayList()); for (int index = 0; index < typeArgs.jjtGetNumChildren(); ++index) { ASTTypeArgument typeArgumentNode = (ASTTypeArgument) typeArgs.jjtGetChild(index); Class typeArgumentClass = typeArgumentNode.getType(); if (isGeneric(typeArgumentClass)) { - classWrapper.genericArgs.add( + typeWrapper.getGenericArgs().add( getClassWrapperOfASTNode((ASTClassOrInterfaceType) typeArgumentNode.jjtGetChild(0).jjtGetChild(0))); } else { - classWrapper.genericArgs.add(new ClassWrapper(typeArgumentClass)); + typeWrapper.getGenericArgs().add(new TypeWrapper(typeArgumentClass)); } } - return classWrapper; + return typeWrapper; } else if (isGeneric(node.getType())) { // raw declaration of a generic class return getDefaultUpperBounds(null, node.getType()); } - return new ClassWrapper(node.getType()); + return new TypeWrapper(node.getType()); } // this exists to avoid infinite recursion in some cases - private Map defaultUpperBounds = new HashMap<>(); + private Map defaultUpperBounds = new HashMap<>(); - private ClassWrapper getDefaultUpperBounds(ClassWrapper original, Class clazzToFill) { + private TypeWrapper getDefaultUpperBounds(TypeWrapper original, Class clazzToFill) { if(defaultUpperBounds.containsKey(clazzToFill)) { return defaultUpperBounds.get(clazzToFill); } - ClassWrapper wrapper = new ClassWrapper(clazzToFill); + TypeWrapper wrapper = new TypeWrapper(clazzToFill); defaultUpperBounds.put(clazzToFill, wrapper); @@ -453,14 +443,14 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { original = wrapper; } - wrapper.genericArgs = new ArrayList<>(); + wrapper.setGenericArgs(new ArrayList()); for (TypeVariable parameter : clazzToFill.getTypeParameters()) { Type upperBound = parameter.getBounds()[0]; // TODO: fix self reference "< ... E extends Something ... >" - wrapper.genericArgs.add(getNextClassWrapper(original, upperBound)); + wrapper.getGenericArgs().add(getNextClassWrapper(original, upperBound)); } } @@ -673,7 +663,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { Class primaryNodeType = null; AbstractJavaTypeNode previousChild = null; - ClassWrapper previousClassWraper = null; + TypeWrapper previousClassWraper = null; for (int childIndex = 0; childIndex < primaryNode.jjtGetNumChildren(); ++childIndex) { AbstractJavaTypeNode currentChild = (AbstractJavaTypeNode) primaryNode.jjtGetChild(childIndex); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java new file mode 100644 index 0000000000..44b6875154 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java @@ -0,0 +1,28 @@ +package net.sourceforge.pmd.lang.java.typeresolution; + +import java.util.List; + +public class TypeWrapper { + private Class clazz; + private List genericArgs = null; + + public TypeWrapper(Class clazz) { + this.clazz = clazz; + } + + public Class getClazz() { + return clazz; + } + + public void setClazz(Class clazz) { + this.clazz = clazz; + } + + public List getGenericArgs() { + return genericArgs; + } + + public void setGenericArgs(List genericArgs) { + this.genericArgs = genericArgs; + } +} From 692fc3acae05f118dad3800e3fa7629e8b0bb34c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Mon, 12 Jun 2017 21:51:56 +0200 Subject: [PATCH 07/20] Java: fix generic type parameter upper bound bug --- .../java/ast/AbstractJavaAccessTypeNode.java | 2 +- .../lang/java/ast/AbstractJavaTypeNode.java | 2 +- .../typeresolution/ClassTypeResolver.java | 99 ++++++++----------- .../lang/java/typeresolution/TypeWrapper.java | 7 +- .../typeresolution/ClassTypeResolverTest.java | 6 +- .../testdata/GenericFieldAccess.java | 27 ++--- 6 files changed, 67 insertions(+), 76 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java index a9a25fa667..ce14ecb957 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java @@ -20,7 +20,7 @@ public abstract class AbstractJavaAccessTypeNode extends AbstractJavaAccessNode @Override public Class getType() { if (typeWrapper != null) { - return typeWrapper.getClazz(); + return typeWrapper.getType(); } return null; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java index 9528d9e502..6a173c3cd7 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java @@ -26,7 +26,7 @@ public abstract class AbstractJavaTypeNode extends AbstractJavaNode implements T @Override public Class getType() { if (typeWrapper != null) { - return typeWrapper.getClazz(); + return typeWrapper.getType(); } return null; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index d7d2078c85..978c5be07a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -229,6 +229,19 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { typeName = parent.getImage() + "$" + anonymousClassCounter; } populateType(node, typeName); + + ASTTypeArguments typeArguments = node.getFirstChildOfType(ASTTypeArguments.class); + + if (typeArguments != null) { + for (int index = 0; index < typeArguments.jjtGetNumChildren(); ++index) { + node.getTypeWrapper().getGenericArgs().add( + ((TypeNode) typeArguments.jjtGetChild(index)).getTypeWrapper() + ); + } + } else if(isGeneric(node.getType()) && node.getTypeWrapper().getGenericArgs().size() == 0) { + node.setTypeWrapper(getDefaultUpperBounds(null, node.getType())); + } + return data; } @@ -293,12 +306,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { break; } - Field field = searchClassForField(previousNameType.getClazz(), dotSplitImage[i]); + Field field = searchClassForField(previousNameType.getType(), dotSplitImage[i]); previousNameType = getNextClassWrapper(previousNameType, field.getGenericType()); } - if(previousNameType != null) { - node.setType(previousNameType.getClazz()); + if (previousNameType != null) { + node.setTypeWrapper(previousNameType); } } } @@ -314,8 +327,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { ASTType typeNode = entry.getKey().getDeclaratorId().getTypeNode(); if (typeNode.jjtGetChild(0) instanceof ASTReferenceType) { - return getClassWrapperOfASTNode((ASTClassOrInterfaceType) typeNode.jjtGetChild(0).jjtGetChild - (0)); + return ((ASTClassOrInterfaceType) typeNode.jjtGetChild(0).jjtGetChild(0)) + .getTypeWrapper(); } else { // primitive type return new TypeWrapper(typeNode.getType()); } @@ -331,10 +344,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { image); if (inheritedField != null) { - TypeWrapper superClass = getClassWrapperOfASTNode( - (ASTClassOrInterfaceType) classScope.getClassDeclaration().getNode() - .getFirstChildOfType(ASTExtendsList.class).jjtGetChild(0) - ); + TypeWrapper superClass = + ((ASTClassOrInterfaceType) classScope.getClassDeclaration().getNode() + .getFirstChildOfType(ASTExtendsList.class).jjtGetChild(0)).getTypeWrapper(); return getClassOfInheritedField(superClass, image); } @@ -352,23 +364,23 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { private TypeWrapper getClassOfInheritedField(TypeWrapper inheritedClass, String fieldImage) { while (true) { try { - Field field = inheritedClass.getClazz().getDeclaredField(fieldImage); + Field field = inheritedClass.getType().getDeclaredField(fieldImage); return getNextClassWrapper(inheritedClass, field.getGenericType()); } catch (NoSuchFieldException e) { /* swallow */ } - Type genericSuperClass = inheritedClass.getClazz().getGenericSuperclass(); + Type genericSuperClass = inheritedClass.getType().getGenericSuperclass(); if (genericSuperClass == null) { return null; } - inheritedClass = getNextClassWrapper(inheritedClass, inheritedClass.getClazz().getGenericSuperclass()); + inheritedClass = getNextClassWrapper(inheritedClass, inheritedClass.getType().getGenericSuperclass()); } } private TypeWrapper getNextClassWrapper(TypeWrapper previousWrapper, Type genericType) { if (genericType instanceof Class) { - return getDefaultUpperBounds(previousWrapper, (Class)genericType); + return getDefaultUpperBounds(previousWrapper, (Class) genericType); } else if (genericType instanceof ParameterizedType) { ParameterizedType parameterizedType = (ParameterizedType) genericType; @@ -381,13 +393,13 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return wrapper; } else if (genericType instanceof TypeVariable) { - int ordinal = getTypeParameterOrdinal(previousWrapper.getClazz(), ((TypeVariable) genericType).getName()); + int ordinal = getTypeParameterOrdinal(previousWrapper.getType(), ((TypeVariable) genericType).getName()); if (ordinal != -1) { return previousWrapper.getGenericArgs().get(ordinal); } } else if (genericType instanceof WildcardType) { - Type[] wildcardUpperBounds = ((WildcardType)genericType).getUpperBounds(); - if(wildcardUpperBounds.length != 0) { // upper bound wildcard + Type[] wildcardUpperBounds = ((WildcardType) genericType).getUpperBounds(); + if (wildcardUpperBounds.length != 0) { // upper bound wildcard return getNextClassWrapper(previousWrapper, wildcardUpperBounds[0]); } else { // lower bound wildcard return new TypeWrapper(Object.class); @@ -397,40 +409,11 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return null; } - private TypeWrapper getClassWrapperOfASTNode(ASTClassOrInterfaceType node) { - - ASTTypeArguments typeArgs = node.getFirstChildOfType(ASTTypeArguments.class); - - if (typeArgs != null) { - TypeWrapper typeWrapper = new TypeWrapper(node.getType()); - typeWrapper.setGenericArgs(new ArrayList()); - - for (int index = 0; index < typeArgs.jjtGetNumChildren(); ++index) { - ASTTypeArgument typeArgumentNode = (ASTTypeArgument) typeArgs.jjtGetChild(index); - Class typeArgumentClass = typeArgumentNode.getType(); - - if (isGeneric(typeArgumentClass)) { - typeWrapper.getGenericArgs().add( - getClassWrapperOfASTNode((ASTClassOrInterfaceType) - typeArgumentNode.jjtGetChild(0).jjtGetChild(0))); - } else { - typeWrapper.getGenericArgs().add(new TypeWrapper(typeArgumentClass)); - } - } - - return typeWrapper; - } else if (isGeneric(node.getType())) { // raw declaration of a generic class - return getDefaultUpperBounds(null, node.getType()); - } - - return new TypeWrapper(node.getType()); - } - // this exists to avoid infinite recursion in some cases private Map defaultUpperBounds = new HashMap<>(); private TypeWrapper getDefaultUpperBounds(TypeWrapper original, Class clazzToFill) { - if(defaultUpperBounds.containsKey(clazzToFill)) { + if (defaultUpperBounds.containsKey(clazzToFill)) { return defaultUpperBounds.get(clazzToFill); } @@ -438,8 +421,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { defaultUpperBounds.put(clazzToFill, wrapper); - if(isGeneric(clazzToFill)) { - if(original == null) { + if (isGeneric(clazzToFill)) { + if (original == null) { original = wrapper; } @@ -470,7 +453,11 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } private boolean isGeneric(Class clazz) { - return clazz.getTypeParameters().length != 0; + if(clazz != null) { + return clazz.getTypeParameters().length != 0; + } + + return false; } @@ -748,13 +735,13 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { super.visit(node, data); rollupTypeUnary(node); - if(node.getType() == null) { + if (node.getType() == null) { // ? extends Something - if(node.jjtGetFirstToken() instanceof Token + if (node.jjtGetFirstToken() instanceof Token && ((Token) node.jjtGetFirstToken()).next.image.equals("extends")) { populateType(node, node.jjtGetLastToken().toString()); - + } else { // ? or ? super Something node.setType(Object.class); } @@ -901,7 +888,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (node.jjtGetNumChildren() >= 1) { Node child = node.jjtGetChild(0); if (child instanceof TypeNode) { - typeNode.setType(((TypeNode) child).getType()); + typeNode.setTypeWrapper(((TypeNode) child).getTypeWrapper()); } } } @@ -1019,11 +1006,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (myType == null) { ASTTypeParameter parameter = getTypeParameterDeclaration(node, className); if (parameter != null) { - myType = parameter.getType(); + node.setTypeWrapper(parameter.getTypeWrapper()); } - } - - if (myType != null) { + } else { node.setType(myType); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java index 44b6875154..e0509727ef 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java @@ -1,5 +1,6 @@ package net.sourceforge.pmd.lang.java.typeresolution; +import java.util.ArrayList; import java.util.List; public class TypeWrapper { @@ -10,7 +11,7 @@ public class TypeWrapper { this.clazz = clazz; } - public Class getClazz() { + public Class getType() { return clazz; } @@ -19,6 +20,10 @@ public class TypeWrapper { } public List getGenericArgs() { + if(genericArgs == null) { + genericArgs = new ArrayList<>(); + } + return genericArgs; } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index d6e6ab7bee..662e68d881 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -795,9 +795,9 @@ public class ClassTypeResolverTest { - // parameterGeneric.first = ""; - //assertEquals(String.class, expressions.get(index).getType()); - //assertEquals(String.class, getChildType(expressions.get(index++), 0)); + // parameterGeneric.second.second = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); // instanceFields.generic.first = ""; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java index 8a2cd130d8..d8b0079c98 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java @@ -17,23 +17,13 @@ import static net.sourceforge.pmd.typeresolution.testdata.dummytypes.StaticField * */ -class GenericSuperClassB { - public S fieldB; -} - -class GenericSuperClassA extends GenericSuperClassB> { - public T fieldA; - public GenericClass inheritedSuperGeneric; - public GenericClass inheritedUpperBound; -} - -public class GenericFieldAccess, S extends Double> extends GenericSuperClassA { +public class GenericFieldAccess>, S extends Double> extends GenericSuperClassA { + public T parameterGeneric; public GenericClass genericField; public GenericClass> genericTypeArg; public GenericClass2 rawGeneric; public GenericFieldAccess field; public S classGeneric; - public T parameterGeneric; public GenericClass superGeneric; public GenericClass upperBound; @@ -53,7 +43,8 @@ public class GenericFieldAccess, S exten inheritedUpperBound.first = null; // String // test type parameters extending generic types - // parameterGeneric.first = ""; + // Primary[Prefix[Name[parameterGeneric.first]]] + parameterGeneric.second.second = new Integer(0); // test static imports // Primary[Prefix[Name[instanceFields.generic.first]]] @@ -110,3 +101,13 @@ public class GenericFieldAccess, S exten localGeneric = null; // Number } } + +class GenericSuperClassB { + public S fieldB; +} + +class GenericSuperClassA extends GenericSuperClassB> { + public T fieldA; + public GenericClass inheritedSuperGeneric; + public GenericClass inheritedUpperBound; +} From ba2041a9d08b6276e913dc1900a9268b5af5643a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Mon, 12 Jun 2017 21:54:11 +0200 Subject: [PATCH 08/20] Java, typeresolution: rename TypeWrapper to TypeDefinition --- .../java/ast/AbstractJavaAccessTypeNode.java | 22 ++++----- .../lang/java/ast/AbstractJavaTypeNode.java | 22 ++++----- .../pmd/lang/java/ast/TypeNode.java | 16 +++---- .../typeresolution/ClassTypeResolver.java | 46 +++++++++---------- ...peWrapper.java => JavaTypeDefinition.java} | 14 +++--- 5 files changed, 61 insertions(+), 59 deletions(-) rename pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/{TypeWrapper.java => JavaTypeDefinition.java} (64%) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java index ce14ecb957..6427ee3d43 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java @@ -4,10 +4,10 @@ package net.sourceforge.pmd.lang.java.ast; -import net.sourceforge.pmd.lang.java.typeresolution.TypeWrapper; +import net.sourceforge.pmd.lang.java.typeresolution.JavaTypeDefinition; public abstract class AbstractJavaAccessTypeNode extends AbstractJavaAccessNode implements TypeNode { - private TypeWrapper typeWrapper; + private JavaTypeDefinition typeDefinition; public AbstractJavaAccessTypeNode(int i) { super(i); @@ -19,8 +19,8 @@ public abstract class AbstractJavaAccessTypeNode extends AbstractJavaAccessNode @Override public Class getType() { - if (typeWrapper != null) { - return typeWrapper.getType(); + if (typeDefinition != null) { + return typeDefinition.getType(); } return null; @@ -28,20 +28,20 @@ public abstract class AbstractJavaAccessTypeNode extends AbstractJavaAccessNode @Override public void setType(Class type) { - if (typeWrapper == null) { - typeWrapper = new TypeWrapper(type); + if (typeDefinition == null) { + typeDefinition = new JavaTypeDefinition(type); } else { - typeWrapper.setClazz(type); + typeDefinition.setClazz(type); } } @Override - public TypeWrapper getTypeWrapper() { - return typeWrapper; + public JavaTypeDefinition getTypeDefinition() { + return typeDefinition; } @Override - public void setTypeWrapper(TypeWrapper typeWrapper) { - this.typeWrapper = typeWrapper; + public void setTypeDefinition(JavaTypeDefinition typeDefinition) { + this.typeDefinition = typeDefinition; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java index 6a173c3cd7..3c08ede85b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java @@ -4,7 +4,7 @@ package net.sourceforge.pmd.lang.java.ast; -import net.sourceforge.pmd.lang.java.typeresolution.TypeWrapper; +import net.sourceforge.pmd.lang.java.typeresolution.JavaTypeDefinition; /** * An extension of the SimpleJavaNode which implements the TypeNode interface. @@ -13,7 +13,7 @@ import net.sourceforge.pmd.lang.java.typeresolution.TypeWrapper; * @see TypeNode */ public abstract class AbstractJavaTypeNode extends AbstractJavaNode implements TypeNode { - private TypeWrapper typeWrapper; + private JavaTypeDefinition typeDefinition; public AbstractJavaTypeNode(int i) { super(i); @@ -25,8 +25,8 @@ public abstract class AbstractJavaTypeNode extends AbstractJavaNode implements T @Override public Class getType() { - if (typeWrapper != null) { - return typeWrapper.getType(); + if (typeDefinition != null) { + return typeDefinition.getType(); } return null; @@ -34,20 +34,20 @@ public abstract class AbstractJavaTypeNode extends AbstractJavaNode implements T @Override public void setType(Class type) { - if (typeWrapper == null) { - typeWrapper = new TypeWrapper(type); + if (typeDefinition == null) { + typeDefinition = new JavaTypeDefinition(type); } else { - typeWrapper.setClazz(type); + typeDefinition.setClazz(type); } } @Override - public TypeWrapper getTypeWrapper() { - return typeWrapper; + public JavaTypeDefinition getTypeDefinition() { + return typeDefinition; } @Override - public void setTypeWrapper(TypeWrapper typeWrapper) { - this.typeWrapper = typeWrapper; + public void setTypeDefinition(JavaTypeDefinition typeDefinition) { + this.typeDefinition = typeDefinition; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java index 9a480c76a1..c16db21b00 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java @@ -5,7 +5,7 @@ package net.sourceforge.pmd.lang.java.ast; import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.typeresolution.TypeWrapper; +import net.sourceforge.pmd.lang.java.typeresolution.JavaTypeDefinition; /** * This interface allows a Java Class to be associated with a node. @@ -20,20 +20,20 @@ public interface TypeNode extends Node { Class getType(); /** - * Get the TypeWrapper associated with this node. The Class object - * contained in the TypeWrapper will always be equal to that which + * Get the TypeDefinition associated with this node. The Class object + * contained in the TypeDefinition will always be equal to that which * is returned by getType(). * - * @return The TypeWrapper, may return null + * @return The TypeDefinition, may return null */ - TypeWrapper getTypeWrapper(); + JavaTypeDefinition getTypeDefinition(); /** - * Set the TypeWrapper associated with this node. + * Set the TypeDefinition associated with this node. * - * @param type A TypeWrapper object + * @param type A TypeDefinition object */ - void setTypeWrapper(TypeWrapper type); + void setTypeDefinition(JavaTypeDefinition type); /** * Set the Java Class associated with this node. diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 978c5be07a..cdcb537ff3 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -234,12 +234,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (typeArguments != null) { for (int index = 0; index < typeArguments.jjtGetNumChildren(); ++index) { - node.getTypeWrapper().getGenericArgs().add( - ((TypeNode) typeArguments.jjtGetChild(index)).getTypeWrapper() + node.getTypeDefinition().getGenericArgs().add( + ((TypeNode) typeArguments.jjtGetChild(index)).getTypeDefinition() ); } - } else if(isGeneric(node.getType()) && node.getTypeWrapper().getGenericArgs().size() == 0) { - node.setTypeWrapper(getDefaultUpperBounds(null, node.getType())); + } else if(isGeneric(node.getType()) && node.getTypeDefinition().getGenericArgs().size() == 0) { + node.setTypeDefinition(getDefaultUpperBounds(null, node.getType())); } return data; @@ -298,7 +298,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } if (node.getType() == null) { - TypeWrapper previousNameType = + JavaTypeDefinition previousNameType = getClassWrapperOfVariableFromScope(node.getScope(), dotSplitImage[0]); for (int i = 1; i < dotSplitImage.length; ++i) { @@ -311,7 +311,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } if (previousNameType != null) { - node.setTypeWrapper(previousNameType); + node.setTypeDefinition(previousNameType); } } } @@ -319,7 +319,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return super.visit(node, data); } - private TypeWrapper getClassWrapperOfVariableFromScope(Scope scope, String image) { + private JavaTypeDefinition getClassWrapperOfVariableFromScope(Scope scope, String image) { for (/* empty */; scope != null; scope = scope.getParent()) { for (Map.Entry> entry : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { @@ -328,9 +328,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (typeNode.jjtGetChild(0) instanceof ASTReferenceType) { return ((ASTClassOrInterfaceType) typeNode.jjtGetChild(0).jjtGetChild(0)) - .getTypeWrapper(); + .getTypeDefinition(); } else { // primitive type - return new TypeWrapper(typeNode.getType()); + return new JavaTypeDefinition(typeNode.getType()); } } } @@ -344,9 +344,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { image); if (inheritedField != null) { - TypeWrapper superClass = + JavaTypeDefinition superClass = ((ASTClassOrInterfaceType) classScope.getClassDeclaration().getNode() - .getFirstChildOfType(ASTExtendsList.class).jjtGetChild(0)).getTypeWrapper(); + .getFirstChildOfType(ASTExtendsList.class).jjtGetChild(0)).getTypeDefinition(); return getClassOfInheritedField(superClass, image); } @@ -361,7 +361,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return null; } - private TypeWrapper getClassOfInheritedField(TypeWrapper inheritedClass, String fieldImage) { + private JavaTypeDefinition getClassOfInheritedField(JavaTypeDefinition inheritedClass, String fieldImage) { while (true) { try { Field field = inheritedClass.getType().getDeclaredField(fieldImage); @@ -378,14 +378,14 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } } - private TypeWrapper getNextClassWrapper(TypeWrapper previousWrapper, Type genericType) { + private JavaTypeDefinition getNextClassWrapper(JavaTypeDefinition previousWrapper, Type genericType) { if (genericType instanceof Class) { return getDefaultUpperBounds(previousWrapper, (Class) genericType); } else if (genericType instanceof ParameterizedType) { ParameterizedType parameterizedType = (ParameterizedType) genericType; - TypeWrapper wrapper = new TypeWrapper((Class) parameterizedType.getRawType()); - wrapper.setGenericArgs(new ArrayList()); + JavaTypeDefinition wrapper = new JavaTypeDefinition((Class) parameterizedType.getRawType()); + wrapper.setGenericArgs(new ArrayList()); for (Type type : parameterizedType.getActualTypeArguments()) { wrapper.getGenericArgs().add(getNextClassWrapper(previousWrapper, type)); @@ -402,7 +402,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (wildcardUpperBounds.length != 0) { // upper bound wildcard return getNextClassWrapper(previousWrapper, wildcardUpperBounds[0]); } else { // lower bound wildcard - return new TypeWrapper(Object.class); + return new JavaTypeDefinition(Object.class); } } @@ -410,14 +410,14 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } // this exists to avoid infinite recursion in some cases - private Map defaultUpperBounds = new HashMap<>(); + private Map defaultUpperBounds = new HashMap<>(); - private TypeWrapper getDefaultUpperBounds(TypeWrapper original, Class clazzToFill) { + private JavaTypeDefinition getDefaultUpperBounds(JavaTypeDefinition original, Class clazzToFill) { if (defaultUpperBounds.containsKey(clazzToFill)) { return defaultUpperBounds.get(clazzToFill); } - TypeWrapper wrapper = new TypeWrapper(clazzToFill); + JavaTypeDefinition wrapper = new JavaTypeDefinition(clazzToFill); defaultUpperBounds.put(clazzToFill, wrapper); @@ -426,7 +426,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { original = wrapper; } - wrapper.setGenericArgs(new ArrayList()); + wrapper.setGenericArgs(new ArrayList()); for (TypeVariable parameter : clazzToFill.getTypeParameters()) { Type upperBound = parameter.getBounds()[0]; @@ -650,7 +650,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { Class primaryNodeType = null; AbstractJavaTypeNode previousChild = null; - TypeWrapper previousClassWraper = null; + JavaTypeDefinition previousClassWraper = null; for (int childIndex = 0; childIndex < primaryNode.jjtGetNumChildren(); ++childIndex) { AbstractJavaTypeNode currentChild = (AbstractJavaTypeNode) primaryNode.jjtGetChild(childIndex); @@ -888,7 +888,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (node.jjtGetNumChildren() >= 1) { Node child = node.jjtGetChild(0); if (child instanceof TypeNode) { - typeNode.setTypeWrapper(((TypeNode) child).getTypeWrapper()); + typeNode.setTypeDefinition(((TypeNode) child).getTypeDefinition()); } } } @@ -1006,7 +1006,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (myType == null) { ASTTypeParameter parameter = getTypeParameterDeclaration(node, className); if (parameter != null) { - node.setTypeWrapper(parameter.getTypeWrapper()); + node.setTypeDefinition(parameter.getTypeDefinition()); } } else { node.setType(myType); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java similarity index 64% rename from pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java rename to pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java index e0509727ef..e64d3d2e72 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeWrapper.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java @@ -3,11 +3,13 @@ package net.sourceforge.pmd.lang.java.typeresolution; import java.util.ArrayList; import java.util.List; -public class TypeWrapper { - private Class clazz; - private List genericArgs = null; - public TypeWrapper(Class clazz) { + +public class JavaTypeDefinition { + private Class clazz; + private List genericArgs = null; + + public JavaTypeDefinition(Class clazz) { this.clazz = clazz; } @@ -19,7 +21,7 @@ public class TypeWrapper { this.clazz = clazz; } - public List getGenericArgs() { + public List getGenericArgs() { if(genericArgs == null) { genericArgs = new ArrayList<>(); } @@ -27,7 +29,7 @@ public class TypeWrapper { return genericArgs; } - public void setGenericArgs(List genericArgs) { + public void setGenericArgs(List genericArgs) { this.genericArgs = genericArgs; } } From b6439685deb65c77e21f24098c8c6cc2ba8e87b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Mon, 12 Jun 2017 22:08:11 +0200 Subject: [PATCH 09/20] Java, typeres: add TypeDefinition interface --- .../typeresolution/JavaTypeDefinition.java | 3 ++- .../java/typeresolution/TypeDefinition.java | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeDefinition.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java index e64d3d2e72..7b35cf8c01 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java @@ -1,11 +1,12 @@ package net.sourceforge.pmd.lang.java.typeresolution; import java.util.ArrayList; +import java.util.Collections; import java.util.List; -public class JavaTypeDefinition { +public class JavaTypeDefinition implements TypeDefinition { private Class clazz; private List genericArgs = null; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeDefinition.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeDefinition.java new file mode 100644 index 0000000000..f11440d80e --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeDefinition.java @@ -0,0 +1,20 @@ +package net.sourceforge.pmd.lang.java.typeresolution; + +import java.util.ArrayList; +import java.util.List; + +public interface TypeDefinition { + /** + * Get the raw Class type of the definition. + * + * @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 getGenericArgs(); +} From af284f28b59e2d9645472936beef443291158476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Tue, 13 Jun 2017 02:39:30 +0200 Subject: [PATCH 10/20] Java, typeres: make JavaTypeDefinition immutable --- .../java/ast/AbstractJavaAccessTypeNode.java | 8 +-- .../lang/java/ast/AbstractJavaTypeNode.java | 8 +-- .../pmd/lang/java/ast/TypeNode.java | 2 +- .../typeresolution/ClassTypeResolver.java | 65 ++++++++++--------- .../typeresolution/JavaTypeDefinition.java | 36 ---------- .../typedefinition/JavaTypeDefinition.java | 45 +++++++++++++ .../JavaTypeDefinitionBuilder.java | 35 ++++++++++ .../{ => typedefinition}/TypeDefinition.java | 3 +- 8 files changed, 120 insertions(+), 82 deletions(-) delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java rename pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/{ => typedefinition}/TypeDefinition.java (82%) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java index 6427ee3d43..19b5d9f7f5 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaAccessTypeNode.java @@ -4,7 +4,7 @@ package net.sourceforge.pmd.lang.java.ast; -import net.sourceforge.pmd.lang.java.typeresolution.JavaTypeDefinition; +import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; public abstract class AbstractJavaAccessTypeNode extends AbstractJavaAccessNode implements TypeNode { private JavaTypeDefinition typeDefinition; @@ -28,11 +28,7 @@ public abstract class AbstractJavaAccessTypeNode extends AbstractJavaAccessNode @Override public void setType(Class type) { - if (typeDefinition == null) { - typeDefinition = new JavaTypeDefinition(type); - } else { - typeDefinition.setClazz(type); - } + typeDefinition = JavaTypeDefinition.build(type); } @Override diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java index 3c08ede85b..6f0a9dee64 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractJavaTypeNode.java @@ -4,7 +4,7 @@ package net.sourceforge.pmd.lang.java.ast; -import net.sourceforge.pmd.lang.java.typeresolution.JavaTypeDefinition; +import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; /** * An extension of the SimpleJavaNode which implements the TypeNode interface. @@ -34,11 +34,7 @@ public abstract class AbstractJavaTypeNode extends AbstractJavaNode implements T @Override public void setType(Class type) { - if (typeDefinition == null) { - typeDefinition = new JavaTypeDefinition(type); - } else { - typeDefinition.setClazz(type); - } + typeDefinition = JavaTypeDefinition.build(type); } @Override diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java index c16db21b00..91a2518be5 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java @@ -5,7 +5,7 @@ package net.sourceforge.pmd.lang.java.ast; import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.typeresolution.JavaTypeDefinition; +import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; /** * This interface allows a Java Class to be associated with a node. diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index cdcb537ff3..b814eaae6a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -80,6 +80,9 @@ import net.sourceforge.pmd.lang.java.ast.Token; 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.java.typeresolution.typedefinition.TypeDefinition; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; import net.sourceforge.pmd.lang.symboltable.Scope; @@ -228,16 +231,19 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } typeName = parent.getImage() + "$" + anonymousClassCounter; } + populateType(node, typeName); ASTTypeArguments typeArguments = node.getFirstChildOfType(ASTTypeArguments.class); if (typeArguments != null) { + JavaTypeDefinitionBuilder builder = JavaTypeDefinition.builder(node.getType()); + for (int index = 0; index < typeArguments.jjtGetNumChildren(); ++index) { - node.getTypeDefinition().getGenericArgs().add( - ((TypeNode) typeArguments.jjtGetChild(index)).getTypeDefinition() - ); + builder.addTypeArg(((TypeNode) typeArguments.jjtGetChild(index)).getTypeDefinition()); } + + node.setTypeDefinition(builder.build()); } else if(isGeneric(node.getType()) && node.getTypeDefinition().getGenericArgs().size() == 0) { node.setTypeDefinition(getDefaultUpperBounds(null, node.getType())); } @@ -330,7 +336,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return ((ASTClassOrInterfaceType) typeNode.jjtGetChild(0).jjtGetChild(0)) .getTypeDefinition(); } else { // primitive type - return new JavaTypeDefinition(typeNode.getType()); + return JavaTypeDefinition.build(typeNode.getType()); } } } @@ -378,66 +384,63 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } } - private JavaTypeDefinition getNextClassWrapper(JavaTypeDefinition previousWrapper, Type genericType) { + private JavaTypeDefinition getNextClassWrapper(JavaTypeDefinition context, Type genericType) { if (genericType instanceof Class) { - return getDefaultUpperBounds(previousWrapper, (Class) genericType); + return getDefaultUpperBounds(context, (Class) genericType); } else if (genericType instanceof ParameterizedType) { ParameterizedType parameterizedType = (ParameterizedType) genericType; - JavaTypeDefinition wrapper = new JavaTypeDefinition((Class) parameterizedType.getRawType()); - wrapper.setGenericArgs(new ArrayList()); + JavaTypeDefinitionBuilder typeDef = JavaTypeDefinition.builder((Class) parameterizedType.getRawType()); for (Type type : parameterizedType.getActualTypeArguments()) { - wrapper.getGenericArgs().add(getNextClassWrapper(previousWrapper, type)); + typeDef.addTypeArg(getNextClassWrapper(context, type)); } - return wrapper; + return typeDef.build(); } else if (genericType instanceof TypeVariable) { - int ordinal = getTypeParameterOrdinal(previousWrapper.getType(), ((TypeVariable) genericType).getName()); + int ordinal = getTypeParameterOrdinal(context.getType(), ((TypeVariable) genericType).getName()); if (ordinal != -1) { - return previousWrapper.getGenericArgs().get(ordinal); + return context.getGenericArgs().get(ordinal); } } else if (genericType instanceof WildcardType) { Type[] wildcardUpperBounds = ((WildcardType) genericType).getUpperBounds(); if (wildcardUpperBounds.length != 0) { // upper bound wildcard - return getNextClassWrapper(previousWrapper, wildcardUpperBounds[0]); + return getNextClassWrapper(context, wildcardUpperBounds[0]); } else { // lower bound wildcard - return new JavaTypeDefinition(Object.class); + return JavaTypeDefinition.build(Object.class); } } return null; } - // this exists to avoid infinite recursion in some cases - private Map defaultUpperBounds = new HashMap<>(); - private JavaTypeDefinition getDefaultUpperBounds(JavaTypeDefinition original, Class clazzToFill) { - if (defaultUpperBounds.containsKey(clazzToFill)) { - return defaultUpperBounds.get(clazzToFill); + private Map classToDefaultUpperBounds = new HashMap<>(); + + private JavaTypeDefinition getDefaultUpperBounds(JavaTypeDefinition context, Class clazzWithDefBounds) { + JavaTypeDefinitionBuilder typeDef = JavaTypeDefinition.builder(clazzWithDefBounds); + + if(classToDefaultUpperBounds.containsKey(clazzWithDefBounds)) { + return classToDefaultUpperBounds.get(clazzWithDefBounds); + } else { + classToDefaultUpperBounds.put(clazzWithDefBounds, typeDef.build()); } - JavaTypeDefinition wrapper = new JavaTypeDefinition(clazzToFill); - - defaultUpperBounds.put(clazzToFill, wrapper); - - if (isGeneric(clazzToFill)) { - if (original == null) { - original = wrapper; + if (isGeneric(clazzWithDefBounds)) { + if(context == null) { + context = typeDef.build(); } - wrapper.setGenericArgs(new ArrayList()); - - for (TypeVariable parameter : clazzToFill.getTypeParameters()) { + for (TypeVariable parameter : clazzWithDefBounds.getTypeParameters()) { Type upperBound = parameter.getBounds()[0]; // TODO: fix self reference "< ... E extends Something ... >" - wrapper.getGenericArgs().add(getNextClassWrapper(original, upperBound)); + typeDef.addTypeArg(getNextClassWrapper(context, upperBound)); } } - return wrapper; + return typeDef.build(); } private int getTypeParameterOrdinal(Class clazz, String parameterName) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java deleted file mode 100644 index 7b35cf8c01..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/JavaTypeDefinition.java +++ /dev/null @@ -1,36 +0,0 @@ -package net.sourceforge.pmd.lang.java.typeresolution; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - - - -public class JavaTypeDefinition implements TypeDefinition { - private Class clazz; - private List genericArgs = null; - - public JavaTypeDefinition(Class clazz) { - this.clazz = clazz; - } - - public Class getType() { - return clazz; - } - - public void setClazz(Class clazz) { - this.clazz = clazz; - } - - public List getGenericArgs() { - if(genericArgs == null) { - genericArgs = new ArrayList<>(); - } - - return genericArgs; - } - - public void setGenericArgs(List genericArgs) { - this.genericArgs = genericArgs; - } -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java new file mode 100644 index 0000000000..28eaf628b7 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java @@ -0,0 +1,45 @@ +package net.sourceforge.pmd.lang.java.typeresolution.typedefinition; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + + +public class JavaTypeDefinition implements TypeDefinition { + private final Class clazz; + private final List genericArgs; + + public Class getType() { + return clazz; + } + + public List getGenericArgs() { + return genericArgs; + } + + JavaTypeDefinition(Class clazz, List genericArgs) { + this.clazz = clazz; + + if (genericArgs == null) { + genericArgs = new ArrayList<>(); + } + + this.genericArgs = Collections.unmodifiableList(genericArgs); + } + + + // builder part of the class + + // TODO: possibly add some sort of caching (with like a map or something) + public static JavaTypeDefinition build(Class clazz) { + return new JavaTypeDefinition(clazz, null); + } + + public static JavaTypeDefinitionBuilder builder(Class clazz) { + return new JavaTypeDefinitionBuilder().setType(clazz); + } + + public static JavaTypeDefinitionBuilder builder() { + return new JavaTypeDefinitionBuilder(); + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java new file mode 100644 index 0000000000..f3edd3e9f4 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java @@ -0,0 +1,35 @@ +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 genericArgs = new ArrayList<>(); + + JavaTypeDefinitionBuilder() {} + + public JavaTypeDefinitionBuilder addTypeArg(JavaTypeDefinition arg) { + genericArgs.add(arg); + return this; + } + + public List 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 new JavaTypeDefinition(clazz, genericArgs); + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeDefinition.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java similarity index 82% rename from pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeDefinition.java rename to pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java index f11440d80e..ecde6e5c06 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeDefinition.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java @@ -1,6 +1,5 @@ -package net.sourceforge.pmd.lang.java.typeresolution; +package net.sourceforge.pmd.lang.java.typeresolution.typedefinition; -import java.util.ArrayList; import java.util.List; public interface TypeDefinition { From 18e243447b87f203f0c0652dbee42748f10bb1d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Tue, 13 Jun 2017 17:11:31 +0200 Subject: [PATCH 11/20] Java, typeres: clean up generic field tests --- .../typeresolution/ClassTypeResolverTest.java | 179 ++++++++++++------ .../testdata/FieldAccessGenericBounds.java | 28 +++ .../testdata/FieldAccessGenericParameter.java | 31 +++ .../testdata/FieldAccessGenericRaw.java | 33 ++++ .../testdata/FieldAccessGenericSimple.java | 48 +++++ .../testdata/GenericFieldAccess.java | 113 ----------- .../dummytypes/GenericSuperClassA.java | 8 + .../dummytypes/GenericSuperClassB.java | 5 + 8 files changed, 274 insertions(+), 171 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericBounds.java create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericParameter.java create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericRaw.java create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java delete mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassA.java create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassB.java diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index 662e68d881..c071056698 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -14,8 +14,10 @@ import java.util.ArrayList; import java.util.List; import java.util.StringTokenizer; -import net.sourceforge.pmd.typeresolution.testdata.GenericFieldAccess; -import net.sourceforge.pmd.typeresolution.testdata.dummytypes.StaticFields; +import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericBounds; +import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericParameter; +import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericRaw; +import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericSimple; import org.apache.commons.io.IOUtils; import org.jaxen.JaxenException; import org.junit.Assert; @@ -759,8 +761,8 @@ public class ClassTypeResolverTest { } @Test - public void testGenericFieldAccess() throws JaxenException { - ASTCompilationUnit acu = parseAndTypeResolveForClass15(GenericFieldAccess.class); + public void testBoundsGenericFieldAccess() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(FieldAccessGenericBounds.class); List expressions = convertList( acu.findChildNodesWithXPath("//StatementExpression/PrimaryExpression"), @@ -794,55 +796,51 @@ public class ClassTypeResolverTest { assertEquals(String.class, getChildType(expressions.get(index++), 0)); + // Make sure we got them all + assertEquals("All expressions not tested", index, expressions.size()); + } + + @Test + public void testParameterGenericFieldAccess() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(FieldAccessGenericParameter.class); + + List expressions = convertList( + acu.findChildNodesWithXPath("//StatementExpression/PrimaryExpression"), + AbstractJavaTypeNode.class); + + + int index = 0; + + // classGeneric = null; // Double + assertEquals(Double.class, expressions.get(index).getType()); + assertEquals(Double.class, getChildType(expressions.get(index++), 0)); + + // localGeneric = null; // Character + assertEquals(Character.class, expressions.get(index).getType()); + assertEquals(Character.class, getChildType(expressions.get(index++), 0)); // parameterGeneric.second.second = new Integer(0); assertEquals(Integer.class, expressions.get(index).getType()); assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + // localGeneric = null; // Number + assertEquals(Number.class, expressions.get(index).getType()); + assertEquals(Number.class, getChildType(expressions.get(index++), 0)); - // instanceFields.generic.first = ""; - //assertEquals(String.class, expressions.get(index).getType()); - //assertEquals(String.class, getChildType(expressions.get(index++), 0)); + // Make sure we got them all + assertEquals("All expressions not tested", index, expressions.size()); + } - // staticGeneric.first = new Long(0); - //assertEquals(Long.class, expressions.get(index).getType()); - //assertEquals(Long.class, getChildType(expressions.get(index++), 0)); + @Test + public void testSimpleGenericFieldAccess() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(FieldAccessGenericSimple.class); + + List expressions = convertList( + acu.findChildNodesWithXPath("//StatementExpression/PrimaryExpression"), + AbstractJavaTypeNode.class); - // fieldA = new Long(0); - assertEquals(Long.class, expressions.get(index).getType()); - assertEquals(Long.class, getChildType(expressions.get(index++), 0)); - - // fieldB.generic.second = ""; - assertEquals(String.class, expressions.get(index).getType()); - assertEquals(String.class, getChildType(expressions.get(index++), 0)); - - - // rawGeneric.first = new Integer(0); - assertEquals(Integer.class, expressions.get(index).getType()); - assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); - - // rawGeneric.second = new Integer(0); - assertEquals(Integer.class, expressions.get(index).getType()); - assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); - - // rawGeneric.third = new Object(); - assertEquals(Object.class, expressions.get(index).getType()); - assertEquals(Object.class, getChildType(expressions.get(index++), 0)); - - // rawGeneric.fourth.second = ""; - assertEquals(String.class, expressions.get(index).getType()); - assertEquals(String.class, getChildType(expressions.get(index++), 0)); - - // rawGeneric.rawGeneric.second = new Integer(0); - assertEquals(Integer.class, expressions.get(index).getType()); - assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); - - - - //genericTypeArg.second.second = new Double(0); - assertEquals(Double.class, expressions.get(index).getType()); - assertEquals(Double.class, getChildType(expressions.get(index++), 0)); + int index = 0; // genericField.first = ""; assertEquals(String.class, expressions.get(index).getType()); @@ -852,6 +850,19 @@ public class ClassTypeResolverTest { assertEquals(Double.class, expressions.get(index).getType()); assertEquals(Double.class, getChildType(expressions.get(index++), 0)); + //genericTypeArg.second.second = new Double(0); + assertEquals(Double.class, expressions.get(index).getType()); + assertEquals(Double.class, getChildType(expressions.get(index++), 0)); + + // param.first = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + + // local.second = new Long(0); + assertEquals(Long.class, expressions.get(index).getType()); + assertEquals(Long.class, getChildType(expressions.get(index++), 0)); + + // param.generic.first = new Character('c'); assertEquals(Character.class, expressions.get(index).getType()); assertEquals(Character.class, getChildType(expressions.get(index++), 0)); @@ -864,25 +875,77 @@ public class ClassTypeResolverTest { assertEquals(Double.class, expressions.get(index).getType()); assertEquals(Double.class, getChildType(expressions.get(index++), 0)); - // param.first = new Integer(0); - assertEquals(Integer.class, expressions.get(index).getType()); - assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); - - // local.second = new Long(0); + // fieldA = new Long(0); assertEquals(Long.class, expressions.get(index).getType()); assertEquals(Long.class, getChildType(expressions.get(index++), 0)); - // classGeneric = null; // Double - assertEquals(Double.class, expressions.get(index).getType()); - assertEquals(Double.class, getChildType(expressions.get(index++), 0)); + // fieldB.generic.second = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 0)); - // localGeneric = null; // Character - assertEquals(Character.class, expressions.get(index).getType()); - assertEquals(Character.class, getChildType(expressions.get(index++), 0)); + // Make sure we got them all + assertEquals("All expressions not tested", index, expressions.size()); + } + + @Test + public void testRawGenericFieldAccess() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(FieldAccessGenericRaw.class); + + List expressions = convertList( + acu.findChildNodesWithXPath("//StatementExpression/PrimaryExpression"), + AbstractJavaTypeNode.class); + + + int index = 0; + + // rawGeneric.first = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + + // rawGeneric.second = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + + // rawGeneric.third = new Object(); + assertEquals(Object.class, expressions.get(index).getType()); + assertEquals(Object.class, getChildType(expressions.get(index++), 0)); + // rawGeneric.fourth.second = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 0)); + // rawGeneric.rawGeneric.second = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + // inheritedRawGeneric.first = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + // inheritedRawGeneric.second = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + // inheritedRawGeneric.third = new Object(); + assertEquals(Object.class, expressions.get(index).getType()); + assertEquals(Object.class, getChildType(expressions.get(index++), 0)); + // inheritedRawGeneric.fourth.second = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 0)); + // inheritedRawGeneric.rawGeneric.second = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + // parameterRawGeneric.first = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + // parameterRawGeneric.second = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); + // parameterRawGeneric.third = new Object(); + assertEquals(Object.class, expressions.get(index).getType()); + assertEquals(Object.class, getChildType(expressions.get(index++), 0)); + // parameterRawGeneric.fourth.second = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 0)); + // parameterRawGeneric.rawGeneric.second = new Integer(0); + assertEquals(Integer.class, expressions.get(index).getType()); + assertEquals(Integer.class, getChildType(expressions.get(index++), 0)); - // localGeneric = null; // Number - assertEquals(Number.class, expressions.get(index).getType()); - assertEquals(Number.class, getChildType(expressions.get(index++), 0)); // Make sure we got them all assertEquals("All expressions not tested", index, expressions.size()); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericBounds.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericBounds.java new file mode 100644 index 0000000000..e16da1f6fb --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericBounds.java @@ -0,0 +1,28 @@ +package net.sourceforge.pmd.typeresolution.testdata; + +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericSuperClassA; + + +public class FieldAccessGenericBounds extends GenericSuperClassA { + GenericClass superGeneric; + GenericClass upperBound; + + public void astPrimaryNameCases() { + // test ?, ? super Something, ? extends Something + // Primary[Prefix[Name[superGeneric.first]]] + superGeneric.first = ""; // Object + superGeneric.second = null; // Object + inheritedSuperGeneric.first = ""; // Object + inheritedSuperGeneric.second = null; // Object + + upperBound.first = null; // Number + inheritedUpperBound.first = null; // String + + // test static imports + // Primary[Prefix[Name[instanceFields.generic.first]]] + //instanceFields.generic.first = ""; + //staticGeneric.first = new Long(0); + } +} + diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericParameter.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericParameter.java new file mode 100644 index 0000000000..b2868024f9 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericParameter.java @@ -0,0 +1,31 @@ +package net.sourceforge.pmd.typeresolution.testdata; + +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; + +public class FieldAccessGenericParameter>, + S extends Double> { + T parameterGeneric; + S classGeneric; + + void foo() { + M localGeneric = null; + + // access type dependant on class/method type arguments + // Primary[Prefix[Name[classGeneric]]] + classGeneric = null; // Double + localGeneric = null; // Character + + + // test type parameters extending generic types + // Primary[Prefix[Name[parameterGeneric.first]]] + parameterGeneric.second.second = new Integer(0); + } + + FieldAccessGenericParameter() { + C constructorGeneric = null; + + // access type dependant on constructor type arugments + // Primary[Prefix[Name[localGeneric]]] + constructorGeneric = null; // Number + } +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericRaw.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericRaw.java new file mode 100644 index 0000000000..1353acdaab --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericRaw.java @@ -0,0 +1,33 @@ +package net.sourceforge.pmd.typeresolution.testdata; + +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass2; +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericSuperClassA; + +public class FieldAccessGenericRaw extends GenericSuperClassA { + GenericClass2 rawGeneric; + T parameterRawGeneric; + + void foo() { + // test raw types + // Primary[Prefix[Name[rawGeneric.first]]] + rawGeneric.first = new Integer(0); + rawGeneric.second = new Integer(0); + rawGeneric.third = new Object(); + rawGeneric.fourth.second = ""; + rawGeneric.rawGeneric.second = new Integer(0); + + // Primary[Prefix[Name[inheritedGeneric.first]]] + inheritedRawGeneric.first = new Integer(0); + inheritedRawGeneric.second = new Integer(0); + inheritedRawGeneric.third = new Object(); + inheritedRawGeneric.fourth.second = ""; + inheritedRawGeneric.rawGeneric.second = new Integer(0); + + // Primary[Prefix[Name[parameterRawGeneric.first]]] + parameterRawGeneric.first = new Integer(0); + parameterRawGeneric.second = new Integer(0); + parameterRawGeneric.third = new Object(); + parameterRawGeneric.fourth.second = ""; + parameterRawGeneric.rawGeneric.second = new Integer(0); + } +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java new file mode 100644 index 0000000000..79fb463818 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java @@ -0,0 +1,48 @@ +package net.sourceforge.pmd.typeresolution.testdata; + +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericSuperClassA; + + +/* + * TODO: test [nested classes, shadowing] + * TODO: add not name cases + * TODO: add anonymous class this (Allocation expression) + * + * TODO: add primitives, parameterized arrays + * TODO: diamond, type parmeter declarations can shadow Class declarations + */ + +public class FieldAccessGenericSimple extends GenericSuperClassA { + GenericClass genericField; + GenericClass> genericTypeArg; + + void foo(GenericClass param) { + GenericClass local = null; + + // access a generic field through member field + // Primary[Prefix[Name[genericField.first]]] + genericField.first = ""; + genericField.second = new Double(0); + + // access a generic field whose type depends on a generic type argument + // Primary[Prefix[Name[genericTypeArg.second.second]]] + genericTypeArg.second.second = new Double(0); + + // access a generic field through a local or a parameter + // Primary[Prefix[Name[param.first]]] + param.first = new Integer(0); + local.second = new Long(0); + + // access a generic field whose type depends on indirect type arguments + // Primary[Prefix[Name[generic.generic.first]]] + param.generic.first = new Character('c'); + local.generic.second = new Float(0); + genericField.generic.generic.generic.first = new Double(0); + + // test inherited generic + // Primary[Prefix[Name[generic.first]]] + fieldA = new Long(0); + fieldB.generic.second = ""; + } +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java deleted file mode 100644 index d8b0079c98..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericFieldAccess.java +++ /dev/null @@ -1,113 +0,0 @@ -package net.sourceforge.pmd.typeresolution.testdata; - -import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; -import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass2; -import static net.sourceforge.pmd.typeresolution.testdata.dummytypes.StaticFields.instanceFields; -import static net.sourceforge.pmd.typeresolution.testdata.dummytypes.StaticFields.staticGeneric; - - -/* - * TODO: test [nested classes, shadowing] - * TODO: add not Name[...] cases - * TODO: add primitives, parameterized arrays - * TODO: diamond, type parmeter declarations can shadow Class declarations - * - * - * Add fields dependent on type arguments from class, method, have super, extends, wild card and what not - * - */ - -public class GenericFieldAccess>, S extends Double> extends GenericSuperClassA { - public T parameterGeneric; - public GenericClass genericField; - public GenericClass> genericTypeArg; - public GenericClass2 rawGeneric; - public GenericFieldAccess field; - public S classGeneric; - public GenericClass superGeneric; - public GenericClass upperBound; - - public void astNameCases(GenericClass param) { - GenericClass local = null; - M localGeneric = null; - - - // test ?, ? super Something, ? extends Something - // Primary[Prefix[Name[superGeneric.first]]] - superGeneric.first = ""; // Object - superGeneric.second = null; // Object - inheritedSuperGeneric.first = ""; // Object - inheritedSuperGeneric.second = null; // Object - - upperBound.first = null; // Number - inheritedUpperBound.first = null; // String - - // test type parameters extending generic types - // Primary[Prefix[Name[parameterGeneric.first]]] - parameterGeneric.second.second = new Integer(0); - - // test static imports - // Primary[Prefix[Name[instanceFields.generic.first]]] - //instanceFields.generic.first = ""; - //staticGeneric.first = new Long(0); - - - // test inherited generic - // Primary[Prefix[Name[generic.first]]] - fieldA = new Long(0); - fieldB.generic.second = ""; - - // test raw types - // Primary[Prefix[Name[rawGeneric.first]]] - rawGeneric.first = new Integer(0); - rawGeneric.second = new Integer(0); - rawGeneric.third = new Object(); - rawGeneric.fourth.second = ""; - rawGeneric.rawGeneric.second = new Integer(0); - - - // access a generic field whose type depends on a generic type argument - // Primary[Prefix[Name[genericTypeArg.second.second]]] - genericTypeArg.second.second = new Double(0); - - // access a generic field through member field - // Primary[Prefix[Name[genericField.first]]] - genericField.first = ""; - genericField.second = new Double(0); - - // access a generic field whose type depends on indirect type arguments - // Primary[Prefix[Name[generic.generic.first]]] - param.generic.first = new Character('c'); - local.generic.second = new Float(0); - genericField.generic.generic.generic.first = new Double(0); - - - // access a generic field through a local or a parameter - // Primary[Prefix[Name[param.first]]] - param.first = new Integer(0); - local.second = new Long(0); - - // access type dependant on class/method type arguments - // Primary[Prefix[Name[classGeneric]]] - classGeneric = null; // Double - localGeneric = null; // Character - } - - public GenericFieldAccess() { - C localGeneric = null; - - // access type dependant on constructor type arguments - // Primary[Prefix[Name[localGeneric]]] - localGeneric = null; // Number - } -} - -class GenericSuperClassB { - public S fieldB; -} - -class GenericSuperClassA extends GenericSuperClassB> { - public T fieldA; - public GenericClass inheritedSuperGeneric; - public GenericClass inheritedUpperBound; -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassA.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassA.java new file mode 100644 index 0000000000..d6e712ebe8 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassA.java @@ -0,0 +1,8 @@ +package net.sourceforge.pmd.typeresolution.testdata.dummytypes; + +public class GenericSuperClassA extends GenericSuperClassB> { + public T fieldA; + public GenericClass2 inheritedRawGeneric; + public GenericClass inheritedSuperGeneric; + public GenericClass inheritedUpperBound; +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassB.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassB.java new file mode 100644 index 0000000000..e228829379 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassB.java @@ -0,0 +1,5 @@ +package net.sourceforge.pmd.typeresolution.testdata.dummytypes; + +public class GenericSuperClassB { + public S fieldB; +} From 3544c8f5ec70b3087a272faf1dc75e8aff52d05d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Wed, 14 Jun 2017 00:49:10 +0200 Subject: [PATCH 12/20] Java, typeres: resolve generic field type in not Name[..] cases --- .../typeresolution/ClassTypeResolver.java | 151 +++++++++++++----- .../typeresolution/ClassTypeResolverTest.java | 86 +++++++++- .../testdata/FieldAccessGenericSimple.java | 14 +- .../FieldAccessPrimaryGenericSimple.java | 44 +++++ 4 files changed, 242 insertions(+), 53 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessPrimaryGenericSimple.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index b814eaae6a..db03d0ddc9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -82,7 +82,6 @@ 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.java.typeresolution.typedefinition.TypeDefinition; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; import net.sourceforge.pmd.lang.symboltable.Scope; @@ -244,13 +243,21 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } node.setTypeDefinition(builder.build()); - } else if(isGeneric(node.getType()) && node.getTypeDefinition().getGenericArgs().size() == 0) { + } else if (isGeneric(node.getType()) && node.getTypeDefinition().getGenericArgs().size() == 0) { node.setTypeDefinition(getDefaultUpperBounds(null, node.getType())); } return data; } + @Override + public Object visit(ASTExtendsList node, Object data) { + super.visit(node, data); + + + return data; + } + @Override public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { populateType(node, node.getImage()); @@ -276,7 +283,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { && node.getNameDeclaration().getNode() instanceof TypeNode) { // Carry over the type from the declaration Class nodeType = ((TypeNode) node.getNameDeclaration().getNode()).getType(); - if (!isGeneric(nodeType)) { + if (!isGeneric(nodeType) && !isGeneric(nodeType.getSuperclass())) { node.setType(nodeType); } } @@ -312,8 +319,16 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { break; } - Field field = searchClassForField(previousNameType.getType(), dotSplitImage[i]); - previousNameType = getNextClassWrapper(previousNameType, field.getGenericType()); + + if (isFieldInherited(previousNameType.getType(), dotSplitImage[i])) { + previousNameType = getInheritedFieldTypeDefinition(getNextSuperClassTypeDefinition(previousNameType), + dotSplitImage[i]); + } else { + Field field = searchClassForField(previousNameType.getType(), dotSplitImage[i]); + if(field != null) { + previousNameType = getNextTypeDefinition(previousNameType, field.getGenericType()); + } + } } if (previousNameType != null) { @@ -325,6 +340,14 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return super.visit(node, data); } + private boolean isFieldInherited(Class classToSearch, String fieldImage) { + try { + return classToSearch.getDeclaredField(fieldImage).getDeclaringClass() != classToSearch; + } catch (NoSuchFieldException e) { + return true; + } + } + private JavaTypeDefinition getClassWrapperOfVariableFromScope(Scope scope, String image) { for (/* empty */; scope != null; scope = scope.getParent()) { for (Map.Entry> entry @@ -354,7 +377,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { ((ASTClassOrInterfaceType) classScope.getClassDeclaration().getNode() .getFirstChildOfType(ASTExtendsList.class).jjtGetChild(0)).getTypeDefinition(); - return getClassOfInheritedField(superClass, image); + return getInheritedFieldTypeDefinition(superClass, image); } } catch (ClassCastException e) { // if there is an anonymous class, getClassDeclaration().getType() will throw @@ -367,24 +390,30 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return null; } - private JavaTypeDefinition getClassOfInheritedField(JavaTypeDefinition inheritedClass, String fieldImage) { - while (true) { + private JavaTypeDefinition getInheritedFieldTypeDefinition(JavaTypeDefinition inheritedClass, String fieldImage) { + while (inheritedClass != null) { try { Field field = inheritedClass.getType().getDeclaredField(fieldImage); - return getNextClassWrapper(inheritedClass, field.getGenericType()); + return getNextTypeDefinition(inheritedClass, field.getGenericType()); } catch (NoSuchFieldException e) { /* swallow */ } - Type genericSuperClass = inheritedClass.getType().getGenericSuperclass(); - - if (genericSuperClass == null) { - return null; - } - - inheritedClass = getNextClassWrapper(inheritedClass, inheritedClass.getType().getGenericSuperclass()); + inheritedClass = getNextSuperClassTypeDefinition(inheritedClass); } + + return null; } - private JavaTypeDefinition getNextClassWrapper(JavaTypeDefinition context, Type genericType) { + private JavaTypeDefinition getNextSuperClassTypeDefinition(JavaTypeDefinition classWrapper) { + Type genericSuperClass = classWrapper.getType().getGenericSuperclass(); + + if (genericSuperClass == null) { + return null; + } + + return getNextTypeDefinition(classWrapper, genericSuperClass); + } + + private JavaTypeDefinition getNextTypeDefinition(JavaTypeDefinition context, Type genericType) { if (genericType instanceof Class) { return getDefaultUpperBounds(context, (Class) genericType); } else if (genericType instanceof ParameterizedType) { @@ -393,7 +422,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { JavaTypeDefinitionBuilder typeDef = JavaTypeDefinition.builder((Class) parameterizedType.getRawType()); for (Type type : parameterizedType.getActualTypeArguments()) { - typeDef.addTypeArg(getNextClassWrapper(context, type)); + typeDef.addTypeArg(getNextTypeDefinition(context, type)); } return typeDef.build(); @@ -405,7 +434,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } else if (genericType instanceof WildcardType) { Type[] wildcardUpperBounds = ((WildcardType) genericType).getUpperBounds(); if (wildcardUpperBounds.length != 0) { // upper bound wildcard - return getNextClassWrapper(context, wildcardUpperBounds[0]); + return getNextTypeDefinition(context, wildcardUpperBounds[0]); } else { // lower bound wildcard return JavaTypeDefinition.build(Object.class); } @@ -420,14 +449,14 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { private JavaTypeDefinition getDefaultUpperBounds(JavaTypeDefinition context, Class clazzWithDefBounds) { JavaTypeDefinitionBuilder typeDef = JavaTypeDefinition.builder(clazzWithDefBounds); - if(classToDefaultUpperBounds.containsKey(clazzWithDefBounds)) { + if (classToDefaultUpperBounds.containsKey(clazzWithDefBounds)) { return classToDefaultUpperBounds.get(clazzWithDefBounds); } else { classToDefaultUpperBounds.put(clazzWithDefBounds, typeDef.build()); } if (isGeneric(clazzWithDefBounds)) { - if(context == null) { + if (context == null) { context = typeDef.build(); } @@ -436,7 +465,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // TODO: fix self reference "< ... E extends Something ... >" - typeDef.addTypeArg(getNextClassWrapper(context, upperBound)); + typeDef.addTypeArg(getNextTypeDefinition(context, upperBound)); } } @@ -456,7 +485,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } private boolean isGeneric(Class clazz) { - if(clazz != null) { + if (clazz != null) { return clazz.getTypeParameters().length != 0; } @@ -647,14 +676,14 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return data; } + + @Override public Object visit(ASTPrimaryExpression primaryNode, Object data) { super.visit(primaryNode, data); - Class primaryNodeType = null; + JavaTypeDefinition primaryNodeType = null; AbstractJavaTypeNode previousChild = null; - JavaTypeDefinition previousClassWraper = null; - for (int childIndex = 0; childIndex < primaryNode.jjtGetNumChildren(); ++childIndex) { AbstractJavaTypeNode currentChild = (AbstractJavaTypeNode) primaryNode.jjtGetChild(childIndex); @@ -668,37 +697,45 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { ASTClassOrInterfaceDeclaration typeDeclaration = currentChild.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class); if (typeDeclaration != null) { - currentChild.setType(typeDeclaration.getType()); + currentChild.setTypeDefinition(typeDeclaration.getTypeDefinition()); } } + // Last token, because if 'super' is a Suffix, it'll have tokens '.' and 'super' } else if (currentChild.jjtGetLastToken().toString().equals("super")) { - if (previousChild != null) { // Qualified 'super' expression - currentChild.setType(previousChild.getType().getSuperclass()); - } else { // simple 'super' expression - ASTClassOrInterfaceDeclaration typeDeclaration - = currentChild.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class); - if (typeDeclaration != null && typeDeclaration.getType() != null) { - currentChild.setType(typeDeclaration.getType().getSuperclass()); - } + if (previousChild != null) { // Qualified 'this' expression + currentChild.setTypeDefinition(getSuperClassTypeDefinition(currentChild, previousChild + .getType())); + } else { // simple 'this' expression + currentChild.setTypeDefinition(getSuperClassTypeDefinition(currentChild, null)); } } else if (previousChild != null && previousChild.getType() != null && currentChild.getImage() != null) { - Field field = searchClassForField(previousChild.getType(), currentChild.getImage()); - if (field != null) { - currentChild.setType(field.getType()); + + if (isFieldInherited(previousChild.getType(), currentChild.getImage())) { + currentChild.setTypeDefinition( + getInheritedFieldTypeDefinition(getNextSuperClassTypeDefinition(previousChild.getTypeDefinition()), + currentChild.getImage()) + ); + } else { + Field field = searchClassForField(previousChild.getType(), currentChild.getImage()); + + if (field != null) { + currentChild.setTypeDefinition(getNextTypeDefinition(previousChild.getTypeDefinition(), + field.getGenericType())); + } } } } - if (currentChild.getType() != null) { - primaryNodeType = currentChild.getType(); + if (currentChild.getTypeDefinition() != null) { + primaryNodeType = currentChild.getTypeDefinition(); } previousChild = currentChild; } - primaryNode.setType(primaryNodeType); + primaryNode.setTypeDefinition(primaryNodeType); return data; } @@ -713,6 +750,23 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return null; } + private JavaTypeDefinition getSuperClassTypeDefinition(Node node, Class clazz) { + for (; node != null; node = node.jjtGetParent()) { + if (node instanceof ASTClassOrInterfaceDeclaration + && (((ASTClassOrInterfaceDeclaration) node).getType() == clazz || clazz == null)) { + ASTExtendsList extendsList = node.getFirstChildOfType(ASTExtendsList.class); + + if (extendsList != null) { + return ((TypeNode) extendsList.jjtGetChild(0)).getTypeDefinition(); + } else { + return JavaTypeDefinition.build(Object.class); + } + } + } + + return null; + } + @Override public Object visit(ASTPrimaryPrefix node, Object data) { super.visit(node, data); @@ -756,6 +810,19 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { @Override public Object visit(ASTTypeParameters node, Object data) { super.visit(node, data); + + 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()); + } + + parent.setTypeDefinition(builder.build()); + } + return data; } @@ -1004,8 +1071,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // try generics // TODO: generic declarations can shadow type declarations ... :( - // TODO: incorrect if type parameter upper bound is generic - // TODO: ? and ? super is not covered if (myType == null) { ASTTypeParameter parameter = getTypeParameterDeclaration(node, className); if (parameter != null) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index c071056698..b6742e077e 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -6,6 +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 java.io.IOException; import java.io.InputStream; @@ -14,10 +15,12 @@ import java.util.ArrayList; import java.util.List; import java.util.StringTokenizer; +import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericBounds; import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericParameter; import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericRaw; import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericSimple; +import net.sourceforge.pmd.typeresolution.testdata.FieldAccessPrimaryGenericSimple; import org.apache.commons.io.IOUtils; import org.jaxen.JaxenException; import org.junit.Assert; @@ -883,6 +886,14 @@ public class ClassTypeResolverTest { assertEquals(String.class, expressions.get(index).getType()); assertEquals(String.class, getChildType(expressions.get(index++), 0)); + // fieldAcc.fieldA = new Long(0); + assertEquals(Long.class, expressions.get(index).getType()); + assertEquals(Long.class, getChildType(expressions.get(index++), 0)); + + // fieldA = new Long(0); + assertEquals(Long.class, expressions.get(index).getType()); + assertEquals(Long.class, getChildType(expressions.get(index++), 0)); + // Make sure we got them all assertEquals("All expressions not tested", index, expressions.size()); } @@ -951,6 +962,64 @@ public class ClassTypeResolverTest { assertEquals("All expressions not tested", index, expressions.size()); } + @Test + public void testPrimarySimpleGenericFieldAccess() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(FieldAccessPrimaryGenericSimple.class); + + List expressions = convertList( + acu.findChildNodesWithXPath("//StatementExpression/PrimaryExpression"), + AbstractJavaTypeNode.class); + + + int index = 0; + + JavaTypeDefinition typeDef; + + + // this.genericField.first = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertChildTypeArgsEqualTo(expressions.get(index), 1, String.class, Double.class); + assertEquals(String.class, getChildType(expressions.get(index++), 2)); + + // (this).genericField.second = new Double(0); + assertEquals(Double.class, expressions.get(index).getType()); + assertChildTypeArgsEqualTo(expressions.get(index), 1, String.class, Double.class); + assertEquals(Double.class, getChildType(expressions.get(index++), 2)); + + // this.genericTypeArg.second.second = new Double(0); + assertEquals(Double.class, expressions.get(index).getType()); + assertChildTypeArgsEqualTo(expressions.get(index), 2, Number.class, Double.class); + assertEquals(Double.class, getChildType(expressions.get(index++), 3)); + + // (this).genericField.generic.generic.generic.first = new Double(0); + assertEquals(Double.class, expressions.get(index).getType()); + assertEquals(Double.class, getChildType(expressions.get(index++), 5)); + + // (this).fieldA = new Long(0); + assertEquals(Long.class, expressions.get(index).getType()); + assertEquals(Long.class, getChildType(expressions.get(index++), 1)); + + // this.fieldB.generic.second = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 3)); + + // super.fieldA = new Long(0); + assertEquals(Long.class, expressions.get(index).getType()); + assertChildTypeArgsEqualTo(expressions.get(index), 0, Long.class); + assertEquals(Long.class, getChildType(expressions.get(index++), 1)); + + // super.fieldB.generic.second = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 3)); + + // this.field.first = ""; + assertEquals(String.class, expressions.get(index).getType()); + assertEquals(String.class, getChildType(expressions.get(index++), 2)); + + // Make sure we got them all + assertEquals("All expressions not tested", index, expressions.size()); + } + private Class getSiblingType(Node node, int siblingIndex) { Node parent = node.jjtGetParent(); if (parent.jjtGetNumChildren() > siblingIndex) { @@ -964,14 +1033,17 @@ public class ClassTypeResolverTest { } private Class getChildType(Node node, int childIndex) { - if (node.jjtGetNumChildren() > childIndex) { - Node child = node.jjtGetChild(childIndex); - if (child instanceof TypeNode) { - return ((TypeNode) child).getType(); - } - } + return ((TypeNode) node.jjtGetChild(childIndex)).getType(); + } - return null; + 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]); + } } private ASTCompilationUnit parseAndTypeResolveForClass15(Class clazz) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java index 79fb463818..80760aef74 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java @@ -5,10 +5,7 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericSuperClassA /* - * TODO: test [nested classes, shadowing] - * TODO: add not name cases * TODO: add anonymous class this (Allocation expression) - * * TODO: add primitives, parameterized arrays * TODO: diamond, type parmeter declarations can shadow Class declarations */ @@ -16,6 +13,7 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericSuperClassA public class FieldAccessGenericSimple extends GenericSuperClassA { GenericClass genericField; GenericClass> genericTypeArg; + FieldAccessGenericSimple fieldAcc; void foo(GenericClass param) { GenericClass local = null; @@ -44,5 +42,15 @@ public class FieldAccessGenericSimple extends GenericSuperClassA { // Primary[Prefix[Name[generic.first]]] fieldA = new Long(0); fieldB.generic.second = ""; + + // test inherited generic + // Primary[Prefix[Name[fieldAcc.fieldA]]] + fieldAcc.fieldA = new Long(0); + } + + public class Nested extends GenericSuperClassA { + void foo() { + fieldA = new Long(0); + } } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessPrimaryGenericSimple.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessPrimaryGenericSimple.java new file mode 100644 index 0000000000..29b5a563ee --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessPrimaryGenericSimple.java @@ -0,0 +1,44 @@ +package net.sourceforge.pmd.typeresolution.testdata; + +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; +import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericSuperClassA; + +public class FieldAccessPrimaryGenericSimple extends GenericSuperClassA { + GenericClass genericField; + GenericClass> genericTypeArg; + + void foo(GenericClass param) { + GenericClass local = null; + + // access a generic field through member field + // Primary[Prefix[this], Suffix[genericField], Suffix[first]] + this.genericField.first = ""; + (this).genericField.second = new Double(0); + + // access a generic field whose type depends on a generic type argument + // Primary[Prefix[this], Suffix[genericTypeArg], Suffix[second], Suffix[second]] + this.genericTypeArg.second.second = new Double(0); + + // access a generic field whose type depends on indirect type arguments + // Primary[Prefix[this], Suffix[genericField], Suffix[generic], Suffix[generic]...] + (this).genericField.generic.generic.generic.first = new Double(0); + + // test inherited generic + // Primary[Primary[Prefix[(this)]], Suffix[fieldA]] + (this).fieldA = new Long(0); + this.fieldB.generic.second = ""; + + // test inherited generic + // Primary[Prefix[super], Suffix[fieldA]] + super.fieldA = new Long(0); + super.fieldB.generic.second = ""; + } + + class Nested> { + T field; + void foo() { + // Primary[Prefix[this], Suffix[field], Suffix[first]] + this.field.first = ""; + } + } +} From a0ede7e4c69cf6679bfbbb580dbca02c09aa1ee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Wed, 14 Jun 2017 23:37:44 +0200 Subject: [PATCH 13/20] Java, typeres: optimize visit(ASTName) by skipping package and import declarations --- .../java/typeresolution/ClassTypeResolver.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 845ad05695..b244354607 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -180,9 +180,15 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return super.visit(node, data); } + @Override + public Object visit(ASTPackageDeclaration node, Object data) { + return data; + } + @Override public Object visit(ASTImportDeclaration node, Object data) { ASTName importedType = (ASTName) node.jjtGetChild(0); + if (importedType.getType() != null) { node.setType(importedType.getType()); } else { @@ -192,6 +198,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (node.getType() != null) { node.setPackage(node.getType().getPackage()); } + return data; } @@ -244,7 +251,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * not */ - Class accessingClass = getEnclosingTypeDeclaration(node); String[] dotSplitImage = node.getImage().split("\\."); @@ -258,12 +264,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } - // Skip these scenarios as there is no type to populate in these - // cases: - // 1) Parent is a PackageDeclaration, which is not a type - // 2) Parent is a ImportDeclaration, this is handled elsewhere. - if (node.getType() == null && !(node.jjtGetParent() instanceof ASTPackageDeclaration - || node.jjtGetParent() instanceof ASTImportDeclaration)) { + if (node.getType() == null) { populateType(node, node.getImage()); From b8e1738f55d35c6927e66cfa1ac0afa02b4d4915 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Thu, 15 Jun 2017 02:48:15 +0200 Subject: [PATCH 14/20] Java, typeres: clean up code --- .../typeresolution/ClassTypeResolver.java | 255 +++++++----------- .../typeresolution/ClassTypeResolverTest.java | 6 + .../typeresolution/testdata/FieldAccess.java | 3 +- .../testdata/FieldAccessNested.java | 1 + .../testdata/FieldAccessShadow.java | 5 - 5 files changed, 112 insertions(+), 158 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index a9148ae6d1..c0d803353c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -291,78 +291,66 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { Class accessingClass = getEnclosingTypeDeclaration(node); String[] dotSplitImage = node.getImage().split("\\."); - JavaTypeDefinition previousNameType - = getClassWrapperOfVariableFromScope(node.getScope(), dotSplitImage[0], accessingClass); + JavaTypeDefinition previousType + = getTypeDefinitionOfVariableFromScope(node.getScope(), dotSplitImage[0], accessingClass); - if (node.getNameDeclaration() != null - && previousNameType == null + + if (node.getNameDeclaration() != null // + && previousType == null // if it's not null, then let other code handle things && 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 (!isGeneric(nodeType) && !isGeneric(nodeType.getSuperclass())) { node.setType(nodeType); } } - /* - * Only doing this for nodes where getNameDeclaration is null this means - * it's not a named node, i.e. Static reference or Annotation Doing this - * for memory - TODO: Investigate if there is a valid memory concern or - * not - */ - if (node.getType() == null) { + // TODO: handle cases where static fields are accessed in a fully qualified way + // make sure it handles same name classes and packages + // TODO: handle generic static field cases + // handles cases where first part is a fully qualified name populateType(node, node.getImage()); - // cases like EnclosingScope.something ... - if (node.getType() == null) { - populateType(node, dotSplitImage[0]); - } - if (node.getType() == null) { for (int i = 1; i < dotSplitImage.length; ++i) { - if (previousNameType == null) { + if (previousType == null) { break; } - - if (isFieldInherited(previousNameType.getType(), dotSplitImage[i])) { - previousNameType = getInheritedFieldTypeDefinition(getNextSuperClassTypeDefinition(previousNameType), - dotSplitImage[i]); - } else { - Field field = getFirstVisibleFieldFromClass(previousNameType.getType(), - dotSplitImage[i], accessingClass); - if(field != null) { - previousNameType = getNextTypeDefinition(previousNameType, field.getGenericType()); - } else { - previousNameType = null; - } - } + previousType = getFieldType(previousType, dotSplitImage[i], accessingClass); } - if (previousNameType != null) { - node.setTypeDefinition(previousNameType); + if (previousType != null) { + node.setTypeDefinition(previousType); } } } return super.visit(node, data); - } - private boolean isFieldInherited(Class classToSearch, String fieldImage) { - try { - return classToSearch.getDeclaredField(fieldImage).getDeclaringClass() != classToSearch; - } catch (NoSuchFieldException e) { - return true; + private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class accessingClass) { + while (typeToSearch != null) { + try { + Field field = typeToSearch.getType().getDeclaredField(fieldImage); + if (isMemberVisibleFromClass(typeToSearch.getType(), field.getModifiers(), accessingClass)) { + return getNextTypeDefinition(typeToSearch, field.getGenericType()); + } + } catch (NoSuchFieldException e) { /* swallow */ } + + typeToSearch = getNextTypeDefinition(typeToSearch, typeToSearch.getType().getGenericSuperclass()); } + + return null; } - private JavaTypeDefinition getClassWrapperOfVariableFromScope(Scope scope, String image, Class accessingClass) { - if (accessingClass == null) { - return null; - } + private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class accessingClass) { + if (accessingClass == null) { + return null; + } for (/* empty */; scope != null; scope = scope.getParent()) { for (Map.Entry> entry @@ -382,17 +370,14 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // Nested class' inherited fields shadow enclosing variables if (scope instanceof ClassScope) { try { - ClassScope classScope = (ClassScope) scope; + JavaTypeDefinition superClass + = getSuperClassTypeDefinition(((ClassScope) scope).getClassDeclaration().getNode(), + null); - Field inheritedField = getFirstVisibleFieldFromClass(classScope.getClassDeclaration().getType(), - image, accessingClass); + JavaTypeDefinition foundTypeDef = getFieldType(superClass, image, accessingClass); - if (inheritedField != null) { - JavaTypeDefinition superClass = - ((ASTClassOrInterfaceType) classScope.getClassDeclaration().getNode() - .getFirstChildOfType(ASTExtendsList.class).jjtGetChild(0)).getTypeDefinition(); - - return getInheritedFieldTypeDefinition(superClass, image); + if (foundTypeDef != null) { + return foundTypeDef; } } catch (ClassCastException e) { // if there is an anonymous class, getClassDeclaration().getType() will throw @@ -404,30 +389,11 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return null; } - private JavaTypeDefinition getInheritedFieldTypeDefinition(JavaTypeDefinition inheritedClass, String fieldImage) { - while (inheritedClass != null) { - try { - Field field = inheritedClass.getType().getDeclaredField(fieldImage); - return getNextTypeDefinition(inheritedClass, field.getGenericType()); - } catch (NoSuchFieldException e) { /* swallow */ } - - inheritedClass = getNextSuperClassTypeDefinition(inheritedClass); - } - - return null; - } - - private JavaTypeDefinition getNextSuperClassTypeDefinition(JavaTypeDefinition classWrapper) { - Type genericSuperClass = classWrapper.getType().getGenericSuperclass(); - - if (genericSuperClass == null) { + private JavaTypeDefinition getNextTypeDefinition(JavaTypeDefinition context, Type genericType) { + if (genericType == null) { return null; } - return getNextTypeDefinition(classWrapper, genericSuperClass); - } - - private JavaTypeDefinition getNextTypeDefinition(JavaTypeDefinition context, Type genericType) { if (genericType instanceof Class) { return getDefaultUpperBounds(context, (Class) genericType); } else if (genericType instanceof ParameterizedType) { @@ -457,6 +423,25 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return null; } + 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; + } + + private boolean isGeneric(Class clazz) { + if (clazz != null) { + return clazz.getTypeParameters().length != 0; + } + + return false; + } private Map classToDefaultUpperBounds = new HashMap<>(); @@ -486,21 +471,40 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return typeDef.build(); } - 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; - } + private boolean isMemberVisibleFromClass(Class classWithMember, int modifiers, Class accessingClass) { + if (accessingClass == null) { + return false; } - return -1; - } + // public members + if (Modifier.isPublic(modifiers)) { + return true; + } - private boolean isGeneric(Class clazz) { - if (clazz != null) { - return clazz.getTypeParameters().length != 0; + Package accessingPackage = accessingClass.getPackage(); + boolean areInTheSamePackage; + if (accessingPackage != null) { + areInTheSamePackage = accessingPackage.getName().startsWith( + classWithMember.getPackage().getName()); + } else { + return false; + } + + // protected members + if (Modifier.isProtected(modifiers) + && (areInTheSamePackage || classWithMember.isAssignableFrom(accessingClass))) { + return true; + } + + // package private + if (!(Modifier.isPrivate(modifiers) || Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers)) + && areInTheSamePackage) { + return true; + } + + // private members + if (Modifier.isPrivate(modifiers) && classWithMember.equals(accessingClass)) { + return true; } return false; @@ -691,7 +695,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } - @Override public Object visit(ASTPrimaryExpression primaryNode, Object data) { super.visit(primaryNode, data); @@ -707,11 +710,13 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (currentChild.getType() == null) { // Last token, because if 'this' is a Suffix, it'll have tokens '.' and 'this' if (currentChild.jjtGetLastToken().toString().equals("this")) { + if (previousChild != null) { // Qualified 'this' expression currentChild.setType(previousChild.getType()); } else { // simple 'this' expression ASTClassOrInterfaceDeclaration typeDeclaration = currentChild.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class); + if (typeDeclaration != null) { currentChild.setTypeDefinition(typeDeclaration.getTypeDefinition()); } @@ -719,28 +724,20 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // Last token, because if 'super' is a Suffix, it'll have tokens '.' and 'super' } else if (currentChild.jjtGetLastToken().toString().equals("super")) { + if (previousChild != null) { // Qualified 'this' expression - currentChild.setTypeDefinition(getSuperClassTypeDefinition(currentChild, previousChild - .getType())); + currentChild.setTypeDefinition( + getSuperClassTypeDefinition(currentChild, previousChild.getType())); } else { // simple 'this' expression currentChild.setTypeDefinition(getSuperClassTypeDefinition(currentChild, null)); } + } else if (previousChild != null && previousChild.getType() != null && currentChild.getImage() != null) { - if (isFieldInherited(previousChild.getType(), currentChild.getImage())) { - currentChild.setTypeDefinition( - getInheritedFieldTypeDefinition(getNextSuperClassTypeDefinition(previousChild.getTypeDefinition()), - currentChild.getImage()) - ); - } else { - Field field = getFirstVisibleFieldFromClass(previousChild.getType(), currentChild.getImage(), - accessingClass); - if (field != null) { - currentChild.setTypeDefinition(getNextTypeDefinition(previousChild.getTypeDefinition(), - field.getGenericType())); - } - } + currentChild.setTypeDefinition(getFieldType(previousChild.getTypeDefinition(), + currentChild.getImage(), + accessingClass)); } } @@ -765,9 +762,10 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { while (node != null) { if (node instanceof ASTClassOrInterfaceDeclaration) { return ((TypeNode) node).getType(); + // anonymous class declaration } else if (node instanceof ASTAllocationExpression) { ASTClassOrInterfaceType typeDecl = node.getFirstChildOfType(ASTClassOrInterfaceType.class); - if (typeDecl != null && typeDecl.getType() != null) { + if (typeDecl != null) { return typeDecl.getType(); } } @@ -778,61 +776,11 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return null; } - private Field getFirstVisibleFieldFromClass(Class classToSerach, String fieldName, Class accessingClass) { - for ( /* empty */; classToSerach != null; classToSerach = classToSerach.getSuperclass()) { - try { - Field field = classToSerach.getDeclaredField(fieldName); - if (isMemberVisibleFromClass(classToSerach, field.getModifiers(), accessingClass)) { - return field; - } - } catch (NoSuchFieldException e) { /* swallow */ } - } - return null; - } - - private boolean isMemberVisibleFromClass(Class classWithMember, int modifiers, Class accessingClass) { - if (accessingClass == null) { - return false; - } - - // public members - if (Modifier.isPublic(modifiers)) { - return true; - } - - Package accessingPackage = accessingClass.getPackage(); - boolean areInTheSamePackage; - if (accessingPackage != null) { - areInTheSamePackage = accessingPackage.getName().startsWith( - classWithMember.getPackage().getName()); - } else { - return false; - } - - // protected members - if (Modifier.isProtected(modifiers) - && (areInTheSamePackage || classWithMember.isAssignableFrom(accessingClass))) { - return true; - } - - // package private - if (!(Modifier.isPrivate(modifiers) || Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers)) - && areInTheSamePackage) { - return true; - } - - // private members - if (Modifier.isPrivate(modifiers) && classWithMember.equals(accessingClass)) { - return true; - } - - return false; - } - private JavaTypeDefinition getSuperClassTypeDefinition(Node node, Class clazz) { for (; node != null; node = node.jjtGetParent()) { if (node instanceof ASTClassOrInterfaceDeclaration && (((ASTClassOrInterfaceDeclaration) node).getType() == clazz || clazz == null)) { + ASTExtendsList extendsList = node.getFirstChildOfType(ASTExtendsList.class); if (extendsList != null) { @@ -840,6 +788,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } else { return JavaTypeDefinition.build(Object.class); } + // anonymous class declaration + } else if (node instanceof ASTAllocationExpression) { + return node.getFirstChildOfType(ASTClassOrInterfaceType.class).getTypeDefinition(); } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index 900eab7b0d..f9fc14bda5 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -744,6 +744,12 @@ public class ClassTypeResolverTest { assertEquals(FieldAccessNested.Nested.class, getChildType(expressions.get(index), 1)); assertEquals(SuperClassA.class, getChildType(expressions.get(index++), 2)); + // FieldAccessNested.Nested.this.a = new SuperClassA(); + assertEquals(SuperClassA.class, expressions.get(index).getType()); + assertEquals(FieldAccessNested.Nested.class, getChildType(expressions.get(index), 0)); + assertEquals(FieldAccessNested.Nested.class, getChildType(expressions.get(index), 1)); + assertEquals(SuperClassA.class, getChildType(expressions.get(index++), 2)); + // Make sure we got them all assertEquals("All expressions not tested", index, expressions.size()); } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java index ccdce624ca..f88aaab8a4 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java @@ -13,11 +13,12 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.SuperClassA; * Note: inherited fields of a nested class shadow outer scope variables * Note: only if they are accessible! * - * TODO: test static field access, array types + * TODO: test static field access, array types, anonymous class */ public class FieldAccess extends SuperClassA { public int field; public FieldAccess f; + public static int a; public void foo(FieldAccess param) { FieldAccess local = null; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessNested.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessNested.java index 2537e4054e..6c6ca983b4 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessNested.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessNested.java @@ -29,6 +29,7 @@ public class FieldAccessNested { a = new SuperClassA(); net.sourceforge.pmd.typeresolution.testdata.FieldAccessNested.Nested.this.a = new SuperClassA(); + FieldAccessNested.Nested.this.a = new SuperClassA(); } } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessShadow.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessShadow.java index 7ec5380549..99d27d0ccd 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessShadow.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessShadow.java @@ -16,13 +16,8 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.SuperClassB2; */ public class FieldAccessShadow { Integer field; - String s2; - - - - public void foo() { String field; From bd187a3c63d6ef844ac2277e8aa874b98c5717cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Thu, 15 Jun 2017 10:45:01 +0200 Subject: [PATCH 15/20] Java, typeres: fix checkstyle and tests --- .../typeresolution/ClassTypeResolver.java | 5 +- .../typedefinition/JavaTypeDefinition.java | 4 ++ .../JavaTypeDefinitionBuilder.java | 4 ++ .../typedefinition/TypeDefinition.java | 4 ++ .../typeresolution/ClassTypeResolverTest.java | 51 +++++++++---------- .../testdata/FieldAccessGenericBounds.java | 4 ++ .../testdata/FieldAccessGenericParameter.java | 4 ++ .../testdata/FieldAccessGenericRaw.java | 4 ++ .../testdata/FieldAccessGenericSimple.java | 4 ++ .../FieldAccessPrimaryGenericSimple.java | 5 ++ .../testdata/dummytypes/GenericClass2.java | 4 ++ .../dummytypes/GenericSuperClassA.java | 4 ++ .../dummytypes/GenericSuperClassB.java | 4 ++ .../testdata/dummytypes/StaticFields.java | 4 ++ 14 files changed, 75 insertions(+), 30 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index c0d803353c..39fbfb02f9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -301,7 +301,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // 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 (!isGeneric(nodeType) && !isGeneric(nodeType.getSuperclass())) { + if (nodeType != null && !isGeneric(nodeType) && !isGeneric(nodeType.getSuperclass())) { node.setType(nodeType); } } @@ -359,8 +359,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { ASTType typeNode = entry.getKey().getDeclaratorId().getTypeNode(); if (typeNode.jjtGetChild(0) instanceof ASTReferenceType) { - return ((ASTClassOrInterfaceType) typeNode.jjtGetChild(0).jjtGetChild(0)) - .getTypeDefinition(); + return ((TypeNode) typeNode.jjtGetChild(0)).getTypeDefinition(); } else { // primitive type return JavaTypeDefinition.build(typeNode.getType()); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java index 28eaf628b7..24e8a43d2c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java @@ -1,3 +1,7 @@ +/** + * 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; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java index f3edd3e9f4..f2f13aa632 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java @@ -1,3 +1,7 @@ +/** + * 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; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java index ecde6e5c06..fb270a392b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.lang.java.typeresolution.typedefinition; import java.util.List; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index f9fc14bda5..dcc4e8595c 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -15,19 +15,11 @@ import java.util.ArrayList; import java.util.List; import java.util.StringTokenizer; -import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; -import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericBounds; -import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericParameter; -import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericRaw; -import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericSimple; -import net.sourceforge.pmd.typeresolution.testdata.FieldAccessPrimaryGenericSimple; import org.apache.commons.io.IOUtils; import org.jaxen.JaxenException; import org.junit.Assert; import org.junit.Test; - - import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.LanguageVersionHandler; import net.sourceforge.pmd.lang.ast.Node; @@ -57,13 +49,19 @@ 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; import net.sourceforge.pmd.typeresolution.testdata.AnonymousInnerClass; import net.sourceforge.pmd.typeresolution.testdata.ArrayListFound; import net.sourceforge.pmd.typeresolution.testdata.DefaultJavaLangImport; import net.sourceforge.pmd.typeresolution.testdata.EnumWithAnonymousInnerClass; import net.sourceforge.pmd.typeresolution.testdata.ExtraTopLevelClass; import net.sourceforge.pmd.typeresolution.testdata.FieldAccess; +import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericBounds; +import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericParameter; +import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericRaw; +import net.sourceforge.pmd.typeresolution.testdata.FieldAccessGenericSimple; import net.sourceforge.pmd.typeresolution.testdata.FieldAccessNested; +import net.sourceforge.pmd.typeresolution.testdata.FieldAccessPrimaryGenericSimple; import net.sourceforge.pmd.typeresolution.testdata.FieldAccessShadow; import net.sourceforge.pmd.typeresolution.testdata.FieldAccessSuper; import net.sourceforge.pmd.typeresolution.testdata.InnerClass; @@ -78,7 +76,6 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.SuperClassB; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.SuperClassB2; - public class ClassTypeResolverTest { @Test @@ -165,9 +162,10 @@ public class ClassTypeResolverTest { @Test public void testInnerClassNotCompiled() throws Exception { Node acu = parseAndTypeResolveForString("public class TestInnerClass {\n" + " public void foo() {\n" - + " Statement statement = new Statement();\n" + " " + - "}\n" + " static class Statement {\n" - + " }\n" + "}", "1.8"); + + " Statement statement = new Statement();\n" + " " + + "}\n" + " static class Statement {\n" + + " }\n" + + "}", "1.8"); ASTClassOrInterfaceType statement = acu.getFirstDescendantOfType(ASTClassOrInterfaceType.class); Assert.assertTrue(statement.isReferenceToClassSameCompilationUnit()); } @@ -343,8 +341,8 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = parseAndTypeResolveForClass15(Promotion.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " + - "'unaryNumericPromotion']]//Expression[UnaryExpression]"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'unaryNumericPromotion']]//Expression[UnaryExpression]"), ASTExpression.class); int index = 0; @@ -365,8 +363,8 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = parseAndTypeResolveForClass15(Promotion.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " + - "'binaryNumericPromotion']]//Expression[AdditiveExpression]"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'binaryNumericPromotion']]//Expression[AdditiveExpression]"), ASTExpression.class); int index = 0; @@ -503,18 +501,18 @@ public class ClassTypeResolverTest { TypeNode.class)); expressions.addAll(convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " + - "'unaryNumericOperators']]//PostfixExpression"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'unaryNumericOperators']]//PostfixExpression"), TypeNode.class)); expressions.addAll(convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " + - "'unaryNumericOperators']]//PreIncrementExpression"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'unaryNumericOperators']]//PreIncrementExpression"), TypeNode.class)); expressions.addAll(convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " + - "'unaryNumericOperators']]//PreDecrementExpression"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'unaryNumericOperators']]//PreDecrementExpression"), TypeNode.class)); int index = 0; @@ -564,8 +562,8 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = parseAndTypeResolveForClass15(Operators.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " + - "'assignmentOperators']]//StatementExpression"), + "//Block[preceding-sibling::MethodDeclarator[@Image = " + + "'assignmentOperators']]//StatementExpression"), ASTStatementExpression.class); int index = 0; @@ -669,8 +667,8 @@ public class ClassTypeResolverTest { assertEquals(SuperClassA.class, expressions.get(index++).getType()); assertEquals(SuperClassA.class, expressions.get(index++).getType()); assertEquals(SuperClassA.class, expressions.get(index++).getType()); - assertEquals(SuperExpression.class, ((TypeNode) expressions.get(index).jjtGetParent().jjtGetChild(0)).getType - ()); + assertEquals(SuperExpression.class, ((TypeNode) expressions.get(index).jjtGetParent().jjtGetChild(0)) + .getType()); assertEquals(SuperClassA.class, ((TypeNode) expressions.get(index++).jjtGetParent().jjtGetChild(1)).getType()); assertEquals(SuperExpression.class, expressions.get(index++).getType()); @@ -682,7 +680,6 @@ public class ClassTypeResolverTest { } - @Test public void testFieldAccess() throws JaxenException { ASTCompilationUnit acu = parseAndTypeResolveForClass15(FieldAccess.class); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericBounds.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericBounds.java index e16da1f6fb..2731c6f208 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericBounds.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericBounds.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.typeresolution.testdata; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericParameter.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericParameter.java index b2868024f9..6f273a0198 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericParameter.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericParameter.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.typeresolution.testdata; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericRaw.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericRaw.java index 1353acdaab..7cf7cb9a8a 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericRaw.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericRaw.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.typeresolution.testdata; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass2; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java index 80760aef74..198b9fdb10 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessGenericSimple.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.typeresolution.testdata; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessPrimaryGenericSimple.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessPrimaryGenericSimple.java index 29b5a563ee..8480f69b14 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessPrimaryGenericSimple.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccessPrimaryGenericSimple.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.typeresolution.testdata; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; @@ -36,6 +40,7 @@ public class FieldAccessPrimaryGenericSimple extends GenericSuperClassA { class Nested> { T field; + void foo() { // Primary[Prefix[this], Suffix[field], Suffix[first]] this.field.first = ""; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java index f9d3c6aab4..e922dcaea3 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericClass2.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.typeresolution.testdata.dummytypes; public class GenericClass2 extends GenericSuperClassB> { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassB.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassB.java index e228829379..62de0a7837 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassB.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/GenericSuperClassB.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.typeresolution.testdata.dummytypes; public class GenericSuperClassB { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java index bc3baaa9fb..71ee2eb86a 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.typeresolution.testdata.dummytypes; public class StaticFields { From 4175460d3559266c2046a632e40e6c878ade355f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Thu, 15 Jun 2017 15:23:19 +0200 Subject: [PATCH 16/20] Java, typeres: clean up code, add comments --- .../typeresolution/ClassTypeResolver.java | 156 ++++++++++++++---- .../typeresolution/testdata/FieldAccess.java | 4 +- .../testdata/dummytypes/StaticFields.java | 14 -- 3 files changed, 127 insertions(+), 47 deletions(-) delete mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 39fbfb02f9..6437176453 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -24,6 +24,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTAdditiveExpression; import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; import net.sourceforge.pmd.lang.java.ast.ASTAndExpression; import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTArguments; import net.sourceforge.pmd.lang.java.ast.ASTArrayDimsAndInits; import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral; import net.sourceforge.pmd.lang.java.ast.ASTCastExpression; @@ -264,7 +265,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { public Object visit(ASTExtendsList node, Object data) { super.visit(node, data); - return data; } @@ -332,6 +332,16 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return super.visit(node, data); } + /** + * Searches a JavaTypeDefinition and it's superclasses until a field with name {@code fieldImage} that + * is visible from the {@code accessingClass} class. Once it's found, it's possibly generic type is + * resolved with the help of {@code typeToSearch} TypeDefinition. + * + * @param typeToSearch The type def. to search the field in. + * @param fieldImage The simple name of the field. + * @param accessingClass The class that is trying to access the field, some Class declared in the current ACU. + * @return JavaTypeDefinition of the resolved field or null if it could not be found. + */ private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class accessingClass) { while (typeToSearch != null) { try { @@ -341,18 +351,29 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } } catch (NoSuchFieldException e) { /* swallow */ } + // transform the type into it's supertype typeToSearch = getNextTypeDefinition(typeToSearch, typeToSearch.getType().getGenericSuperclass()); } return null; } + /** + * Search for a field by it's image stating from a scope and taking into account if it's visible from the + * accessingClass Class. The method takes into account that Nested inherited fields shadow outer scope fields. + * + * @param scope The scope to start the search from. + * @param image The name of the field, local variable or method parameter. + * @param accessingClass The Class (which is defined in the current ACU) that is trying to access the field. + * @return Type def. of the field, or null if it could not be resolved. + */ private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class accessingClass) { if (accessingClass == null) { return null; } for (/* empty */; scope != null; scope = scope.getParent()) { + // search each enclosing scope one by one for (Map.Entry> entry : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { if (entry.getKey().getImage().equals(image)) { @@ -369,13 +390,16 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // Nested class' inherited fields shadow enclosing variables if (scope instanceof ClassScope) { try { + // get the superclass type def. ot the Class the ClassScope belongs to JavaTypeDefinition superClass = getSuperClassTypeDefinition(((ClassScope) scope).getClassDeclaration().getNode(), null); + // TODO: check if anonymous classes are class scope + // try searching this type def. JavaTypeDefinition foundTypeDef = getFieldType(superClass, image, accessingClass); - if (foundTypeDef != null) { + if (foundTypeDef != null) { // if null, then it's not an inherited field return foundTypeDef; } } catch (ClassCastException e) { @@ -388,18 +412,28 @@ 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) { if (genericType == null) { return null; } - if (genericType instanceof Class) { + 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()); + // recursively determine each type argument's type def. for (Type type : parameterizedType.getActualTypeArguments()) { typeDef.addTypeArg(getNextTypeDefinition(context, type)); } @@ -422,6 +456,13 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { 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(); @@ -434,6 +475,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { 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; @@ -442,11 +489,23 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return false; } + /** + * Contains Class -> JavaTypeDefinitions map for raw Class types. Also helps to avoid infinite recursion. + */ private Map classToDefaultUpperBounds = new HashMap<>(); + /** + * 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 { @@ -454,22 +513,31 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } 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 ... > if (context == null) { context = typeDef.build(); } for (TypeVariable parameter : clazzWithDefBounds.getTypeParameters()) { - Type upperBound = parameter.getBounds()[0]; - // TODO: fix self reference "< ... E extends Something ... >" - - typeDef.addTypeArg(getNextTypeDefinition(context, upperBound)); + 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. + * + * @param classWithMember The Class with the member. + * @param modifiers The modifiers of that member. + * @param accessingClass The Class trying to access the member. + * @return True if the member is visible from the accessingClass Class. + */ private boolean isMemberVisibleFromClass(Class classWithMember, int modifiers, Class accessingClass) { if (accessingClass == null) { return false; @@ -480,29 +548,26 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return true; } - Package accessingPackage = accessingClass.getPackage(); boolean areInTheSamePackage; - if (accessingPackage != null) { - areInTheSamePackage = accessingPackage.getName().startsWith( + if (accessingClass.getPackage() != null) { + areInTheSamePackage = accessingClass.getPackage().getName().startsWith( classWithMember.getPackage().getName()); } else { - return false; + return false; // if the package information is null, we can't do nothin' } // protected members - if (Modifier.isProtected(modifiers) - && (areInTheSamePackage || classWithMember.isAssignableFrom(accessingClass))) { - return true; - } - - // package private - if (!(Modifier.isPrivate(modifiers) || Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers)) - && areInTheSamePackage) { - return true; - } - - // private members - if (Modifier.isPrivate(modifiers) && classWithMember.equals(accessingClass)) { + if (Modifier.isProtected(modifiers)) { + if (areInTheSamePackage || classWithMember.isAssignableFrom(accessingClass)) { + return true; + } + // private members + } else if (Modifier.isPrivate(modifiers)) { + if (classWithMember.equals(accessingClass)) { + return true; + } + // package private members + } else if (areInTheSamePackage) { return true; } @@ -724,10 +789,13 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // Last token, because if 'super' is a Suffix, it'll have tokens '.' and 'super' } else if (currentChild.jjtGetLastToken().toString().equals("super")) { - if (previousChild != null) { // Qualified 'this' expression + if (previousChild != null) { // Qualified 'super' expression + // anonymous classes can't have qualified super expression, thus + // getSuperClassTypeDefinition's second argumet isn't null, but we are not + // looking for enclosing super types currentChild.setTypeDefinition( getSuperClassTypeDefinition(currentChild, previousChild.getType())); - } else { // simple 'this' expression + } else { // simple 'super' expression currentChild.setTypeDefinition(getSuperClassTypeDefinition(currentChild, null)); } @@ -757,28 +825,50 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return data; } + /** + * Returns the type def. of the first Class declaration around the node. Looks for Class declarations + * and if the second argument is null, then for anonymous classes as well. + * + * @param node The node with the enclosing Class declaration. + * @return The JavaTypeDefinition of the enclosing Class declaration. + */ private Class getEnclosingTypeDeclaration(Node node) { + Node previousNode = null; while (node != null) { if (node instanceof ASTClassOrInterfaceDeclaration) { return ((TypeNode) node).getType(); // anonymous class declaration - } else if (node instanceof ASTAllocationExpression) { + } else if (node instanceof ASTAllocationExpression // is anonymous class declaration + && node.getFirstChildOfType(ASTArrayDimsAndInits.class) == null // array cant anonymous + && !(previousNode instanceof ASTArguments)) { // we might come out of the constructor ASTClassOrInterfaceType typeDecl = node.getFirstChildOfType(ASTClassOrInterfaceType.class); if (typeDecl != null) { return typeDecl.getType(); } } + previousNode = node; node = node.jjtGetParent(); } return null; } + /** + * Get the type def. of the super class of the enclosing type declaration which has the same class + * as the second argument, or if the second argument is null, then anonymous classes are considered + * as well and the first enclosing scope's super class is returned. + * + * @param node The node from which to start searching. + * @param clazz The type of the enclosing class. + * @return The TypeDefinition of the superclass. + */ private JavaTypeDefinition getSuperClassTypeDefinition(Node node, Class clazz) { - for (; node != null; node = node.jjtGetParent()) { - if (node instanceof ASTClassOrInterfaceDeclaration - && (((ASTClassOrInterfaceDeclaration) node).getType() == clazz || clazz == null)) { + Node previousNode = null; + for (; node != null; previousNode = node, node = node.jjtGetParent()) { + if (node instanceof ASTClassOrInterfaceDeclaration // class declaration + // is the class we are looking for or caller requested first class + && (((TypeNode) node).getType() == clazz || clazz == null)) { ASTExtendsList extendsList = node.getFirstChildOfType(ASTExtendsList.class); @@ -788,7 +878,11 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return JavaTypeDefinition.build(Object.class); } // anonymous class declaration - } else if (node instanceof ASTAllocationExpression) { + + } else if (clazz == null // callers requested any class scope + && node instanceof ASTAllocationExpression // is anonymous class decl + && node.getFirstChildOfType(ASTArrayDimsAndInits.class) == null // arrays can't be anonymous + && !(previousNode instanceof ASTArguments)) { // we might come out of the constructor return node.getFirstChildOfType(ASTClassOrInterfaceType.class).getTypeDefinition(); } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java index f88aaab8a4..6a8eaa806b 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/FieldAccess.java @@ -13,12 +13,12 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.SuperClassA; * Note: inherited fields of a nested class shadow outer scope variables * Note: only if they are accessible! * - * TODO: test static field access, array types, anonymous class + * TODO: test static field access, array types, anonymous class (super type access) */ public class FieldAccess extends SuperClassA { public int field; public FieldAccess f; - public static int a; + public static FieldAccess staticF; public void foo(FieldAccess param) { FieldAccess local = null; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java deleted file mode 100644 index 71ee2eb86a..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/dummytypes/StaticFields.java +++ /dev/null @@ -1,14 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.typeresolution.testdata.dummytypes; - -public class StaticFields { - public static StaticFields instanceFields; - public static int staticPrimitive; - public static GenericClass staticGeneric; - - public long primitive; - public GenericClass generic; -} From 46daa878b03ea43d4eccaa7898023dbfb12c83cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Tue, 20 Jun 2017 00:18:21 +0200 Subject: [PATCH 17/20] Java, typeres: fix PR comments, optimize by caching single class JavaTypeDefinitions --- .../java/ast/ASTClassOrInterfaceType.java | 4 ++ .../typeresolution/ClassTypeResolver.java | 58 ++++++------------- .../typedefinition/JavaTypeDefinition.java | 51 +++++++++++----- .../JavaTypeDefinitionBuilder.java | 8 +-- .../typedefinition/TypeDefinition.java | 2 +- 5 files changed, 66 insertions(+), 57 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java index 66120c0b14..f1a1480859 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java @@ -47,4 +47,8 @@ public class ASTClassOrInterfaceType extends AbstractJavaTypeNode { } return false; } + + public boolean isAnonymousClass() { + return jjtGetParent().hasDescendantOfType(ASTClassOrInterfaceBody.class); + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 6437176453..dfe948a4cc 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -28,7 +28,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTArguments; import net.sourceforge.pmd.lang.java.ast.ASTArrayDimsAndInits; import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral; import net.sourceforge.pmd.lang.java.ast.ASTCastExpression; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; @@ -58,7 +57,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTPreDecrementExpression; import net.sourceforge.pmd.lang.java.ast.ASTPreIncrementExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; -import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType; import net.sourceforge.pmd.lang.java.ast.ASTReferenceType; import net.sourceforge.pmd.lang.java.ast.ASTRelationalExpression; @@ -158,6 +156,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { private List 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, JavaTypeDefinition> classToDefaultUpperBounds = new HashMap<>(); + public ClassTypeResolver() { this(ClassTypeResolver.class.getClassLoader()); } @@ -232,8 +236,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { super.visit(node, data); String typeName = node.getImage(); - // branch deals with anonymous classes - if (node.jjtGetParent().hasDescendantOfType(ASTClassOrInterfaceBody.class)) { + + if (node.isAnonymousClass()) { anonymousClassCounter++; AbstractNode parent = node.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class); if (parent == null) { @@ -261,13 +265,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return data; } - @Override - public Object visit(ASTExtendsList node, Object data) { - super.visit(node, data); - - return data; - } - @Override public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { populateType(node, node.getImage()); @@ -288,18 +285,18 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { @Override public Object visit(ASTName node, Object data) { - Class accessingClass = getEnclosingTypeDeclaration(node); + Class accessingClass = getEnclosingTypeDeclaration(node); String[] dotSplitImage = node.getImage().split("\\."); JavaTypeDefinition previousType = getTypeDefinitionOfVariableFromScope(node.getScope(), dotSplitImage[0], accessingClass); - if (node.getNameDeclaration() != null // + if (node.getNameDeclaration() != null && previousType == null // if it's not null, then let other code handle things && node.getNameDeclaration().getNode() instanceof TypeNode) { // Carry over the type from the declaration - Class nodeType = ((TypeNode) node.getNameDeclaration().getNode()).getType(); + 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())) { node.setType(nodeType); @@ -342,7 +339,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param accessingClass The class that is trying to access the field, some Class declared in the current ACU. * @return JavaTypeDefinition of the resolved field or null if it could not be found. */ - private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class accessingClass) { + private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class accessingClass) { while (typeToSearch != null) { try { Field field = typeToSearch.getType().getDeclaredField(fieldImage); @@ -367,7 +364,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param accessingClass The Class (which is defined in the current ACU) that is trying to access the field. * @return Type def. of the field, or null if it could not be resolved. */ - private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class accessingClass) { + private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class accessingClass) { if (accessingClass == null) { return null; } @@ -463,7 +460,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param parameterName The name of the type parameter. * @return The ordinal of the type parameter. */ - private int getTypeParameterOrdinal(Class clazz, String parameterName) { + private int getTypeParameterOrdinal(Class clazz, String parameterName) { TypeVariable[] classTypeParameters = clazz.getTypeParameters(); for (int index = 0; index < classTypeParameters.length; ++index) { @@ -481,7 +478,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param clazz The Class to examine. * @return True if the Class is generic. */ - private boolean isGeneric(Class clazz) { + private boolean isGeneric(Class clazz) { if (clazz != null) { return clazz.getTypeParameters().length != 0; } @@ -489,11 +486,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return false; } - /** - * Contains Class -> JavaTypeDefinitions map for raw Class types. Also helps to avoid infinite recursion. - */ - private Map classToDefaultUpperBounds = new HashMap<>(); - /** * 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. @@ -502,7 +494,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param clazzWithDefBounds The raw Class type. * @return The type def. of the raw Class. */ - private JavaTypeDefinition getDefaultUpperBounds(JavaTypeDefinition context, Class clazzWithDefBounds) { + private JavaTypeDefinition getDefaultUpperBounds(JavaTypeDefinition context, Class clazzWithDefBounds) { JavaTypeDefinitionBuilder typeDef = JavaTypeDefinition.builder(clazzWithDefBounds); // helps avoid infinite recursion with Something<.... E extends Something (<- same raw type)... > @@ -765,7 +757,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { JavaTypeDefinition primaryNodeType = null; AbstractJavaTypeNode previousChild = null; - Class accessingClass = getEnclosingTypeDeclaration(primaryNode); + Class accessingClass = getEnclosingTypeDeclaration(primaryNode); for (int childIndex = 0; childIndex < primaryNode.jjtGetNumChildren(); ++childIndex) { AbstractJavaTypeNode currentChild = (AbstractJavaTypeNode) primaryNode.jjtGetChild(childIndex); @@ -832,7 +824,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param node The node with the enclosing Class declaration. * @return The JavaTypeDefinition of the enclosing Class declaration. */ - private Class getEnclosingTypeDeclaration(Node node) { + private Class getEnclosingTypeDeclaration(Node node) { Node previousNode = null; while (node != null) { if (node instanceof ASTClassOrInterfaceDeclaration) { @@ -863,7 +855,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param clazz The type of the enclosing class. * @return The TypeDefinition of the superclass. */ - private JavaTypeDefinition getSuperClassTypeDefinition(Node node, Class clazz) { + private JavaTypeDefinition getSuperClassTypeDefinition(Node node, Class clazz) { Node previousNode = null; for (; node != null; previousNode = node, node = node.jjtGetParent()) { if (node instanceof ASTClassOrInterfaceDeclaration // class declaration @@ -898,18 +890,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return data; } - @Override - public Object visit(ASTPrimarySuffix node, Object data) { - super.visit(node, data); - return data; - } - - @Override - public Object visit(ASTTypeArguments node, Object data) { - super.visit(node, data); - return data; - } - @Override public Object visit(ASTTypeArgument node, Object data) { super.visit(node, data); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java index 24e8a43d2c..76e3052d13 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinition.java @@ -6,43 +6,68 @@ package net.sourceforge.pmd.lang.java.typeresolution.typedefinition; 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 final List genericArgs; + private final Class clazz; + private List genericArgs; + // contains TypeDefs where only the clazz field is used + private static Map, JavaTypeDefinition> onlyClassTypeDef = new HashMap<>(); - public Class getType() { + public Class getType() { return clazz; } public List getGenericArgs() { + if (genericArgs == null) { + genericArgs = Collections.unmodifiableList(new ArrayList()); + } + return genericArgs; } - JavaTypeDefinition(Class clazz, List genericArgs) { + private JavaTypeDefinition(Class clazz, List genericArgs) { this.clazz = clazz; - if (genericArgs == null) { - genericArgs = new ArrayList<>(); + if (genericArgs != null) { + this.genericArgs = Collections.unmodifiableList(genericArgs); } - - this.genericArgs = Collections.unmodifiableList(genericArgs); } - // builder part of the class - // TODO: possibly add some sort of caching (with like a map or something) - public static JavaTypeDefinition build(Class clazz) { - return new JavaTypeDefinition(clazz, null); + 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; } - public static JavaTypeDefinitionBuilder builder(Class clazz) { + /** + * @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 genericArgs) { + if (genericArgs == null) { + return build(clazz); + } + + return new JavaTypeDefinition(clazz, genericArgs); + } + + public static JavaTypeDefinitionBuilder builder(Class clazz) { return new JavaTypeDefinitionBuilder().setType(clazz); } + public static JavaTypeDefinitionBuilder builder() { return new JavaTypeDefinitionBuilder(); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java index f2f13aa632..b24b4c3b23 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionBuilder.java @@ -9,10 +9,10 @@ import java.util.Collections; import java.util.List; public class JavaTypeDefinitionBuilder { - private Class clazz = null; + private Class clazz = null; private List genericArgs = new ArrayList<>(); - JavaTypeDefinitionBuilder() {} + /* default */ JavaTypeDefinitionBuilder() {} public JavaTypeDefinitionBuilder addTypeArg(JavaTypeDefinition arg) { genericArgs.add(arg); @@ -28,12 +28,12 @@ public class JavaTypeDefinitionBuilder { return this; } - public JavaTypeDefinitionBuilder setType(Class clazz) { + public JavaTypeDefinitionBuilder setType(Class clazz) { this.clazz = clazz; return this; } public JavaTypeDefinition build() { - return new JavaTypeDefinition(clazz, genericArgs); + return JavaTypeDefinition.build(clazz, genericArgs); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java index fb270a392b..74235f25bd 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/TypeDefinition.java @@ -12,7 +12,7 @@ public interface TypeDefinition { * * @return Raw Class type. */ - Class getType(); + Class getType(); /** * Get the list of type arguments for this TypeDefinition. From f1d315ff4c228fbec335cb370690a58f3a459ede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Nagy?= Date: Fri, 23 Jun 2017 19:20:15 +0200 Subject: [PATCH 18/20] Java, typeres: Fix NPE with lambdas --- .../lang/java/typeresolution/ClassTypeResolver.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index dfe948a4cc..b8ff3df64b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -339,7 +339,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param accessingClass The class that is trying to access the field, some Class declared in the current ACU. * @return JavaTypeDefinition of the resolved field or null if it could not be found. */ - private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class accessingClass) { + private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class + accessingClass) { while (typeToSearch != null) { try { Field field = typeToSearch.getType().getDeclaredField(fieldImage); @@ -364,7 +365,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param accessingClass The Class (which is defined in the current ACU) that is trying to access the field. * @return Type def. of the field, or null if it could not be resolved. */ - private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class accessingClass) { + private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class + accessingClass) { if (accessingClass == null) { return null; } @@ -376,6 +378,10 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (entry.getKey().getImage().equals(image)) { ASTType typeNode = entry.getKey().getDeclaratorId().getTypeNode(); + if (typeNode == null) { + return null; + } + if (typeNode.jjtGetChild(0) instanceof ASTReferenceType) { return ((TypeNode) typeNode.jjtGetChild(0)).getTypeDefinition(); } else { // primitive type @@ -851,7 +857,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * as the second argument, or if the second argument is null, then anonymous classes are considered * as well and the first enclosing scope's super class is returned. * - * @param node The node from which to start searching. + * @param node The node from which to start searching. * @param clazz The type of the enclosing class. * @return The TypeDefinition of the superclass. */ From a0f8a106bd03b805f2e9dd757c5914d7212b3729 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Sat, 24 Jun 2017 15:31:48 -0300 Subject: [PATCH 19/20] Add TODO for future self --- .../pmd/lang/java/typeresolution/ClassTypeResolver.java | 1 + 1 file changed, 1 insertion(+) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index b8ff3df64b..eee3bbb85f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -379,6 +379,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { ASTType typeNode = entry.getKey().getDeclaratorId().getTypeNode(); if (typeNode == null) { + // TODO : Type is infered, ie, this is a lambda such as (var) -> var.equals(other) return null; } From aa10a825eb71a780db9082ac8a263d17e6d18b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Sat, 24 Jun 2017 15:32:01 -0300 Subject: [PATCH 20/20] Update changelog, refs #444 --- src/site/markdown/overview/changelog.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index c5ab323759..d4c51c5ea5 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -25,7 +25,7 @@ His progress so far has allowed to properly resolve, in addition to previously s - References to `this` and `super`, even when qualified - References to fields, even when chained (ie: `this.myObject.aField`), and properly handling inheritance / shadowing -Fields using generics are still Work in Progress, but we expect to fully support it soon enough. +Lambda parameter types where these are infered rather than explicit are still not supported. Expect future releases to do so. #### Modified Rules @@ -68,4 +68,5 @@ Fields using generics are still Work in Progress, but we expect to fully support * [#423](https://github.com/pmd/pmd/pull/423): \[java] Add field access type resolution in non-generic cases * [#425](https://github.com/pmd/pmd/pull/425): \[java] False positive with builder pattern in java-design/PreserveStackTrace * [#426](https://github.com/pmd/pmd/pull/426): \[java] UnnecessaryFinalModifier final in private method +* [#444](https://github.com/pmd/pmd/pull/444): \[java] [typeresolution]: add support for generic fields