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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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 2f0e12da6cffb876c35c3f7f1b242c0c84242799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 19 Jun 2017 11:07:12 -0300 Subject: [PATCH 17/36] [java] Fix false positive in UnusedImport - When referencing static inner members of imports, false positives would be reported. - Fixes #348 --- .../lang/java/rule/imports/UnusedImportsRule.java | 9 ++++++++- .../rule/typeresolution/xml/UnusedImports.xml | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnusedImportsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnusedImportsRule.java index ceb81ef077..419f8962c8 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnusedImportsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnusedImportsRule.java @@ -86,7 +86,14 @@ public class UnusedImportsRule extends AbstractJavaRule { if (s != null) { String[] params = s.split("\\s*,\\s*"); for (String param : params) { - imports.remove(new ImportWrapper(param, param, new DummyJavaNode(-1))); + final int firstDot = param.indexOf('.'); + final String expectedImportName; + if (firstDot == -1) { + expectedImportName = param; + } else { + expectedImportName = param.substring(0, firstDot); + } + imports.remove(new ImportWrapper(param, expectedImportName, new DummyJavaNode(-1))); } } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/UnusedImports.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/UnusedImports.xml index 2deb2d9a11..2561d9acb4 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/UnusedImports.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/UnusedImports.xml @@ -261,6 +261,21 @@ public interface Interface { */ void doSomething(); +} + ]]> + + + #348 False Positive UnusedImports with javadoc for public static inner classes of imports + 0 + From 6ed0336971fe0108bf4d69ff8c0ced2769f48538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 19 Jun 2017 11:19:52 -0300 Subject: [PATCH 18/36] Update changelog, refs #449 --- src/site/markdown/overview/changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index fd12d4dc9b..e54f761ecf 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -57,6 +57,8 @@ Fields using generics are still Work in Progress, but we expect to fully support * [#397](https://github.com/pmd/pmd/issues/397): \[java] ConstructorCallsOverridableMethodRule: false positive for method called from lambda expression * [#410](https://github.com/pmd/pmd/issues/410): \[java] ImmutableField: False positive with lombok * [#422](https://github.com/pmd/pmd/issues/422): \[java] PreserveStackTraceRule: false positive when using builder pattern +* java-imports: + * [#348]((https://github.com/pmd/pmd/issues/348): \[java] imports/UnusedImport rule not considering static inner classes of imports * java-junit * [#428](https://github.com/pmd/pmd/issues/428): \[java] PMD requires public modifier on JUnit 5 test * java-unnecessary 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 19/36] 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 e1f33504b97aed90a1b606ffaed512a99c86f755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 16:45:32 -0300 Subject: [PATCH 20/36] [jsp] Update grammar to accept boolean attributes --- pmd-jsp/etc/grammar/JspParser.jjt | 8 ++++++-- .../net/sourceforge/pmd/lang/jsp/JspParserTest.java | 10 ++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/pmd-jsp/etc/grammar/JspParser.jjt b/pmd-jsp/etc/grammar/JspParser.jjt index 13863d8247..7ea94dcf5f 100644 --- a/pmd-jsp/etc/grammar/JspParser.jjt +++ b/pmd-jsp/etc/grammar/JspParser.jjt @@ -1,4 +1,8 @@ -/* +/* + * Allow boolean attributes + * + * Juan Martín Sotuyo Dodero 06/2017 + *==================================================================== * Added capability for Tracking Tokens. * * Amit Kumar Prasad 10/2015 @@ -580,7 +584,7 @@ void Attribute() : ( AttributeValue() - ) + )? } /** diff --git a/pmd-jsp/src/test/java/net/sourceforge/pmd/lang/jsp/JspParserTest.java b/pmd-jsp/src/test/java/net/sourceforge/pmd/lang/jsp/JspParserTest.java index 4d7d221f3b..b05018c207 100644 --- a/pmd-jsp/src/test/java/net/sourceforge/pmd/lang/jsp/JspParserTest.java +++ b/pmd-jsp/src/test/java/net/sourceforge/pmd/lang/jsp/JspParserTest.java @@ -29,6 +29,16 @@ public class JspParserTest { "$129.00"); Assert.assertNotNull(node); } + + /** + * Verifies bug #311 Jsp parser fails on boolean attribute + */ + @Test + public void testParseBooleanAttribute() { + Node node = parse( + ""); + Assert.assertNotNull(node); + } private Node parse(String code) { LanguageVersionHandler jspLang = LanguageRegistry.getLanguage(JspLanguageModule.NAME).getDefaultVersion() From 263a7d57aba45de58f76b29c595c209124a7901b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 16:46:38 -0300 Subject: [PATCH 21/36] Update changelog, refs #311 --- src/site/markdown/overview/changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index e54f761ecf..98359d46f9 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -63,6 +63,8 @@ Fields using generics are still Work in Progress, but we expect to fully support * [#428](https://github.com/pmd/pmd/issues/428): \[java] PMD requires public modifier on JUnit 5 test * java-unnecessary * [#421](https://github.com/pmd/pmd/issues/421): \[java] UnnecessaryFinalModifier final in private method +* jsp + * [#311](https://github.com/pmd/pmd/issues/311): \[jsp] Parse error on HTML boolean attribute ### API Changes From cc4294b39925163ca0ac45ae3d4c7c3bf67641af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 17:04:41 -0300 Subject: [PATCH 22/36] [java] Simplify handling of SLF4J log arguments - Fixes #365 --- .../InvalidSlf4jMessageFormatRule.java | 35 ++++++------------- .../xml/InvalidSlf4jMessageFormat.xml | 20 +++++++++++ 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/InvalidSlf4jMessageFormatRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/InvalidSlf4jMessageFormatRule.java index b99abf17ff..4eb61ffcdd 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/InvalidSlf4jMessageFormatRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/InvalidSlf4jMessageFormatRule.java @@ -70,23 +70,9 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule { // find the arguments final List argumentList = parentNode.getFirstChildOfType(ASTPrimarySuffix.class) .getFirstDescendantOfType(ASTArgumentList.class).findChildrenOfType(ASTExpression.class); - final List params = new ArrayList(argumentList.size()); - for (final ASTExpression astExpression : argumentList) { - ASTPrimaryExpression primaryExpression = astExpression.getFirstChildOfType(ASTPrimaryExpression.class); - if (primaryExpression != null) { - params.add(primaryExpression); - } - } - - if (params.isEmpty()) { - // no params we could analyze - return super.visit(node, data); - } - - final ASTPrimaryExpression messageParam = params.get(0); // remove the message parameter - params.remove(0); + final ASTPrimaryExpression messageParam = argumentList.remove(0).getFirstDescendantOfType(ASTPrimaryExpression.class); final int expectedArguments = expectedArguments(messageParam); if (expectedArguments == 0) { @@ -97,14 +83,14 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule { // Remove throwable param, since it is shown separately. // But only, if it is not used as a placeholder argument - if (params.size() > expectedArguments) { - removeThrowableParam(params); + if (argumentList.size() > expectedArguments) { + removeThrowableParam(argumentList); } - if (params.size() < expectedArguments) { - addViolationWithMessage(data, node, "Missing arguments," + getExpectedMessage(params, expectedArguments)); - } else if (params.size() > expectedArguments) { - addViolationWithMessage(data, node, "Too many arguments," + getExpectedMessage(params, expectedArguments)); + if (argumentList.size() < expectedArguments) { + addViolationWithMessage(data, node, "Missing arguments," + getExpectedMessage(argumentList, expectedArguments)); + } else if (argumentList.size() > expectedArguments) { + addViolationWithMessage(data, node, "Too many arguments," + getExpectedMessage(argumentList, expectedArguments)); } return super.visit(node, data); @@ -146,21 +132,20 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule { return false; } - private void removeThrowableParam(final List params) { + private void removeThrowableParam(final List params) { // Throwable parameters are the last one in the list, if any. if (params.isEmpty()) { return; } int lastIndex = params.size() - 1; - ASTPrimaryExpression last = params.get(lastIndex); + ASTPrimaryExpression last = params.get(lastIndex).getFirstDescendantOfType(ASTPrimaryExpression.class); if (isNewThrowable(last) || hasTypeThrowable(last) || isReferencingThrowable(last)) { params.remove(lastIndex); - return; } } - private String getExpectedMessage(final List params, final int expectedArguments) { + private String getExpectedMessage(final List params, final int expectedArguments) { return " expected " + expectedArguments + (expectedArguments > 1 ? " arguments " : " argument ") + "but have " + params.size(); } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/InvalidSlf4jMessageFormat.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/InvalidSlf4jMessageFormat.xml index ac0f174986..38f74f2666 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/InvalidSlf4jMessageFormat.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/InvalidSlf4jMessageFormat.xml @@ -198,6 +198,26 @@ public class TestBug1551 { return "message"; } +} + ]]> + + + + #365 [java] InvalidSlf4jMessageFormat: false positive with pre-incremented variable + 0 + From 60359ad1cc74bc43e0169ae0b6ae0d380ed67815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 17:07:26 -0300 Subject: [PATCH 23/36] Update changelog, refs #365 --- src/site/markdown/overview/changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index e54f761ecf..4598374faf 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -61,6 +61,8 @@ Fields using generics are still Work in Progress, but we expect to fully support * [#348]((https://github.com/pmd/pmd/issues/348): \[java] imports/UnusedImport rule not considering static inner classes of imports * java-junit * [#428](https://github.com/pmd/pmd/issues/428): \[java] PMD requires public modifier on JUnit 5 test +* java-logging: + * [#365](https://github.com/pmd/pmd/issues/365): \[java] InvalidSlf4jMessageFormat does not handle inline incrementation of arguments * java-unnecessary * [#421](https://github.com/pmd/pmd/issues/421): \[java] UnnecessaryFinalModifier final in private method From 878f73158a76fed252d1d048767b9cbeea28857d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 17:20:11 -0300 Subject: [PATCH 24/36] [java] Allow try-with-resources to be empty * Fixes #432 --- pmd-java/src/main/resources/rulesets/java/empty.xml | 2 +- .../pmd/lang/java/rule/empty/xml/EmptyTryBlock.xml | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/resources/rulesets/java/empty.xml b/pmd-java/src/main/resources/rulesets/java/empty.xml index 7141ed3fc2..aa2765ea61 100644 --- a/pmd-java/src/main/resources/rulesets/java/empty.xml +++ b/pmd-java/src/main/resources/rulesets/java/empty.xml @@ -129,7 +129,7 @@ Avoid empty try blocks - what's the point? diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/empty/xml/EmptyTryBlock.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/empty/xml/EmptyTryBlock.xml index c671f1d1c0..8e4a64079e 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/empty/xml/EmptyTryBlock.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/empty/xml/EmptyTryBlock.xml @@ -46,6 +46,19 @@ public class EmptyTryBlock3 { int x = 5; } } +} + ]]> + + + #432 false positive for empty try-with-resource + 0 + target.request(mediaTypes).delete(), DELETE, new ExpectedResponse(status, required))) { + // false positive + } + } } ]]> From 7ffa76e3e232f38a2dfc645f0d364b186ce48ef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 17:27:26 -0300 Subject: [PATCH 25/36] Update changelog, refs #432 --- src/site/markdown/overview/changelog.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index e54f761ecf..368b950e13 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -39,6 +39,8 @@ Fields using generics are still Work in Progress, but we expect to fully support * The ruleset java-junit now properly detects JUnit5, and rules are being adapted to the changes on it's API. This support is, however, still incomplete. Let us know of any uses we are still missing on the [issue tracker](https://github.com/pmd/pmd/issues) +* The Java rule `EmptyTryBlock` (ruleset java-empty) now allows empty blocks when usin try-with-resources. + ### Fixed Issues * General @@ -57,8 +59,10 @@ Fields using generics are still Work in Progress, but we expect to fully support * [#397](https://github.com/pmd/pmd/issues/397): \[java] ConstructorCallsOverridableMethodRule: false positive for method called from lambda expression * [#410](https://github.com/pmd/pmd/issues/410): \[java] ImmutableField: False positive with lombok * [#422](https://github.com/pmd/pmd/issues/422): \[java] PreserveStackTraceRule: false positive when using builder pattern +* java-empty + * [#432](https://github.com/pmd/pmd/issues/432): \[java] EmptyTryBlock: false positive for empty try-with-resource * java-imports: - * [#348]((https://github.com/pmd/pmd/issues/348): \[java] imports/UnusedImport rule not considering static inner classes of imports + * [#348](https://github.com/pmd/pmd/issues/348): \[java] imports/UnusedImport rule not considering static inner classes of imports * java-junit * [#428](https://github.com/pmd/pmd/issues/428): \[java] PMD requires public modifier on JUnit 5 test * java-unnecessary From 15716ffd932fe0cf95c91ebcd92a4be957a7fee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 17:50:29 -0300 Subject: [PATCH 26/36] [java] SignatureDeclareThrowsException ignores @Override - Avoid reporting on @Overrides, since we have no control of the interface. - The original definition is still reported if it's part of our application's code. - Fixes #350 --- .../SignatureDeclareThrowsExceptionRule.java | 10 ++++++++++ .../rules/SignatureDeclareThrowsException.java | 10 ++++++++++ .../xml/SignatureDeclareThrowsException.xml | 12 ++++++++++++ .../xml/SignatureDeclareThrowsException.xml | 12 ++++++++++++ 4 files changed, 44 insertions(+) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strictexception/SignatureDeclareThrowsExceptionRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strictexception/SignatureDeclareThrowsExceptionRule.java index 0c36c5a87f..8b22268cce 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strictexception/SignatureDeclareThrowsExceptionRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strictexception/SignatureDeclareThrowsExceptionRule.java @@ -8,6 +8,7 @@ import java.util.Collections; import java.util.List; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; @@ -51,6 +52,15 @@ public class SignatureDeclareThrowsExceptionRule extends AbstractJavaRule { if (methodDeclaration.getMethodName().startsWith("test")) { return super.visit(methodDeclaration, o); } + + // Ignore overridden methods, the issue should be marked on the method definition + final List methodAnnotations = methodDeclaration.jjtGetParent().findChildrenOfType(ASTAnnotation.class); + for (final ASTAnnotation annotation : methodAnnotations) { + final ASTName annotationName = annotation.getFirstDescendantOfType(ASTName.class); + if (annotationName.hasImageEqualTo("Override") || annotationName.hasImageEqualTo("java.lang.Override")) { + return super.visit(methodDeclaration, o); + } + } List exceptionList = Collections.emptyList(); ASTNameList nameList = methodDeclaration.getFirstChildOfType(ASTNameList.class); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/SignatureDeclareThrowsException.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/SignatureDeclareThrowsException.java index 7309ea1515..d041ff6cd6 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/SignatureDeclareThrowsException.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/SignatureDeclareThrowsException.java @@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang.java.typeresolution.rules; import java.util.List; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; @@ -113,6 +114,15 @@ public class SignatureDeclareThrowsException extends AbstractJavaRule { if (junitImported && isAllowedMethod(methodDeclaration)) { return super.visit(methodDeclaration, o); } + + // Ignore overridden methods, the issue should be marked on the method definition + final List methodAnnotations = methodDeclaration.jjtGetParent().findChildrenOfType(ASTAnnotation.class); + for (final ASTAnnotation annotation : methodAnnotations) { + final ASTName annotationName = annotation.getFirstDescendantOfType(ASTName.class); + if (annotationName.hasImageEqualTo("Override") || annotationName.hasImageEqualTo("java.lang.Override")) { + return super.visit(methodDeclaration, o); + } + } checkExceptions(methodDeclaration, o); diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/SignatureDeclareThrowsException.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/SignatureDeclareThrowsException.xml index f646a49cb1..1b12f2deab 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/SignatureDeclareThrowsException.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/SignatureDeclareThrowsException.xml @@ -103,4 +103,16 @@ public class BugSignature { public class UnmodifiableList implements @Readonly List<@Readonly T> {} ]]> + + + #350 allow throws exception when overriding a method defined elsewhere + 0 + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/SignatureDeclareThrowsException.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/SignatureDeclareThrowsException.xml index 76780a567e..9c042f1cab 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/SignatureDeclareThrowsException.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/SignatureDeclareThrowsException.xml @@ -151,4 +151,16 @@ public class Foo { public class UnmodifiableList implements @Readonly List<@Readonly T> {} ]]> + + + #350 allow throws exception when overriding a method defined elsewhere + 0 + + From 52814a9ba97433883c8f187c8877dea23f2ab3ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 17:53:31 -0300 Subject: [PATCH 27/36] Update changelog, refs #350 --- src/site/markdown/overview/changelog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index e54f761ecf..6ad23d7b82 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -61,6 +61,10 @@ Fields using generics are still Work in Progress, but we expect to fully support * [#348]((https://github.com/pmd/pmd/issues/348): \[java] imports/UnusedImport rule not considering static inner classes of imports * java-junit * [#428](https://github.com/pmd/pmd/issues/428): \[java] PMD requires public modifier on JUnit 5 test +* java-strictexceptions + * [#350](https://github.com/pmd/pmd/issues/350): \[java] Throwing Exception in method signature is fine if the method is overriding or implementing something +* java-typeresolution + * [#350](https://github.com/pmd/pmd/issues/350): \[java] Throwing Exception in method signature is fine if the method is overriding or implementing something * java-unnecessary * [#421](https://github.com/pmd/pmd/issues/421): \[java] UnnecessaryFinalModifier final in private method From eda54ae86aabfb2470270159def6c35d6086e9fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 23:52:05 -0300 Subject: [PATCH 28/36] [java] EmptyCatchBlock may ignore blocks based on exception name - Allow the developer to setup a regular expression for exception names to be ignored by the rule. - Fixes #413 --- .../src/main/resources/rulesets/java/empty.xml | 2 ++ .../java/rule/empty/xml/EmptyCatchBlock.xml | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/pmd-java/src/main/resources/rulesets/java/empty.xml b/pmd-java/src/main/resources/rulesets/java/empty.xml index 7141ed3fc2..a75e50b053 100644 --- a/pmd-java/src/main/resources/rulesets/java/empty.xml +++ b/pmd-java/src/main/resources/rulesets/java/empty.xml @@ -30,10 +30,12 @@ or reported. [FormalParameter/Type/ReferenceType /ClassOrInterfaceType[@Image != 'InterruptedException' and @Image != 'CloneNotSupportedException'] ] + [FormalParameter/VariableDeclaratorId[not(matches(@Image, $allowExceptionNameRegex))]] ]]> + + + + ^(ignored|expected)$ + 0 + + From dac5c8b04ab2e927f7289484c51449802078ff74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 23:59:51 -0300 Subject: [PATCH 29/36] Update changelog, refs #413 --- src/site/markdown/overview/changelog.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index e54f761ecf..ca59dca775 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -39,6 +39,11 @@ Fields using generics are still Work in Progress, but we expect to fully support * The ruleset java-junit now properly detects JUnit5, and rules are being adapted to the changes on it's API. This support is, however, still incomplete. Let us know of any uses we are still missing on the [issue tracker](https://github.com/pmd/pmd/issues) +* The Java rule `EmptyCatchBlock` (ruleset java-empty) now exposes a new property called `allowExceptionNameRegex`. + This allow to setup a regular expression for names of exceptions you wish to ignore for this rule. For instance, + setting it to `^(ignored|expected)$` would ignore all empty catch blocks where the catched exception is named + either `ignored` or `expected`. The default ignores no exceptions, being backwards compatible. + ### Fixed Issues * General @@ -57,7 +62,9 @@ Fields using generics are still Work in Progress, but we expect to fully support * [#397](https://github.com/pmd/pmd/issues/397): \[java] ConstructorCallsOverridableMethodRule: false positive for method called from lambda expression * [#410](https://github.com/pmd/pmd/issues/410): \[java] ImmutableField: False positive with lombok * [#422](https://github.com/pmd/pmd/issues/422): \[java] PreserveStackTraceRule: false positive when using builder pattern -* java-imports: +* java-empty + * [#413](https://github.com/pmd/pmd/issues/413): \[java] EmptyCatchBlock don't fail when exception is named ignore / expected +* java-imports * [#348]((https://github.com/pmd/pmd/issues/348): \[java] imports/UnusedImport rule not considering static inner classes of imports * java-junit * [#428](https://github.com/pmd/pmd/issues/428): \[java] PMD requires public modifier on JUnit 5 test 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 30/36] 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 556cc6da90522b56da23b5076c3092f3d7007446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 23 Jun 2017 15:45:58 -0300 Subject: [PATCH 31/36] [java] Fix NPE in JUnitTestsShouldIncludeAssertRule - Don't assume all anotations are marker annotations - Fixes #465 --- .../JUnitTestsShouldIncludeAssertRule.java | 3 +-- .../xml/JUnitTestsShouldIncludeAssert.xml | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/JUnitTestsShouldIncludeAssertRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/JUnitTestsShouldIncludeAssertRule.java index eb20a28299..fd9678d0ac 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/JUnitTestsShouldIncludeAssertRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/JUnitTestsShouldIncludeAssertRule.java @@ -9,7 +9,6 @@ import java.util.List; import java.util.Map; import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation; @@ -81,7 +80,7 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule { for (NameDeclaration decl : decls.keySet()) { Node parent = decl.getNode().jjtGetParent().jjtGetParent().jjtGetParent(); - if (parent.hasDescendantOfType(ASTAnnotation.class) + if (parent.hasDescendantOfType(ASTMarkerAnnotation.class) && parent.getFirstChildOfType(ASTFieldDeclaration.class) != null) { String annot = parent.getFirstDescendantOfType(ASTMarkerAnnotation.class).jjtGetChild(0).getImage(); if (!"Rule".equals(annot) && !"org.junit.Rule".equals(annot)) { diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/junit/xml/JUnitTestsShouldIncludeAssert.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/junit/xml/JUnitTestsShouldIncludeAssert.xml index 61f5047edc..9014686439 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/junit/xml/JUnitTestsShouldIncludeAssert.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/junit/xml/JUnitTestsShouldIncludeAssert.xml @@ -392,6 +392,26 @@ public class FooTest { Mockito.verify(bar, Mockito.times(1)).actuallyDoTask(); } +}]]> + + + #465 NullPointerException when dealing with @SuppressWarnings + 0 + From 85092dba04c5588b78a0b911f216202cc267f347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 23 Jun 2017 15:48:39 -0300 Subject: [PATCH 32/36] Update changelog --- src/site/markdown/overview/changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index fe4c977dcd..1a9f281b10 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -71,6 +71,7 @@ Fields using generics are still Work in Progress, but we expect to fully support * [#348](https://github.com/pmd/pmd/issues/348): \[java] imports/UnusedImport rule not considering static inner classes of imports * java-junit * [#428](https://github.com/pmd/pmd/issues/428): \[java] PMD requires public modifier on JUnit 5 test + * [#465](https://github.com/pmd/pmd/issues/465): \[java] NullPointerException in JUnitTestsShouldIncludeAssertRule * java-logging: * [#365](https://github.com/pmd/pmd/issues/365): \[java] InvalidSlf4jMessageFormat does not handle inline incrementation of arguments * java-strictexceptions From 6fff60bf1d3f8075aa1a6396af227acbcf893f13 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 24 Jun 2017 10:52:55 +0200 Subject: [PATCH 33/36] Update changelog, refs #451 --- src/site/markdown/overview/changelog.md | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index fe4c977dcd..ffc9dae0f3 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -10,6 +10,7 @@ This is a minor release. * [New and noteworthy](#New_and_noteworthy) * [Java Type Resolution](#Java_Type_Resolution) + * [Metrics Framework](#Metrics_Framework) * [Modified Rules](#Modified_Rules) * [Fixed Issues](#Fixed_Issues) * [API Changes](#API_Changes) @@ -17,9 +18,9 @@ This is a minor release. ### New and noteworthy -### Java Type Resolution +#### Java Type Resolution -As part of Google Summer of Code 2017, [Bendeguz Nagy](https://github.com/WinterGrascph) has been working on completing type resolution for Java. +As part of Google Summer of Code 2017, [Bendegúz Nagy](https://github.com/WinterGrascph) has been working on completing type resolution for Java. His progress so far has allowed to properly resolve, in addition to previously supported statements: - References to `this` and `super`, even when qualified @@ -27,6 +28,22 @@ His progress so far has allowed to properly resolve, in addition to previously s Fields using generics are still Work in Progress, but we expect to fully support it soon enough. + +#### Metrics Framework + +As part of Google Summer of Code 2017, [Clément Fournier](https://github.com/oowekyala) has been working on +a new metrics framework for object-oriented metrics. + +The basic groundwork has been done already and with this release, including a first rule based on the +metrics framework as a proof-of-concept: The rule *CyclomaticComplexity*, currently in the temporary +ruleset *java-metrics*, uses the Cyclomatic Complexity metric to find overly complex code. +This rule will eventually replace the existing three *CyclomaticComplexity* rules that are currently +defined in the *java-codesize* ruleset (see also [issue #445](https://github.com/pmd/pmd/issues/445)). + +Since this work is still in progress, the metrics API (package `net.sourceforge.pmd.lang.java.oom`) +is not finalized yet and is expected to change. + + #### Modified Rules * The Java rule `UnnecessaryFinalModifier` (ruleset java-unnecessary) now also reports on private methods marked as `final`. @@ -96,4 +113,5 @@ Fields using generics are still Work in Progress, but we expect to fully support * [#436](https://github.com/pmd/pmd/pull/436): \[java] Metrics framework tests and various improvements * [#440](https://github.com/pmd/pmd/pull/440): \[core] Created ruleset schema 3.0.0 (to use metrics) * [#443](https://github.com/pmd/pmd/pull/443): \[java] Optimize typeresolution, by skipping package and import declarations in visit(ASTName) +* [#451](https://github.com/pmd/pmd/pull/451): \[java] Metrics framework: first metrics + first rule From 0289fef501c83fde3bfb4b31914989875c3b1f41 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 24 Jun 2017 10:59:05 +0200 Subject: [PATCH 34/36] Deprecate the three rules *CyclomaticComplexity (java-codesize) Refs #445 --- pmd-java/src/main/resources/rulesets/java/codesize.xml | 9 ++++++--- src/site/markdown/overview/changelog.md | 6 ++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/pmd-java/src/main/resources/rulesets/java/codesize.xml b/pmd-java/src/main/resources/rulesets/java/codesize.xml index ba41cd6477..6f109353bf 100644 --- a/pmd-java/src/main/resources/rulesets/java/codesize.xml +++ b/pmd-java/src/main/resources/rulesets/java/codesize.xml @@ -148,7 +148,8 @@ public class Foo { since="1.03" message = "The {0} ''{1}'' has a Cyclomatic Complexity of {2}." class="net.sourceforge.pmd.lang.java.rule.codesize.CyclomaticComplexityRule" - externalInfoUrl="${pmd.website.baseurl}/rules/java/codesize.html#CyclomaticComplexity"> + externalInfoUrl="${pmd.website.baseurl}/rules/java/codesize.html#CyclomaticComplexity" + deprecated="true"> + externalInfoUrl="${pmd.website.baseurl}/rules/java/codesize.html#StdCyclomaticComplexity" + deprecated="true"> + externalInfoUrl="${pmd.website.baseurl}/rules/java/codesize.html#ModifiedCyclomaticComplexity" + deprecated="true"> Date: Sat, 24 Jun 2017 15:31:48 -0300 Subject: [PATCH 35/36] 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 36/36] 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