From 79a84939c578ccb2f052cc0dc7e22ca40322851a Mon Sep 17 00:00:00 2001 From: Romain Pelisse Date: Fri, 13 Jun 2008 20:36:32 +0000 Subject: [PATCH] Applying patch [ 1966013 ] Missing cases in UselessOperationOnImmutable git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/branches/pmd/4.2.x@6197 51baf565-9d33-0410-a72c-fc3788e3496d --- pmd/etc/changelog.txt | 2 + .../basic/xml/UselessOperationOnImmutable.xml | 4 +- pmd/rulesets/basic.xml | 4 +- .../rules/UselessOperationOnImmutable.java | 58 +++++++++++++++---- 4 files changed, 54 insertions(+), 14 deletions(-) diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 14047068d9..4960df39a9 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -2,9 +2,11 @@ Fixes for exclude-pattern Updates to RuleChain to honor RuleSet exclude-pattern +Upgrading UselessOperationOnImmutable to detect more use cases, especially on String Fixed bug 1988829 - Violation reported without source file name (actually a fix to ConsecutiveLiteralAppends) Fixed bug 1989814 - false +: ConsecutiveLiteralAppends + May 20, 2008 - 4.2.2: Fixed false positive in UnusedImports: javadoc comments are parsed to check @see and other tags diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/basic/xml/UselessOperationOnImmutable.xml b/pmd/regress/test/net/sourceforge/pmd/rules/basic/xml/UselessOperationOnImmutable.xml index 5aa79d2895..f176373e60 100644 --- a/pmd/regress/test/net/sourceforge/pmd/rules/basic/xml/UselessOperationOnImmutable.xml +++ b/pmd/regress/test/net/sourceforge/pmd/rules/basic/xml/UselessOperationOnImmutable.xml @@ -10,7 +10,7 @@ useless operation on BigDecimal public class Foo { public void foo() { BigDecimal bd = new BigDecimal(5); - bd.add(new BigDecimal(5)); + bd.divideToIntegralValue(new BigDecimal(5)); } } ]]> @@ -24,7 +24,7 @@ useless operation on BigInteger public class Foo { public void foo() { BigInteger bi = new BigInteger(5); - bi.add(new BigInteger(5)); + bi.modPow(new BigInteger(1), new BigInteger(5)); } } ]]> diff --git a/pmd/rulesets/basic.xml b/pmd/rulesets/basic.xml index cb11101c7a..7ccb666160 100644 --- a/pmd/rulesets/basic.xml +++ b/pmd/rulesets/basic.xml @@ -789,11 +789,11 @@ public class Test { - An operation on an Immutable object (BigDecimal or BigInteger) won't change the object itself. The + An operation on an Immutable object (String, BigDecimal or BigInteger) won't change the object itself. The result of the operation is a new object. Therefore, ignoring the operation result is an error. 3 diff --git a/pmd/src/net/sourceforge/pmd/rules/UselessOperationOnImmutable.java b/pmd/src/net/sourceforge/pmd/rules/UselessOperationOnImmutable.java index 3a1b447d2b..13169d3698 100644 --- a/pmd/src/net/sourceforge/pmd/rules/UselessOperationOnImmutable.java +++ b/pmd/src/net/sourceforge/pmd/rules/UselessOperationOnImmutable.java @@ -1,9 +1,12 @@ package net.sourceforge.pmd.rules; +import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; +import net.sourceforge.pmd.AbstractJavaRule; import net.sourceforge.pmd.AbstractRule; import net.sourceforge.pmd.ast.ASTConditionalExpression; import net.sourceforge.pmd.ast.ASTExpression; @@ -18,21 +21,40 @@ import net.sourceforge.pmd.symboltable.NameOccurrence; import net.sourceforge.pmd.util.CollectionUtil; /** - * An operation on an Immutable object (BigDecimal or BigInteger) won't change + * An operation on an Immutable object (String, BigDecimal or BigInteger) won't change * the object itself. The result of the operation is a new object. Therefore, * ignoring the operation result is an error. */ -public class UselessOperationOnImmutable extends AbstractRule { +public class UselessOperationOnImmutable extends AbstractJavaRule { + /** - * These are the methods which are immutable + * These are the BigDecimal methods which are immutable */ - private static final Set targetMethods = CollectionUtil.asSet(new String[] { ".add", ".multiply", ".divide", ".subtract", ".setScale", ".negate", ".movePointLeft", ".movePointRight", ".pow", ".shiftLeft", ".shiftRight" }); - + private static final Set decMethods = CollectionUtil.asSet(new String[] { ".abs", ".add", ".divide", ".divideToIntegralValue", ".max", ".min", ".movePointLeft", ".movePointRight", ".multiply", ".negate", ".plus", ".pow", ".remainder", ".round", ".scaleByPowerOfTen", ".setScale", ".stripTrailingZeros", ".subtract", ".ulp" }); + + /** + * These are the BigInteger methods which are immutable + */ + private static final Set intMethods = CollectionUtil.asSet(new String[] { ".abs", ".add", ".and", ".andNot", ".clearBit", ".divide", ".flipBit", ".gcd", ".max", ".min", ".mod", ".modInverse", ".modPow", ".multiply", ".negate", ".nextProbablePrine", ".not", ".or", ".pow", ".remainder", ".setBit", ".shiftLeft", ".shiftRight", ".subtract", ".xor" }); + + /** + * These are the String methods which are immutable + */ + private static final Set strMethods = CollectionUtil.asSet(new String[] { ".concat", ".intern", ".replace", ".replaceAll", ".replaceFirst", ".substring", ".toLowerCase", ".toString", ".toUpperCase", ".trim" }); + /** * These are the classes that the rule can apply to */ - private static final Set targetClasses = CollectionUtil.asSet(new String[] { "java.math.BigDecimal", "BigDecimal", "java.math.BigInteger", "BigInteger" }); - + private static final Map> mapClasses = new HashMap>(); + static { + mapClasses.put("java.math.BigDecimal", decMethods); + mapClasses.put("BigDecimal", decMethods); + mapClasses.put("java.math.BigInteger", intMethods); + mapClasses.put("BigInteger", intMethods); + mapClasses.put("java.lang.String", strMethods); + mapClasses.put("String", strMethods); + } + public Object visit(ASTLocalVariableDeclaration node, Object data) { ASTVariableDeclaratorId var = getDeclaration(node); @@ -49,7 +71,7 @@ public class UselessOperationOnImmutable extends AbstractRule { if (!(parentClass.equals(ASTExpression.class) || parentClass.equals(ASTConditionalExpression.class) || hasComparisons(primaryExpression))) { String methodCall = sn.getImage().substring(variableName.length()); - if (targetMethods.contains(methodCall)) { + if (mapClasses.get(getType(node)).contains(methodCall)) { addViolation(data, sn); } } @@ -83,10 +105,26 @@ public class UselessOperationOnImmutable extends AbstractRule { * @return ASTVariableDeclaratorId */ private ASTVariableDeclaratorId getDeclaration(ASTLocalVariableDeclaration node) { - ASTType type = node.getTypeNode(); - if (targetClasses.contains(type.getTypeImage())) { + String type = getType(node); + if (mapClasses.keySet().contains(type)) { return (ASTVariableDeclaratorId) node.jjtGetChild(1).jjtGetChild(0); } return null; } + + /** + * This method returns the name of the node's class. + * + * @param node + * The ASTLocalVariableDeclaration which is a problem + * @return the class name + */ + private static String getType(ASTLocalVariableDeclaration node) { + ASTType type = node.getTypeNode(); + if (type != null) { + return type.getTypeImage(); + } + return null; + } + }