From 05927af5743ceec6305d9eeb93f72fac0127e398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 26 Oct 2017 22:30:14 -0300 Subject: [PATCH] [java] Properly resolve array types - Honor dimensions of arrays - Resolve types for allocations as well as declarations of arrays --- pmd-java/etc/grammar/Java.jjt | 4 +- .../lang/java/ast/ASTArrayDimsAndInits.java | 10 ++++ .../java/ast/ASTClassOrInterfaceType.java | 8 ++++ .../symboltable/VariableNameDeclaration.java | 10 ++++ .../typeresolution/ClassTypeResolver.java | 46 ++++++------------- .../typeresolution/MethodTypeResolution.java | 1 - .../typeresolution/ClassTypeResolverTest.java | 27 +++++++++++ .../typeresolution/testdata/ArrayTypes.java | 15 ++++++ 8 files changed, 85 insertions(+), 36 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayTypes.java diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 28f615cf39..71548d28e7 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -2024,9 +2024,9 @@ void ArrayDimsAndInits() : { LOOKAHEAD(2) - ( LOOKAHEAD(2) (Annotation() {checkForBadTypeAnnotations();})* "[" Expression() "]" )+ ( LOOKAHEAD(2) "[" "]" )* + ( LOOKAHEAD(2) (Annotation() {checkForBadTypeAnnotations();})* "[" Expression() "]" {jjtThis.bumpArrayDepth();})+ ( LOOKAHEAD(2) "[" "]" {jjtThis.bumpArrayDepth();} )* | - ( "[" "]" )+ ArrayInitializer() + ( "[" "]" {jjtThis.bumpArrayDepth();})+ ArrayInitializer() } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java index e3986bc2d1..d40639bc6a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java @@ -6,6 +6,8 @@ package net.sourceforge.pmd.lang.java.ast; public class ASTArrayDimsAndInits extends AbstractJavaNode { + private int dimensions; + public ASTArrayDimsAndInits(int id) { super(id); } @@ -20,4 +22,12 @@ public class ASTArrayDimsAndInits extends AbstractJavaNode { public Object jjtAccept(JavaParserVisitor visitor, Object data) { return visitor.visit(this, data); } + + public void bumpArrayDepth() { + dimensions++; + } + + public int getDimensions() { + return dimensions; + } } 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 057f2d3600..7f68048d53 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 @@ -61,4 +61,12 @@ public class ASTClassOrInterfaceType extends AbstractJavaTypeNode { } return false; } + + public int getArrayDepth() { + Node p = jjtGetParent(); + if (p instanceof ASTReferenceType) { + return ((ASTReferenceType) p).getArrayDepth(); + } + return 0; + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/VariableNameDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/VariableNameDeclaration.java index b2d71756e7..3a58bd4f5c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/VariableNameDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/VariableNameDeclaration.java @@ -36,6 +36,16 @@ public class VariableNameDeclaration extends AbstractNameDeclaration implements return false; } } + + public int getArrayDepth() { + ASTVariableDeclaratorId astVariableDeclaratorId = (ASTVariableDeclaratorId) node; + ASTType typeNode = astVariableDeclaratorId.getTypeNode(); + if (typeNode != null) { + return ((Dimensionable) typeNode.jjtGetParent()).getArrayDepth(); + } else { + return 0; + } + } public boolean isVarargs() { ASTVariableDeclaratorId astVariableDeclaratorId = (ASTVariableDeclaratorId) 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 c5206c56bf..abb23eb4b0 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 @@ -268,7 +268,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } } - populateType(node, typeName, node.isArray()); + populateType(node, typeName, node.getArrayDepth()); ASTTypeArguments typeArguments = node.getFirstChildOfType(ASTTypeArguments.class); @@ -546,14 +546,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (prefix instanceof ASTPrimaryPrefix && prefix.jjtGetParent().jjtGetNumChildren() >= 2) { - ASTArguments args = prefix.jjtGetParent().jjtGetChild(1).getFirstChildOfType(ASTArguments.class); - return args; + return prefix.jjtGetParent().jjtGetChild(1).getFirstChildOfType(ASTArguments.class); } return null; } - /** * 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 @@ -684,7 +682,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } String name = node.getNameDeclaration().getTypeImage(); if (name != null) { - populateType(node, name, node.getNameDeclaration().isArray()); + populateType(node, name, node.getNameDeclaration().getArrayDepth()); } return super.visit(node, data); } @@ -1137,8 +1135,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { public Object visit(ASTAllocationExpression node, Object data) { super.visit(node, data); - if (node.jjtGetNumChildren() >= 2 && node.jjtGetChild(1) instanceof ASTArrayDimsAndInits - || node.jjtGetNumChildren() >= 3 && node.jjtGetChild(2) instanceof ASTArrayDimsAndInits) { + if (node.hasDescendantOfType(ASTArrayDimsAndInits.class)) { // // Classes for Array types cannot be found directly using // reflection. @@ -1147,28 +1144,11 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // dimensionality, and then ask for the type from the instance. OMFG // that's ugly. // - - // TODO Need to create utility method to allow array type creation - // which will use - // caching to avoid repeated object creation. - // TODO Modify Parser to tell us array dimensions count. - // TODO Parser seems to do some work to handle arrays in certain - // case already. - // Examine those to figure out what's going on, make sure _all_ - // array scenarios - // are ultimately covered. Appears to use a Dimensionable interface - // to handle - // only a part of the APIs (not bump), but is implemented several - // times, so - // look at refactoring to eliminate duplication. Dimensionable is - // also used - // on AccessNodes for some scenarios, need to account for that. - // Might be - // missing some TypeNode candidates we can add to the AST and have - // to deal - // with here (e.g. FormalParameter)? Plus some existing usages may - // be - // incorrect. + final Class arrayType = ((TypeNode) node.jjtGetChild(0)).getType(); + if (arrayType != null) { + final ASTArrayDimsAndInits dims = node.getFirstChildOfType(ASTArrayDimsAndInits.class); + node.setType(Array.newInstance(arrayType, (int[]) Array.newInstance(int.class, dims.getDimensions())).getClass()); + } } else { rollupTypeUnary(node); } @@ -1275,10 +1255,10 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } private void populateType(TypeNode node, String className) { - populateType(node, className, false); + populateType(node, className, 0); } - private void populateType(TypeNode node, String className, boolean isArray) { + private void populateType(TypeNode node, String className, int arrayDimens) { String qualifiedName = className; Class myType = PRIMITIVE_TYPES.get(className); @@ -1330,8 +1310,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { node.setTypeDefinition(parameter.getTypeDefinition()); } } else { - if (isArray) { - myType = Array.newInstance(myType, 0).getClass(); + if (arrayDimens > 0) { + myType = Array.newInstance(myType, (int[]) Array.newInstance(int.class, arrayDimens)).getClass(); } node.setType(myType); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java index faa593bf2b..4393a24124 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java @@ -622,7 +622,6 @@ public final class MethodTypeResolution { return false; } - public static boolean isSubtypeable(JavaTypeDefinition parameter, ASTExpression argument) { if (argument.getTypeDefinition() == null) { LOG.log(Level.FINE, "No type information for node {0}", argument.toString()); 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 8cf55f6788..608267ca84 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 @@ -71,6 +71,7 @@ import net.sourceforge.pmd.typeresolution.testdata.AnonymousClassFromInterface; import net.sourceforge.pmd.typeresolution.testdata.AnonymousInnerClass; import net.sourceforge.pmd.typeresolution.testdata.AnoymousExtendingObject; import net.sourceforge.pmd.typeresolution.testdata.ArrayListFound; +import net.sourceforge.pmd.typeresolution.testdata.ArrayTypes; import net.sourceforge.pmd.typeresolution.testdata.DefaultJavaLangImport; import net.sourceforge.pmd.typeresolution.testdata.EnumWithAnonymousInnerClass; import net.sourceforge.pmd.typeresolution.testdata.ExtraTopLevelClass; @@ -748,6 +749,32 @@ public class ClassTypeResolverTest { assertEquals("All expressions not tested", index, expressions.size()); } + @Test + public void testArrayTypes() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(ArrayTypes.class); + + List expressions = convertList( + acu.findChildNodesWithXPath("//VariableDeclarator"), + AbstractJavaTypeNode.class); + + int index = 0; + + // int[] a = new int[1]; + AbstractJavaTypeNode typeNode = expressions.get(index++); + assertEquals(int[].class, typeNode.getType()); + for (AbstractJavaTypeNode n : typeNode.findDescendantsOfType(AbstractJavaTypeNode.class)) { + assertEquals(int[].class, n.getType()); + } + + // Object[][] b = new Object[1][0]; + assertEquals(Object[][].class, expressions.get(index++).getType()); + + // ArrayTypes[][][] c = new ArrayTypes[][][] { new ArrayTypes[1][2] }; + assertEquals(ArrayTypes[][][].class, expressions.get(index++).getType()); + + // Make sure we got them all + assertEquals("All expressions not tested", index, expressions.size()); + } @Test public void testFieldAccess() throws JaxenException { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayTypes.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayTypes.java new file mode 100644 index 0000000000..15dcda6bbf --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayTypes.java @@ -0,0 +1,15 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.typeresolution.testdata; + +public class ArrayTypes { + + @SuppressWarnings("unused") + public void test() { + int[] a = new int[1]; + Object[][] b = new Object[1][0]; + ArrayTypes[][][] c = new ArrayTypes[][][] { new ArrayTypes[1][2] }; + } +}