diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 1c58049681..118097a9ea 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -18,6 +18,8 @@ This is a {{ site.pmd.release_type }} release. * java-bestpractices * [#2149](https://github.com/pmd/pmd/issues/2149): \[java] JUnitAssertionsShouldIncludeMessage - False positive with assertEquals and JUnit5 +* java-performance + * [#2141](https://github.com/pmd/pmd/issues/2141): \[java] StringInstatiation: False negative with String-array access ### API Changes diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/StringInstantiationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/StringInstantiationRule.java index 8f17a26fca..ee7c593953 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/StringInstantiationRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/StringInstantiationRule.java @@ -6,12 +6,17 @@ package net.sourceforge.pmd.lang.java.rule.performance; import java.util.List; +import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTAdditiveExpression; import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; +import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; +import net.sourceforge.pmd.lang.java.ast.ASTArguments; import net.sourceforge.pmd.lang.java.ast.ASTArrayDimsAndInits; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTName; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; +import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.symboltable.TypedNameDeclaration; import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper; @@ -19,6 +24,10 @@ import net.sourceforge.pmd.lang.symboltable.NameDeclaration; public class StringInstantiationRule extends AbstractJavaRule { + public StringInstantiationRule() { + addRuleChainVisit(ASTAllocationExpression.class); + } + @Override public Object visit(ASTAllocationExpression node, Object data) { if (!(node.jjtGetChild(0) instanceof ASTClassOrInterfaceType)) { @@ -29,6 +38,11 @@ public class StringInstantiationRule extends AbstractJavaRule { return data; } + if (isArrayAccess(node)) { + addViolation(data, node); + return data; + } + List exp = node.findDescendantsOfType(ASTExpression.class); if (exp.size() >= 2) { return data; @@ -55,4 +69,20 @@ public class StringInstantiationRule extends AbstractJavaRule { } return data; } + + private boolean isArrayAccess(ASTAllocationExpression node) { + ASTArguments arguments = node.getFirstChildOfType(ASTArguments.class); + if (arguments == null || arguments.getArgumentCount() != 1) { + return false; + } + + Node firstArg = arguments.getFirstChildOfType(ASTArgumentList.class).jjtGetChild(0); + ASTPrimaryExpression primary = firstArg.getFirstChildOfType(ASTPrimaryExpression.class); + if (primary == null || primary.getType() != String.class) { + return false; + } + + ASTPrimarySuffix suffix = primary.getFirstChildOfType(ASTPrimarySuffix.class); + return suffix != null && suffix.isArrayDereference(); + } } 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 b58389e76c..7b9feb997b 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 @@ -69,6 +69,7 @@ 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; @@ -951,8 +952,10 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (currentChild.getType() != null) { - // rollup type from the child: PrimaryPrefix -> PrimaryExpression - primaryNodeType = currentChild.getTypeDefinition(); + // rollup type from the child: PrimaryPrefix/PrimarySuffx -> PrimaryExpression + if (primaryNodeType == null || !primaryNodeType.isArrayType()) { + primaryNodeType = currentChild.getTypeDefinition(); + } // if this expression is a method call, then make sure, PrimaryPrefix has the type // on which the method is executed (type of the target reference) @@ -969,6 +972,13 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { name.setTypeDefinition(primaryNodeType); } } + + // maybe array access? + if (primaryNodeType != null && primaryNodeType.isArrayType()) { + if (currentChild instanceof ASTPrimarySuffix && ((ASTPrimarySuffix) currentChild).isArrayDereference()) { + primaryNodeType = JavaTypeDefinition.forClass(primaryNodeType.getType().getComponentType()); + } + } } else { // avoid falsely passing tests primaryNodeType = null; @@ -1066,6 +1076,14 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return data; } + @Override + public Object visit(ASTPrimarySuffix node, Object data) { + super.visit(node, data); + rollupTypeUnary(node); + + return data; + } + @Override public Object visit(ASTTypeArgument node, Object data) { if (node.jjtGetNumChildren() == 0) { // if type argument is '?' 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 049c873d65..163917e813 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 @@ -77,6 +77,7 @@ import net.sourceforge.pmd.typeresolution.testdata.AbstractReturnTypeUseCase; 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.ArrayAccess; import net.sourceforge.pmd.typeresolution.testdata.ArrayListFound; import net.sourceforge.pmd.typeresolution.testdata.ArrayTypes; import net.sourceforge.pmd.typeresolution.testdata.ArrayVariableDeclaration; @@ -797,6 +798,22 @@ public class ClassTypeResolverTest { assertEquals("All expressions not tested", index, expressions.size()); } + @Test + public void testArrayAccess() throws JaxenException { + List expressions = selectNodes(ArrayAccess.class, ASTExpression.class, "//VariableInitializer/Expression"); + + int index = 1; + // int aElement = a[0]; + assertEquals(int.class, expressions.get(index).getType()); + + index += 2; + // Object bElement = b[0][0]; + assertEquals(Object.class, expressions.get(index).getType()); + + index += 3; + // ArrayAccess cElement = c[0][0][0]; + assertEquals(ArrayAccess.class, expressions.get(index).getType()); + } @Test public void testReferenceType() { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayAccess.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayAccess.java new file mode 100644 index 0000000000..07766e56ce --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayAccess.java @@ -0,0 +1,21 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.typeresolution.testdata; + +public class ArrayAccess { + + @SuppressWarnings("unused") + public void test() { + int[] a = new int[1]; + int aElement = a[0]; + + Object[][] b = new Object[1][0]; + Object bElement = b[0][0]; + + ArrayAccess[][][] c = new ArrayAccess[][][] { new ArrayAccess[1][2] }; + ArrayAccess cElement = c[0][0][0]; + } + +} diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/StringInstantiation.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/StringInstantiation.xml index d905d16cd1..3ad5d3ba96 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/StringInstantiation.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/StringInstantiation.xml @@ -3,10 +3,9 @@ xmlns="http://pmd.sourceforge.net/rule-tests" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd"> + - + new 'new String's 2 + - + new String array 0 + - + using multiple parameter constructor 0 + - + using 4 parameter constructor 0 + - + byte array constructor is ok 0 + - + Method returning new String 1 + - + Not a new String 0 + - + Returns new String(str) 1 + + + #2141 [java] StringInstatiation: False negative with String-array access + 2 + 4,10 + +