Checking in SingularField, thanks to Eric Olander for the code!
git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@3459 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
@ -1,5 +1,5 @@
|
||||
????, 2005 - 3.1:
|
||||
New rule: SimplifyStartsWith [put in optimizations ruleset], UnnecessaryParentheses [put in controversial ruleset], CollapsibleIfStatements [put in basic ruleset], UseAssertEqualsInsteadOfAssertTrue[put in junit ruleset], UseAssertSameInsteadOfAssertTrue [put in junit ruleset], UseStringBufferForStringAppends [put in optimizations ruleset], SimplifyConditional [design ruleset?]
|
||||
New rules: SimplifyStartsWith [put in optimizations ruleset], UnnecessaryParentheses [put in controversial ruleset], CollapsibleIfStatements [put in basic ruleset], UseAssertEqualsInsteadOfAssertTrue[put in junit ruleset], UseAssertSameInsteadOfAssertTrue [put in junit ruleset], UseStringBufferForStringAppends [put in optimizations ruleset], SimplifyConditional [design ruleset?], SingularField [controversial ruleset]
|
||||
Fixed bug 1169731 - UnusedImports no longer reports false positives on types used inside generics. This bug also resulted in a bug in ForLoopShouldBeWhileLoop being fixed, thanks Wim!
|
||||
Fixed bug 1170109 - The Ant task now supports an optional 'targetjdk' attribute that accepts values of '1.3', '1.4', or '1.5'.
|
||||
Fixed bug 1170535 - LongVariable now report variables longer than 17 characters, not 12.
|
||||
|
@ -0,0 +1,76 @@
|
||||
package test.net.sourceforge.pmd.rules;
|
||||
|
||||
import test.net.sourceforge.pmd.testframework.SimpleAggregatorTst;
|
||||
import test.net.sourceforge.pmd.testframework.TestDescriptor;
|
||||
import net.sourceforge.pmd.Rule;
|
||||
import net.sourceforge.pmd.RuleSetNotFoundException;
|
||||
import net.sourceforge.pmd.PMD;
|
||||
|
||||
public class SingularFieldRuleTest extends SimpleAggregatorTst {
|
||||
private Rule rule;
|
||||
|
||||
public void setUp() throws RuleSetNotFoundException {
|
||||
// put in controversial ruleset
|
||||
rule = findRule("rulesets/newrules.xml", "SingularField");
|
||||
}
|
||||
|
||||
public void testAll() {
|
||||
runTests(new TestDescriptor[] {
|
||||
new TestDescriptor(TEST1, "failure case", 1, rule),
|
||||
new TestDescriptor(TEST2, "ok", 0, rule),
|
||||
new TestDescriptor(TEST3, "second method uses 'this'", 0, rule),
|
||||
new TestDescriptor(TEST4, "skip publics", 0, rule),
|
||||
new TestDescriptor(TEST5, "skip statics", 0, rule),
|
||||
});
|
||||
}
|
||||
|
||||
private static final String TEST1 =
|
||||
"public class Foo {" + PMD.EOL +
|
||||
" private int x;" + PMD.EOL +
|
||||
" int bar(int y) {" + PMD.EOL +
|
||||
" x = y + 5; " + PMD.EOL +
|
||||
" return x;" + PMD.EOL +
|
||||
" }" + PMD.EOL +
|
||||
"}";
|
||||
|
||||
private static final String TEST2 =
|
||||
"public class Foo {" + PMD.EOL +
|
||||
" private int x;" + PMD.EOL +
|
||||
" void setX(int x) {" + PMD.EOL +
|
||||
" this.x = x;" + PMD.EOL +
|
||||
" }" + PMD.EOL +
|
||||
" int getX() {" + PMD.EOL +
|
||||
" return x;" + PMD.EOL +
|
||||
" }" + PMD.EOL +
|
||||
"}";
|
||||
|
||||
private static final String TEST3 =
|
||||
"public class Foo {" + PMD.EOL +
|
||||
" private int x;" + PMD.EOL +
|
||||
" void setX(int x) {" + PMD.EOL +
|
||||
" this.x = x;" + PMD.EOL +
|
||||
" }" + PMD.EOL +
|
||||
" int getX() {" + PMD.EOL +
|
||||
" return this.x;" + PMD.EOL +
|
||||
" }" + PMD.EOL +
|
||||
"}";
|
||||
|
||||
private static final String TEST4 =
|
||||
"public class Foo {" + PMD.EOL +
|
||||
" public int x;" + PMD.EOL +
|
||||
" int bar(int y) {" + PMD.EOL +
|
||||
" x = y + 5; " + PMD.EOL +
|
||||
" return x;" + PMD.EOL +
|
||||
" }" + PMD.EOL +
|
||||
"}";
|
||||
|
||||
private static final String TEST5 =
|
||||
"public class Foo {" + PMD.EOL +
|
||||
" private static int x;" + PMD.EOL +
|
||||
" int bar(int y) {" + PMD.EOL +
|
||||
" x = y + 5; " + PMD.EOL +
|
||||
" return x;" + PMD.EOL +
|
||||
" }" + PMD.EOL +
|
||||
"}";
|
||||
|
||||
}
|
@ -286,5 +286,26 @@ class Foo {
|
||||
</example>
|
||||
</rule>
|
||||
|
||||
<rule name="SingularField"
|
||||
message="Perhaps ''{0}'' could be replaced by a local variable."
|
||||
class="net.sourceforge.pmd.rules.SingularField">
|
||||
<description>
|
||||
A field that's only used by one method could perhaps be replaced by a local variable
|
||||
</description>
|
||||
<priority>3</priority>
|
||||
<example>
|
||||
<![CDATA[
|
||||
public class Foo {
|
||||
private int x;
|
||||
public void foo(int y) {
|
||||
x = y + 5;
|
||||
return x;
|
||||
}
|
||||
}
|
||||
]]>
|
||||
</example>
|
||||
</rule>
|
||||
|
||||
|
||||
</ruleset>
|
||||
|
||||
|
@ -29,50 +29,7 @@ These are new rules that are still in progress
|
||||
</rule>
|
||||
|
||||
|
||||
<rule name="SimplifyConditional"
|
||||
message="No need to check for null before an instanceof"
|
||||
class="net.sourceforge.pmd.rules.XPathRule">
|
||||
<description>
|
||||
No need to check for null before an instanceof; the instanceof keyword returns false when given a null argument.
|
||||
</description>
|
||||
<properties>
|
||||
<property name="xpath">
|
||||
<value>
|
||||
<![CDATA[
|
||||
//Expression
|
||||
[ConditionalOrExpression
|
||||
[EqualityExpression[@Image='==']
|
||||
//NullLiteral
|
||||
and
|
||||
UnaryExpressionNotPlusMinus
|
||||
[@Image='!']//InstanceOfExpression[PrimaryExpression
|
||||
//Name/@Image = ancestor::ConditionalOrExpression/EqualityExpression
|
||||
//PrimaryPrefix/Name/@Image]]
|
||||
or
|
||||
ConditionalAndExpression
|
||||
[EqualityExpression[@Image='!=']//NullLiteral
|
||||
and
|
||||
InstanceOfExpression
|
||||
[PrimaryExpression
|
||||
//Name/@Image = ancestor::ConditionalAndExpression
|
||||
/EqualityExpression//PrimaryPrefix/Name/@Image]]
|
||||
]]>
|
||||
</value>
|
||||
</property>
|
||||
</properties>
|
||||
<priority>3</priority>
|
||||
|
||||
<example>
|
||||
<![CDATA[
|
||||
class Foo {
|
||||
void bar(Object x) {
|
||||
if (x != null && x instanceof Bar) {
|
||||
// just drop the "x != null" check
|
||||
}
|
||||
}
|
||||
} ]]>
|
||||
</example>
|
||||
</rule>
|
||||
<!--
|
||||
|
||||
|
||||
|
52
pmd/src/net/sourceforge/pmd/rules/SingularField.java
Normal file
52
pmd/src/net/sourceforge/pmd/rules/SingularField.java
Normal file
@ -0,0 +1,52 @@
|
||||
/*
|
||||
* SingularField.java
|
||||
*
|
||||
* Created on April 17, 2005, 9:49 PM
|
||||
*/
|
||||
|
||||
package net.sourceforge.pmd.rules;
|
||||
|
||||
import java.text.MessageFormat;
|
||||
import java.util.List;
|
||||
import net.sourceforge.pmd.AbstractRule;
|
||||
import net.sourceforge.pmd.RuleContext;
|
||||
import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration;
|
||||
import net.sourceforge.pmd.ast.ASTConstructorDeclaration;
|
||||
import net.sourceforge.pmd.ast.ASTFieldDeclaration;
|
||||
import net.sourceforge.pmd.ast.ASTMethodDeclaration;
|
||||
import net.sourceforge.pmd.ast.ASTMethodDeclarator;
|
||||
import net.sourceforge.pmd.ast.ASTVariableDeclaratorId;
|
||||
import org.jaxen.JaxenException;
|
||||
|
||||
/**
|
||||
*
|
||||
* @author Eric Olander
|
||||
*/
|
||||
public class SingularField extends AbstractRule {
|
||||
|
||||
public Object visit(ASTFieldDeclaration node, Object data) {
|
||||
if (node.isPrivate() && !node.isStatic()) {
|
||||
List list = node.findChildrenOfType(ASTVariableDeclaratorId.class);
|
||||
ASTVariableDeclaratorId decl = (ASTVariableDeclaratorId)list.get(0);
|
||||
String name = decl.getImage();
|
||||
String path = "//MethodDeclaration[.//PrimaryExpression[.//Name[@Image = \""+name+"\" or substring-before(@Image, \".\") = \""+name+"\"] or .//PrimarySuffix[@Image = \""+name+"\"]]] |" +
|
||||
"//ConstructorDeclaration[.//PrimaryExpression[.//Name[@Image = \""+name+"\" or substring-before(@Image, \".\") = \""+name+"\"] or .//PrimarySuffix[@Image = \""+name+"\"]]]";
|
||||
try {
|
||||
List nodes = node.findChildNodesWithXPath(path);
|
||||
if (nodes.size() == 1) {
|
||||
String method;
|
||||
if (nodes.get(0) instanceof ASTMethodDeclaration) {
|
||||
method = ((ASTMethodDeclarator)((ASTMethodDeclaration)nodes.get(0)).findChildrenOfType(ASTMethodDeclarator.class).get(0)).getImage();
|
||||
} else {
|
||||
method = ((ASTClassOrInterfaceDeclaration)((ASTConstructorDeclaration)nodes.get(0)).getFirstParentOfType(ASTClassOrInterfaceDeclaration.class)).getImage();
|
||||
}
|
||||
((RuleContext)data).getReport().addRuleViolation(createRuleViolation((RuleContext) data, decl, MessageFormat.format(getMessage(), new Object[]{name, method})));
|
||||
}
|
||||
} catch (JaxenException je) {
|
||||
je.printStackTrace();
|
||||
}
|
||||
}
|
||||
return data;
|
||||
}
|
||||
|
||||
}
|
@ -43,11 +43,12 @@
|
||||
</subsection>
|
||||
<subsection name="Contributors">
|
||||
<ul>
|
||||
<li>Eric Olander - SingularField, SimplifyConditional fix, UseStringBufferForStringAppends, CollapsibleIfStatements, AvoidInstanceofChecksInCatchClause, AssignmentToNonFinalStatic rule, nice patch for DFAPanel cleanup, AvoidProtectedFieldInFinalClass, ImmutableFieldRule, noticed missing image in Postfix nodes</li>
|
||||
<li>Tomas Gustavsson - reported pmd-web breakage</li>
|
||||
<li>Tom Parker - AvoidInstantiatingObjectsInLoops bug report, AtLeastOneConstructor bug report</li>
|
||||
<li>Fred Hartman - Fixed bug in UnnecessaryBooleanAssertion</li>
|
||||
<li>Payal Subhash - Tweaks to CSVRenderer</li>
|
||||
<li>Wouter Zelle - initial implementation of SimplifyConditional</li>
|
||||
<li>Eric Olander - SimplifyConditional fix, UseStringBufferForStringAppends, CollapsibleIfStatements, AvoidInstanceofChecksInCatchClause, AssignmentToNonFinalStatic rule, nice patch for DFAPanel cleanup, AvoidProtectedFieldInFinalClass, ImmutableFieldRule, noticed missing image in Postfix nodes</li>
|
||||
<li>Christophe Mourette - Reported JDK 1.3 problem with XMLRenderer</li>
|
||||
<li>Wim Deblauwe - SystemPrintln bug report, helped get Ant task and CLI squared away with JDK 1.5 params, JDK 1.5-specific bug reports, suggested improvements for ExceptionSignatureDeclaration</li>
|
||||
<li>Alex Givant - caught documentation bug</li>
|
||||
|
Reference in New Issue
Block a user