Fixed bugs in AvoidDuplicateLiterals - now it ignores small duplicate literals, its rule violation message is more helpful, and it catches more cases.

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@2816 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Tom Copeland
2004-07-19 18:32:08 +00:00
parent 8c8d0ff0aa
commit 7b5b1f2aa9
5 changed files with 31 additions and 66 deletions

View File

@ -2,7 +2,8 @@
Moved development environment to Maven 1.0.
Moved development environment to Ant 1.6.2. This is nice because using the new JUnit task attribute "forkmode='perBatch'" cuts test runtime from 90 seconds to 7 seconds. Sweet.
Applied patch in RFE 992576 - Enhancements to VariableNamingConventionsRule
Fixed bug in SimplifyBooleanExpressions - now it catches more cases.
Fixed bug in SimplifyBooleanExpressions - now it catches more cases.
Fixed bugs in AvoidDuplicateLiterals - now it ignores small duplicate literals, its rule violation message is more helpful, and it catches more cases.
July 14, 2004 - 1.9:
New rules: CloneMethodMustImplementCloneable, CloneThrowsCloneNotSupportedException, EqualsNull, ConfusingTernary

View File

@ -16,7 +16,7 @@ public class AvoidDuplicateLiteralsRuleTest extends SimpleAggregatorTst {
public void setUp() {
rule = new AvoidDuplicateLiteralsRule();
rule.setMessage("avoid ''{0}'' and ''{1}''");
rule.setMessage("avoid ''{0}'' and ''{1}'' and ''{2}''");
rule.addProperty("threshold", "2");
}

View File

@ -5,43 +5,30 @@
These are new rules that are still in progress
</description>
<rule name="ConstructorCallsOverridableMethodRule"
message="Avoid calls to overridable methods during construction"
class="net.sourceforge.pmd.rules.ConstructorCallsOverridableMethodRule">
<description>
Calling overridable methods during construction poses a risk of invoking methods on an
incompletely constructed object. This situation can be difficult to discern.
It may leave the sub-class unable to construct its superclass or forced to
replicate the construction process completely within itself, losing the ability to call
super(). If the default constructor contains a call to an overridable method,
the subclass may be completely uninstantiable. Note that this includes method calls
throughout the control flow graph - i.e., if a constructor Foo() calls a private method
bar() that calls a public method buz(), there's a problem.
</description>
<priority>1</priority>
<example>
<![CDATA[
public class SeniorClass {
public SeniorClass(){
toString(); //may throw NullPointerException if overridden
}
public String toString(){
return "IAmSeniorClass";
}
}
public class JuniorClass extends SeniorClass {
private String name;
public JuniorClass(){
super(); //Automatic call leads to NullPointerException
name = "JuniorClass";
}
public String toString(){
return name.toUpperCase();
}
}
]]>
</example>
</rule>
<rule name="AvoidDuplicateLiterals"
message="The String literal {0} appears {1} times in this file; the first occurrence is on line {2}"
class="net.sourceforge.pmd.rules.AvoidDuplicateLiteralsRule">
<description>
Code containing duplicate String literals can usually be improved by declaring the String as a constant field.
</description>
<priority>3</priority>
<properties>
<property name="threshold" value="4"/>
</properties>
<example>
<![CDATA[
public class Foo {
private void bar() {
buz("Howdy");
buz("Howdy");
buz("Howdy");
buz("Howdy");
}
private void buz(String x) {}
}
]]>
</example>
</rule>

View File

@ -7,7 +7,7 @@ These rules deal with different problems that can occur with String manipulation
<rule name="AvoidDuplicateLiterals"
message="The same String literal appears {0} times in this file; the first occurrence is on line {1}"
message="The String literal {0} appears {1} times in this file; the first occurrence is on line {2}"
class="net.sourceforge.pmd.rules.AvoidDuplicateLiteralsRule">
<description>
Code containing duplicate String literals can usually be improved by declaring the String as a constant field.

View File

@ -105,7 +105,7 @@ public class AvoidDuplicateLiteralsRule extends AbstractRule {
String key = (String) i.next();
List occurrences = (List) literals.get(key);
if (occurrences.size() >= threshold) {
Object[] args = new Object[]{new Integer(occurrences.size()), new Integer(((SimpleNode) occurrences.get(0)).getBeginLine())};
Object[] args = new Object[]{key, new Integer(occurrences.size()), new Integer(((SimpleNode) occurrences.get(0)).getBeginLine())};
String msg = MessageFormat.format(getMessage(), args);
RuleContext ctx = (RuleContext) data;
ctx.getReport().addRuleViolation(createRuleViolation(ctx, ((SimpleNode) occurrences.get(0)).getBeginLine(), msg));
@ -115,12 +115,8 @@ public class AvoidDuplicateLiteralsRule extends AbstractRule {
}
public Object visit(ASTLiteral node, Object data) {
if (!hasAtLeast4Parents(node) || (!fourthParentIsAnArgList(node) && !fourthParentIsAVariableInitializer(node))) {
return data;
}
// just catching strings of 3 chars or more for now - no numbers
if (node.getImage() == null || node.getImage().indexOf('\"') == -1 || node.getImage().length() < 3) {
// just catching strings of 3 chars or more (including the enclosing quotes) for now - no numbers
if (node.getImage() == null || node.getImage().indexOf('\"') == -1 || node.getImage().length() < 5) {
return data;
}
@ -140,24 +136,5 @@ public class AvoidDuplicateLiteralsRule extends AbstractRule {
return data;
}
private boolean fourthParentIsAVariableInitializer(ASTLiteral node) {
return node.jjtGetParent().jjtGetParent().jjtGetParent().jjtGetParent() instanceof ASTVariableInitializer;
}
private boolean fourthParentIsAnArgList(ASTLiteral node) {
return node.jjtGetParent().jjtGetParent().jjtGetParent().jjtGetParent() instanceof ASTArgumentList;
}
private boolean hasAtLeast4Parents(Node node) {
Node currentNode = node;
for (int i = 0; i < 4; i++) {
if (currentNode instanceof ASTCompilationUnit) {
return false;
}
currentNode = currentNode.jjtGetParent();
}
return true;
}
}