Applied patch 1565532 Java NCSS checks from Jason Bennett

Changes made:
- Changed classes to Ncss* to maintain consistency between them and the rule name
- Added JUnit tests. Ran against NCSS current release to validate code size
- Moved to codesize package


git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4681 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Allan Caplan
2006-10-16 15:42:45 +00:00
parent 2378d27100
commit da63ae1c76
11 changed files with 673 additions and 2 deletions

View File

@ -1,7 +1,7 @@
?????, 2006 - 3.9:
New rules:
Basic ruleset: BigIntegerInstantiation(1.4 and 1.5)
Codesize ruleset: NPathComplexity
Codesize ruleset: NPathComplexity, NcssTypeCount, NcssMethodCount, NcssConstructorCount
Design ruleset: UseCollectionIsEmpty
Fixed bug 1570915 - AvoidRethrowingException no longer reports a false positive for certain nested exceptions.
Fixed bug 1571324 - UselessStringValueOf no longer reports a false positive for additive expressions.

View File

@ -0,0 +1,85 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package test.net.sourceforge.pmd.rules.codesize;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.RuleSetNotFoundException;
import test.net.sourceforge.pmd.testframework.SimpleAggregatorTst;
import test.net.sourceforge.pmd.testframework.TestDescriptor;
public class NcssConstructorCountTest extends SimpleAggregatorTst {
private Rule rule;
public void setUp() throws RuleSetNotFoundException {
rule = findRule("codesize", "NcssConstructorCount");
}
public void testAll() {
rule.addProperty("minimum", "13");
runTests(new TestDescriptor[]{
new TestDescriptor(TEST1, "short", 0, rule),
new TestDescriptor(TEST2, "lots of comments", 0, rule),
new TestDescriptor(TEST3, "long method", 1, rule),
});
}
public void testChangeMinimum() {
rule.addProperty("minimum", "14"); // obtained this value by using NCSS directly
runTests(new TestDescriptor[]{
new TestDescriptor(TEST3, "long method - changed minimum", 0, rule),
});
}
private static final String TEST1 =
"public class Foo {" + PMD.EOL +
" public Foo() {" + PMD.EOL +
" bar();" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST2 =
"public class Foo {" + PMD.EOL +
" public Foo() {" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" bar();" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" foo();" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST3 =
"public class Foo {" + PMD.EOL +
" public Foo() {" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" }" + PMD.EOL +
"}";
}

View File

@ -0,0 +1,85 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package test.net.sourceforge.pmd.rules.codesize;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.RuleSetNotFoundException;
import test.net.sourceforge.pmd.testframework.SimpleAggregatorTst;
import test.net.sourceforge.pmd.testframework.TestDescriptor;
public class NcssMethodCountTest extends SimpleAggregatorTst {
private Rule rule;
public void setUp() throws RuleSetNotFoundException {
rule = findRule("codesize", "NcssMethodCount");
}
public void testAll() {
rule.addProperty("minimum", "13");
runTests(new TestDescriptor[]{
new TestDescriptor(TEST1, "short", 0, rule),
new TestDescriptor(TEST2, "lots of comments", 0, rule),
new TestDescriptor(TEST3, "long method", 1, rule),
});
}
public void testChangeMinimum() {
rule.addProperty("minimum", "14"); // validated this number against NCSS
runTests(new TestDescriptor[]{
new TestDescriptor(TEST3, "long method - changed minimum", 0, rule),
});
}
private static final String TEST1 =
"public class Foo {" + PMD.EOL +
" public static void main(String args[]) {" + PMD.EOL +
" bar();" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST2 =
"public class Foo {" + PMD.EOL +
" public static void main(String args[]) {" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" bar();" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" foo();" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST3 =
"public class Foo {" + PMD.EOL +
" public static void main(String args[]) {" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" }" + PMD.EOL +
"}";
}

View File

@ -0,0 +1,85 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package test.net.sourceforge.pmd.rules.codesize;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.RuleSetNotFoundException;
import test.net.sourceforge.pmd.testframework.SimpleAggregatorTst;
import test.net.sourceforge.pmd.testframework.TestDescriptor;
public class NcssTypeCountTest extends SimpleAggregatorTst {
private Rule rule;
public void setUp() throws RuleSetNotFoundException {
rule = findRule("codesize", "NcssTypeCount");
}
public void testAll() {
rule.addProperty("minimum", "13");
runTests(new TestDescriptor[]{
new TestDescriptor(TEST1, "short", 0, rule),
new TestDescriptor(TEST2, "lots of comments", 0, rule),
new TestDescriptor(TEST3, "long method", 1, rule),
});
}
public void testChangeMinimum() {
rule.addProperty("minimum", "15"); // validated this number against NCSS
runTests(new TestDescriptor[]{
new TestDescriptor(TEST3, "long method - changed minimum", 0, rule),
});
}
private static final String TEST1 =
"public class Foo {" + PMD.EOL +
" public static void main(String args[]) {" + PMD.EOL +
" bar();" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST2 =
"public class Foo {" + PMD.EOL +
" public static void main(String args[]) {" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" bar();" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" foo();" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST3 =
"public class Foo {" + PMD.EOL +
" public static void main(String args[]) {" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" bar();" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" //nothing to see here" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" foo();" + PMD.EOL +
" }" + PMD.EOL +
"}";
}

View File

@ -221,6 +221,77 @@ public class Person {
</example>
</rule>
<rule name="NcssMethodCount" message="The method {0}() has an NCSS line count of {1}"
class="net.sourceforge.pmd.rules.codesize.NcssMethodCount"
externalInfoUrl="http://pmd.sourceforge.net/rules/codesize.html#NcssMethodCount">
<properties>
<property name="minimum" description="The method NCSS count reporting threshold" value="100"/>
</properties>
<example>
<![CDATA[
public class Foo extends Bar {
public int methd() {
super.methd();
//this method only has 1 NCSS lines
return 1;
}
}
]]>
</example>
</rule>
<rule name="NcssTypeCount" message="The type has an NCSS line count of {0}"
class="net.sourceforge.pmd.rules.codesize.NcssTypeCount"
externalInfoUrl="http://pmd.sourceforge.net/rules/codesize.html#NcssTypeCount">
<properties>
<property name="minimum" description="The type NCSS count reporting threshold" value="1500"/>
</properties>
<example>
<![CDATA[
public class Foo extends Bar {
public Foo() {
//this class only has 6 NCSS lines
super();
super.foo();
}
}
]]>
</example></rule>
<rule name="NcssConstructorCount" message="The constructor with {0} parameters has an NCSS line count of {1}"
class="net.sourceforge.pmd.rules.codesize.NcssConstructorCount"
externalInfoUrl="http://pmd.sourceforge.net/rules/codesize.html#NcssConstructorCount">
<properties>
<property name="minimum" description="The constructor NCSS count reporting threshold" value="100"/>
</properties>
<example>
<![CDATA[
public class Foo extends Bar {
public Foo() {
super();
//this constructor only has 1 NCSS lines
super.foo();
}
}
]]>
</example>
</rule>
</ruleset>

View File

@ -9,4 +9,7 @@ This ruleset contains links to rules that are new in PMD v3.9
<rule ref="rulesets/basic.xml/BigIntegerInstantiation_1.5"/>
<rule ref="rulesets/codesize.xml/NPathComplexity"/>
<rule ref="rulesets/design.xml/UseCollectionIsEmpty"/>
<rule ref="rulesets/codesize.xml/NcssMethodCount"/>
<rule ref="rulesets/codesize.xml/NcssTypeCount"/>
<rule ref="rulesets/codesize.xml/NcssConstructorCount"/>
</ruleset>

View File

@ -0,0 +1,179 @@
package net.sourceforge.pmd.rules.codesize;
import net.sourceforge.pmd.ast.ASTBreakStatement;
import net.sourceforge.pmd.ast.ASTCatchStatement;
import net.sourceforge.pmd.ast.ASTContinueStatement;
import net.sourceforge.pmd.ast.ASTDoStatement;
import net.sourceforge.pmd.ast.ASTFinallyStatement;
import net.sourceforge.pmd.ast.ASTForInit;
import net.sourceforge.pmd.ast.ASTForStatement;
import net.sourceforge.pmd.ast.ASTIfStatement;
import net.sourceforge.pmd.ast.ASTLabeledStatement;
import net.sourceforge.pmd.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.ast.ASTReturnStatement;
import net.sourceforge.pmd.ast.ASTStatementExpression;
import net.sourceforge.pmd.ast.ASTStatementExpressionList;
import net.sourceforge.pmd.ast.ASTSwitchLabel;
import net.sourceforge.pmd.ast.ASTSwitchStatement;
import net.sourceforge.pmd.ast.ASTSynchronizedStatement;
import net.sourceforge.pmd.ast.ASTThrowStatement;
import net.sourceforge.pmd.ast.ASTWhileStatement;
import net.sourceforge.pmd.ast.SimpleJavaNode;
import net.sourceforge.pmd.stat.DataPoint;
import net.sourceforge.pmd.stat.StatisticalRule;
/**
* Abstract superclass for NCSS counting methods. Counts tokens according to <a
* href="http://www.kclee.de/clemens/java/javancss/">JavaNCSS rules</a>.
*
* @author Jason Bennett
*/
public abstract class AbstractNcssCount extends StatisticalRule {
private Class nodeClass;
/**
* Count the nodes of the given type using NCSS rules.
*
* @param nodeClass
* class of node to count
*/
protected AbstractNcssCount(Class nodeClass) {
this.nodeClass = nodeClass;
}
public Object visit(SimpleJavaNode node, Object data) {
int numNodes = 0;
for ( int i = 0; i < node.jjtGetNumChildren(); i++ ) {
SimpleJavaNode simpleNode = (SimpleJavaNode) node.jjtGetChild( i );
Integer treeSize = (Integer) simpleNode.jjtAccept( this, data );
numNodes += treeSize.intValue();
}
if ( this.nodeClass.isInstance( node ) ) {
// Add 1 to account for base node
numNodes++;
DataPoint point = new DataPoint();
point.setNode( node );
point.setScore( 1.0 * numNodes );
point.setMessage( getMessage() );
addDataPoint( point );
}
return new Integer( numNodes );
}
/**
* Count the number of children of the given Java node. Adds one to count the
* node itself.
*
* @param node
* java node having children counted
* @param data
* node data
* @return count of the number of children of the node, plus one
*/
protected Integer countNodeChildren(SimpleJavaNode node, Object data) {
Integer nodeCount = null;
int lineCount = 0;
for ( int i = 0; i < node.jjtGetNumChildren(); i++ ) {
nodeCount = (Integer) ( (SimpleJavaNode) node.jjtGetChild( i ) ).jjtAccept(
this, data );
lineCount += nodeCount.intValue();
}
return Integer.valueOf( ++lineCount );
}
public Object visit(ASTForStatement node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTDoStatement node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTIfStatement node, Object data) {
Integer lineCount = countNodeChildren( node, data );
if ( node.hasElse() ) {
int lines = lineCount.intValue();
lines++;
lineCount = Integer.valueOf( lines );
}
return lineCount;
}
public Object visit(ASTWhileStatement node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTBreakStatement node, Object data) {
return Integer.valueOf( 1 );
}
public Object visit(ASTCatchStatement node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTContinueStatement node, Object data) {
return Integer.valueOf( 1 );
}
public Object visit(ASTFinallyStatement node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTReturnStatement node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTSwitchStatement node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTSynchronizedStatement node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTThrowStatement node, Object data) {
return Integer.valueOf( 1 );
}
public Object visit(ASTStatementExpression node, Object data) {
// "For" update expressions do not count as separate lines of code
if ( node.jjtGetParent() instanceof ASTStatementExpressionList ) {
return Integer.valueOf( 0 );
}
return Integer.valueOf( 1 );
}
public Object visit(ASTLabeledStatement node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTLocalVariableDeclaration node, Object data) {
// "For" init declarations do not count as separate lines of code
if ( node.jjtGetParent() instanceof ASTForInit ) {
return Integer.valueOf( 0 );
}
/*
* This will count variables declared on the same line as separate NCSS
* counts. This violates JavaNCSS standards, but I'm not convinced that's a
* bad thing here.
*/
return countNodeChildren( node, data );
}
public Object visit(ASTSwitchLabel node, Object data) {
return countNodeChildren( node, data );
}
}

View File

@ -0,0 +1,42 @@
package net.sourceforge.pmd.rules.codesize;
import java.util.Iterator;
import java.util.Set;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.ast.ASTExplicitConstructorInvocation;
import net.sourceforge.pmd.stat.DataPoint;
/**
* Non-commented source statement counter for constructors.
*
* @author Jason Bennett
*/
public class NcssConstructorCount extends AbstractNcssCount {
/**
* Count constructor declarations. This includes any explicit super() calls.
*/
public NcssConstructorCount() {
super( ASTConstructorDeclaration.class );
}
public Object visit(ASTExplicitConstructorInvocation node, Object data) {
return Integer.valueOf( 1 );
}
protected void makeViolations(RuleContext ctx, Set p) {
Iterator points = p.iterator();
while ( points.hasNext() ) {
DataPoint point = (DataPoint) points.next();
// TODO need to put class name or constructor ID in string
addViolation(
ctx,
point.getNode(),
new String[] {
String.valueOf( ( (ASTConstructorDeclaration) point.getNode() ).getParameterCount() ),
String.valueOf( (int) point.getScore() ) } );
}
}
}

View File

@ -0,0 +1,38 @@
package net.sourceforge.pmd.rules.codesize;
import java.util.Iterator;
import java.util.Set;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.stat.DataPoint;
/**
* Non-commented source statement counter for methods.
*
* @author Jason Bennett
*/
public class NcssMethodCount extends AbstractNcssCount {
/**
* Count the size of all non-constructor methods.
*/
public NcssMethodCount() {
super( ASTMethodDeclaration.class );
}
public Object visit(ASTMethodDeclaration node, Object data) {
return super.visit( node, data );
}
protected void makeViolations(RuleContext ctx, Set p) {
Iterator points = p.iterator();
while ( points.hasNext() ) {
DataPoint point = (DataPoint) points.next();
addViolation( ctx, point.getNode(), new String[] {
( (ASTMethodDeclaration) point.getNode() ).getMethodName(),
String.valueOf( (int) point.getScore() ) } );
}
}
}

View File

@ -0,0 +1,83 @@
package net.sourceforge.pmd.rules.codesize;
import java.util.Iterator;
import java.util.Set;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.ast.ASTEnumDeclaration;
import net.sourceforge.pmd.ast.ASTExplicitConstructorInvocation;
import net.sourceforge.pmd.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.ast.ASTInitializer;
import net.sourceforge.pmd.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.ast.ASTTypeDeclaration;
import net.sourceforge.pmd.stat.DataPoint;
/**
* Non-commented source statement counter for type declarations.
*
* @author Jason Bennett
*/
public class NcssTypeCount extends AbstractNcssCount {
/**
* Count type declarations. This includes classes as well as enums and
* annotations.
*/
public NcssTypeCount() {
super( ASTTypeDeclaration.class );
}
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
if ( !node.isNested() ) {
return super.visit( node, data );
}
return countNodeChildren( node, data );
}
public Object visit(ASTConstructorDeclaration node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTExplicitConstructorInvocation node, Object data) {
return Integer.valueOf( 1 );
}
public Object visit(ASTEnumDeclaration node, Object data) {
/*
* If the enum is a type in and of itself, don't count its declaration
* twice.
*/
if ( node.jjtGetParent() instanceof ASTTypeDeclaration ) {
Integer nodeCount = countNodeChildren( node, data );
int count = nodeCount.intValue() - 1;
return Integer.valueOf( count );
}
return countNodeChildren( node, data );
}
public Object visit(ASTMethodDeclaration node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTInitializer node, Object data) {
return countNodeChildren( node, data );
}
public Object visit(ASTFieldDeclaration node, Object data) {
return Integer.valueOf( 1 );
}
protected void makeViolations(RuleContext ctx, Set p) {
Iterator points = p.iterator();
while ( points.hasNext() ) {
DataPoint point = (DataPoint) points.next();
addViolation( ctx, point.getNode(),
String.valueOf( (int) point.getScore() ) );
}
}
}

View File

@ -55,7 +55,7 @@
<li>Peter Van de Voorde - Rewrote the 'create rule XML' functionality in the designer utility</li>
<li>Josh Devins - Reported bug with annotation parsing</li>
<li>Alan Berg - Reported bug in Ant task</li>
<li>Jason Bennett - Fix for UnnecessaryLocalBeforeReturn, wrote NPathComplexity rule, patches to improve CyclomaticComplexity rule, Implemented UseCollectionIsEmpty</li>
<li>Jason Bennett - Fix for UnnecessaryLocalBeforeReturn, wrote NPathComplexity rule, patches to improve CyclomaticComplexity rule, Implemented: UseCollectionIsEmpty, NcssTypeCount, NcssMethodCount, NcssConstructor</li>
<li>Brent Fisher - SummaryHTML report improvements</li>
<li>George Thomas - Wrote AvoidRethrowingException rule</li>
<li>Robert Simmons - Reported bug in optimizations package along with suggestions for fix</li>