Fixed bug in SimplifyBooleanExpressions - now it catches more cases. Thanks to Paul Rowe for the report. Also fixed a problem in the RuleViolationComparator; violations on the same line of the same file resulted in one of them getting discarded since I'm using a TreeSet

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@2814 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Tom Copeland
2004-07-19 17:33:23 +00:00
parent 34ef382e64
commit bc9af74eed
9 changed files with 55 additions and 25 deletions

View File

@ -1,7 +1,8 @@
?????, 2004 - 2.0:
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
Applied patch in RFE 992576 - Enhancements to VariableNamingConventionsRule
Fixed bug in SimplifyBooleanExpressions - now it catches more cases.
July 14, 2004 - 1.9:
New rules: CloneMethodMustImplementCloneable, CloneThrowsCloneNotSupportedException, EqualsNull, ConfusingTernary

View File

@ -81,7 +81,7 @@ public class RuleViolationTest extends TestCase {
ctx.setSourceCodeFilename("filename");
RuleViolation r1 = new RuleViolation(rule, 10, "description", ctx);
RuleViolation r2 = new RuleViolation(rule, 10, "description", ctx);
assertEquals(0, comp.compare(r1, r2));
assertEquals(0, comp.compare(r2, r1));
assertEquals(1, comp.compare(r1, r2));
assertEquals(1, comp.compare(r2, r1));
}
}

View File

@ -21,7 +21,7 @@ public class ClassNamingConventionsRuleTest extends SimpleAggregatorTst {
"public class foo {};";
private static final String TEST2 =
"public class foo_bar {};";
"public class Foo_bar {};";
private static final String TEST3 =
"public class FooBar {};";

View File

@ -6,9 +6,10 @@ package test.net.sourceforge.pmd.rules;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.RuleSetNotFoundException;
import test.net.sourceforge.pmd.testframework.RuleTst;
import test.net.sourceforge.pmd.testframework.SimpleAggregatorTst;
import test.net.sourceforge.pmd.testframework.TestDescriptor;
public class SimplifyBooleanExpressionsRuleTest extends RuleTst {
public class SimplifyBooleanExpressionsRuleTest extends SimpleAggregatorTst {
private Rule rule;
@ -16,14 +17,13 @@ public class SimplifyBooleanExpressionsRuleTest extends RuleTst {
rule = findRule("rulesets/design.xml", "SimplifyBooleanExpressions");
}
public void testInFieldAssignment() throws Throwable {
runTestFromString(TEST1, 1, rule);
}
public void testInMethodBody() throws Throwable {
runTestFromString(TEST2, 1, rule);
}
public void testOK() throws Throwable {
runTestFromString(TEST3, 0, rule);
public void testAll() {
runTests(new TestDescriptor[] {
new TestDescriptor(TEST1, "in field assignment", 1, rule),
new TestDescriptor(TEST2, "in method body", 1, rule),
new TestDescriptor(TEST3, "ok", 0, rule),
new TestDescriptor(TEST4, "two cases in an && expression", 2, rule),
});
}
private static final String TEST1 =
@ -43,4 +43,19 @@ public class SimplifyBooleanExpressionsRuleTest extends RuleTst {
"public class Foo {" + PMD.EOL +
" boolean bar = true;" + PMD.EOL +
"}";
private static final String TEST4 =
"public class Foo {" + PMD.EOL +
" void bar() {" + PMD.EOL +
" if (getFoo() == false && " + PMD.EOL +
" isBar() == true) {}" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST5 =
"public class Foo {" + PMD.EOL +
" void bar() {" + PMD.EOL +
" if (getFoo() == false && isBar() == true) {}" + PMD.EOL +
" }" + PMD.EOL +
"}";
}

View File

@ -26,12 +26,12 @@ public class UnusedLocalVariableTest extends SimpleAggregatorTst {
new TestDescriptor(TEST5, "unused local in static initializer", 1, rule),
new TestDescriptor(TEST6, "unused field", 0, rule),
new TestDescriptor(TEST7, "loop indexes are not unused locals", 0, rule),
new TestDescriptor(TEST8, "", 0, rule),
new TestDescriptor(TEST9, "", 0, rule),
new TestDescriptor(TEST10, "", 2, rule),
new TestDescriptor(TEST11, "", 0, rule),
new TestDescriptor(TEST12, "", 0, rule),
new TestDescriptor(TEST13, "", 2, rule),
new TestDescriptor(TEST8, "8", 0, rule),
new TestDescriptor(TEST9, "9", 0, rule),
new TestDescriptor(TEST10, "10", 2, rule),
new TestDescriptor(TEST11, "11", 0, rule),
new TestDescriptor(TEST12, "12", 0, rule),
new TestDescriptor(TEST13, "13", 2, rule),
new TestDescriptor(TEST14, "an assignment does not a usage make", 1, rule),
new TestDescriptor(TEST15, "a compound assignment operator doth a usage make", 0, rule),
new TestDescriptor(TEST16, "assignment to a member field means used", 0, rule),

View File

@ -66,10 +66,9 @@ public class Foo {
<properties>
<property name="xpath">
<value>
<![CDATA[
//Expression/EqualityExpression
/PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral
]]>
<![CDATA[
//EqualityExpression/PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral
]]>
</value>
</property>
</properties>

View File

@ -26,6 +26,11 @@ public class RuleViolation {
if (r1.getDescription() != null && r2.getDescription() != null && !r1.getDescription().equals(r2.getDescription())) {
return r1.getDescription().compareTo(r2.getDescription());
}
if (r1.getLine() == r2.getLine()) {
return 1;
}
// line number diff maps nicely to compare()
return r1.getLine() - r2.getLine();
}
@ -62,4 +67,8 @@ public class RuleViolation {
public String getFilename() {
return filename;
}
public String toString() {
return getFilename() + ":" + getRule() + ":" + getDescription() + ":" + getLine();
}
}

View File

@ -11,14 +11,19 @@ import net.sourceforge.pmd.symboltable.NameOccurrence;
import net.sourceforge.pmd.symboltable.VariableNameDeclaration;
import java.text.MessageFormat;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
public class UnusedLocalVariableRule extends AbstractRule {
private Set visited = new HashSet();
public Object visit(ASTVariableDeclaratorId node, Object data) {
if (node.jjtGetParent().jjtGetParent() instanceof ASTLocalVariableDeclaration) {
if (node.jjtGetParent().jjtGetParent() instanceof ASTLocalVariableDeclaration && !visited.contains(node.jjtGetParent().jjtGetParent())) {
visited.add(node.jjtGetParent().jjtGetParent());
Map locals = node.getScope().getVariableDeclarations();
for (Iterator i = locals.keySet().iterator(); i.hasNext();) {
VariableNameDeclaration decl = (VariableNameDeclaration) i.next();

View File

@ -40,6 +40,7 @@
</subsection>
<subsection name="Contributors">
<ul>
<li>Paul Rowe - bug report for SimplifyBooleanExpressions</li>
<li>Dave Brosius - a nice patch to clean up some string handling inefficiencies</li>
<li>Enno Derksen - enhancements to VariableNamingConventionsRule</li>
<li>Michael Haggerty - bug reports for FinalizeDoesNotCallSuperFinalize and UnusedModifier</li>