diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 826ef9b0af..d63f9618b5 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -27,6 +27,8 @@ TODO - Release blockers - Must implement before this release can be finally fini and pmd-java:typeof). +Fixed bug 1503099: InefficientStringBuffering bug false + +Fixed bug 3109408: String.InefficientStringBuffering false + Fixed bug 3424397: Unable to parse Fixed bug 3530124: pmd: parsing of generic method call with super fails Fixed bug 3496028: pmd-4.2.6: MissingBreakInSwitch fails to report violation diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/strings/InefficientStringBufferingRule.java b/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/strings/InefficientStringBufferingRule.java index 8e78f8f97a..94e1847fcd 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/strings/InefficientStringBufferingRule.java +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/strings/InefficientStringBufferingRule.java @@ -12,14 +12,18 @@ import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; +import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter; import net.sourceforge.pmd.lang.java.ast.ASTLiteral; +import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTName; +import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType; import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; +import net.sourceforge.pmd.lang.java.ast.ASTType; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper; -/* +/** * How this rule works: * find additive expressions: + * check that the addition is between anything other than two literals @@ -38,10 +42,14 @@ public class InefficientStringBufferingRule extends AbstractJavaRule { } int immediateLiterals = 0; + int immediateStringLiterals = 0; List nodes = node.findDescendantsOfType(ASTLiteral.class); for (ASTLiteral literal: nodes) { if (literal.getNthParent(3) instanceof ASTAdditiveExpression) { immediateLiterals++; + if (literal.isStringLiteral()) { + immediateStringLiterals++; + } } if (literal.isIntLiteral() || literal.isFloatLiteral()) { return data; @@ -62,6 +70,18 @@ public class InefficientStringBufferingRule extends AbstractJavaRule { } } } + + // if literal primitive type and not strings variables, then return + boolean stringFound = false; + for (ASTName name: nameNodes) { + if (!isPrimitiveType(name) && isStringType(name)) { + stringFound = true; + break; + } + } + if (!stringFound && immediateStringLiterals == 0) { + return data; + } if (bs.isAllocation()) { for (Iterator iterator = nameNodes.iterator(); iterator.hasNext();) { @@ -82,7 +102,43 @@ public class InefficientStringBufferingRule extends AbstractJavaRule { return data; } - protected static boolean isInStringBufferOperation(Node node, int length, String methodName) { + private boolean isStringType(ASTName name) { + ASTType type = getTypeNode(name); + if (type != null) { + List types = type.findDescendantsOfType(ASTClassOrInterfaceType.class); + if (!types.isEmpty()) { + ASTClassOrInterfaceType typeDeclaration = types.get(0); + if (String.class == typeDeclaration.getType() || "String".equals(typeDeclaration.getImage())) { + return true; + } + } + } + return false; + } + + private boolean isPrimitiveType(ASTName name) { + ASTType type = getTypeNode(name); + if (type != null && !type.findChildrenOfType(ASTPrimitiveType.class).isEmpty()) { + return true; + } + return false; + } + + private ASTType getTypeNode(ASTName name) { + if (name.getNameDeclaration() instanceof VariableNameDeclaration) { + VariableNameDeclaration vnd = (VariableNameDeclaration) name.getNameDeclaration(); + if (vnd.getAccessNodeParent() instanceof ASTLocalVariableDeclaration) { + ASTLocalVariableDeclaration l = (ASTLocalVariableDeclaration)vnd.getAccessNodeParent(); + return l.getTypeNode(); + } else if (vnd.getAccessNodeParent() instanceof ASTFormalParameter) { + ASTFormalParameter p = (ASTFormalParameter) vnd.getAccessNodeParent(); + return p.getTypeNode(); + } + } + return null; + } + + protected static boolean isInStringBufferOperation(Node node, int length, String methodName) { if (!(node.getNthParent(length) instanceof ASTStatementExpression)) { return false; } diff --git a/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/strings/xml/InefficientStringBuffering.xml b/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/strings/xml/InefficientStringBuffering.xml index de2d4cd20d..ca25ef1d66 100644 --- a/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/strings/xml/InefficientStringBuffering.xml +++ b/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/strings/xml/InefficientStringBuffering.xml @@ -332,18 +332,38 @@ public class Foo { } ]]> - +