diff --git a/docs/config_metrics.html b/docs/config_metrics.html index a0fa5e711..7004da666 100644 --- a/docs/config_metrics.html +++ b/docs/config_metrics.html @@ -26,6 +26,7 @@
  • ClassDataAbstractionCoupling
  • ClassFanOutComplexity
  • CyclomaticComplexity
    • +
  • JavaNCSS
  • NPathComplexity
    • @@ -320,6 +321,95 @@ considered good, 5-7 ok, 8-10 consider re-factoring, and 11+ re-factor now! <module name="NPathComplexity"> <property name="max" value="20"/> </module> + + + +

    Package

    +

    + com.puppycrawl.tools.checkstyle.checks.metrics +

    + +

    Parent Module

    +

    + TreeWalker +

    + + +

    JavaNCSS

    +

    Description

    + +

    + Determines complexity of methods, classes and files by counting + the Non Commenting Source Statements (NCSS). + This check adheres to the + specification for the JavaNCSS-Tool written by + Chr. Clemens Lee which sadly seems to have been pulled + off the web.
    + Rougly said the NCSS metric is calculated by counting the source + lines which are not comments, (nearly) equivalent to counting + the semicolons and opening curly braces.
    + The NCSS for a class is summarized from the NCSS of all its methods, + the NCSS of its nested classes and the number of member variable + declarations.
    + The NCSS for a file is summarized from the ncss of all its top level + classes, the number of imports and the package declaration. +

    +

    + Rationale: + Too large methods and classes are hard to read and costly to + maintain. A large NCSS number often means that a method or + class has too many responsabilities and/or functionalities + which should be decomposed into smaller units. +

    + + +

    Properties

    + + + + + + + + + + + + + + + + + + + + + + + + + +
    namedescriptiontypedefault value
    methodMaximumthe maximum allowed number of non commenting lines in + a method.integer50
    classMaximumthe maximum allowed number of non commenting lines in + a class.integer1500
    fileMaximumthe maximum allowed number of non commenting lines in + a file including all top level and nested classes.integer2000
    + +

    Examples

    +

    + To configure the check: +

    + 
    +<module name="JavaNCSS"/>
+
    +

    + To configure the check with 40 allowed non commenting lines + for a method: +

    + 
    +<module name="JavaNCSS">
+    <property name="methodMaximum" value="40"/>
+</module>
    diff --git a/docs/releasenotes.html b/docs/releasenotes.html index 934765d34..0052468bb 100644 --- a/docs/releasenotes.html +++ b/docs/releasenotes.html @@ -66,6 +66,9 @@ contributed by Daniel Grenner (module RequiredRegexp, request 606115, patch 902189). +
  • Added check for the ncss metric. (module JavaNCSS, + patch 920820)
    • +

    diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/metrics/JavaNCSSCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/metrics/JavaNCSSCheck.java new file mode 100755 index 000000000..aa779627f --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/metrics/JavaNCSSCheck.java @@ -0,0 +1,370 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2004 Oliver Burn +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; either +// version 2.1 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +//////////////////////////////////////////////////////////////////////////////// +package com.puppycrawl.tools.checkstyle.checks.metrics; + +import java.util.Stack; + +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.DetailAST; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; + +/** + * This check calculates the Non Commenting Source Statements (NCSS) metric for + * java source files and methods. The check adheres to the JavaNCSS specification + * and gives the same results as the JavaNCSS tool. + * + * The NCSS-metric tries to determine complexity of methods, classes and files + * by counting the non commenting lines. Roughly said this is (nearly) + * equivalent to counting the semicolons and opening curly braces. + * + * @author Lars Ködderitzsch + */ +public class JavaNCSSCheck extends Check +{ + /** default constant for max file ncss */ + private static final int FILE_MAX_NCSS = 2000; + + /** default constant for max file ncss */ + private static final int CLASS_MAX_NCSS = 1500; + + /** default constant for max method ncss */ + private static final int METHOD_MAX_NCSS = 50; + + /** maximum ncss for a complete source file */ + private int mFileMax = FILE_MAX_NCSS; + + /** maximum ncss for a class */ + private int mClassMax = CLASS_MAX_NCSS; + + /** maximum ncss for a method */ + private int mMethodMax = METHOD_MAX_NCSS; + + /** list containing the stacked counters */ + private Stack mCounters; + + /** + * @see com.puppycrawl.tools.checkstyle.api.Check#getDefaultTokens() + */ + public int[] getDefaultTokens() + { + return new int[]{ + TokenTypes.CLASS_DEF, + TokenTypes.INTERFACE_DEF, + TokenTypes.METHOD_DEF, + TokenTypes.CTOR_DEF, + TokenTypes.INSTANCE_INIT, + TokenTypes.STATIC_INIT, + TokenTypes.PACKAGE_DEF, + TokenTypes.IMPORT, + TokenTypes.VARIABLE_DEF, + TokenTypes.CTOR_CALL, + TokenTypes.SUPER_CTOR_CALL, + TokenTypes.LITERAL_IF, + TokenTypes.LITERAL_ELSE, + TokenTypes.LITERAL_WHILE, + TokenTypes.LITERAL_DO, + TokenTypes.LITERAL_FOR, + TokenTypes.LITERAL_SWITCH, + TokenTypes.LITERAL_BREAK, + TokenTypes.LITERAL_CONTINUE, + TokenTypes.LITERAL_RETURN, + TokenTypes.LITERAL_THROW, + TokenTypes.LITERAL_SYNCHRONIZED, + TokenTypes.LITERAL_CATCH, + TokenTypes.LITERAL_FINALLY, + TokenTypes.EXPR, + TokenTypes.LABELED_STAT, + TokenTypes.LITERAL_CASE, + TokenTypes.LITERAL_DEFAULT, + }; + } + + /** + * @see com.puppycrawl.tools.checkstyle.api.Check#getRequiredTokens() + */ + public int[] getRequiredTokens() + { + return new int[]{ + TokenTypes.CLASS_DEF, + TokenTypes.INTERFACE_DEF, + TokenTypes.METHOD_DEF, + TokenTypes.CTOR_DEF, + TokenTypes.INSTANCE_INIT, + TokenTypes.STATIC_INIT, + TokenTypes.PACKAGE_DEF, + TokenTypes.IMPORT, + TokenTypes.VARIABLE_DEF, + TokenTypes.CTOR_CALL, + TokenTypes.SUPER_CTOR_CALL, + TokenTypes.LITERAL_IF, + TokenTypes.LITERAL_ELSE, + TokenTypes.LITERAL_WHILE, + TokenTypes.LITERAL_DO, + TokenTypes.LITERAL_FOR, + TokenTypes.LITERAL_SWITCH, + TokenTypes.LITERAL_BREAK, + TokenTypes.LITERAL_CONTINUE, + TokenTypes.LITERAL_RETURN, + TokenTypes.LITERAL_THROW, + TokenTypes.LITERAL_SYNCHRONIZED, + TokenTypes.LITERAL_CATCH, + TokenTypes.LITERAL_FINALLY, + TokenTypes.EXPR, + TokenTypes.LABELED_STAT, + TokenTypes.LITERAL_CASE, + TokenTypes.LITERAL_DEFAULT, + }; + } + + /** + * @see com.puppycrawl.tools.checkstyle.api.Check + */ + public void beginTree(DetailAST aRootAST) + { + mCounters = new Stack(); + + //add a counter for the file + mCounters.push(new Counter()); + } + + /** + * @see com.puppycrawl.tools.checkstyle.api.Check + */ + public void visitToken(DetailAST aAST) + { + int tokenType = aAST.getType(); + + if (TokenTypes.CLASS_DEF == tokenType + || TokenTypes.METHOD_DEF == tokenType + || TokenTypes.CTOR_DEF == tokenType + || TokenTypes.STATIC_INIT == tokenType + || TokenTypes.INSTANCE_INIT == tokenType) + { + //add a counter for this class/method + mCounters.push(new Counter()); + } + + //check if token is countable + if (isCountable(aAST)) { + //increment the stacked counters + int size = mCounters.size(); + for (int i = 0; i < size; i++) { + ((Counter) mCounters.get(i)).increment(); + } + } + } + + /** + * @see com.puppycrawl.tools.checkstyle.api.Check + */ + public void leaveToken(DetailAST aAST) + { + int tokenType = aAST.getType(); + if (TokenTypes.METHOD_DEF == tokenType + || TokenTypes.CTOR_DEF == tokenType + || TokenTypes.STATIC_INIT == tokenType + || TokenTypes.INSTANCE_INIT == tokenType) + { + //pop counter from the stack + Counter counter = (Counter) mCounters.pop(); + + int count = counter.getCount(); + if (count > mMethodMax) { + log(aAST.getLineNo(), aAST.getColumnNo(), "ncss.method", + new Integer(count), new Integer(mMethodMax)); + } + } + else if (TokenTypes.CLASS_DEF == tokenType) { + //pop counter from the stack + Counter counter = (Counter) mCounters.pop(); + + int count = counter.getCount(); + if (count > mClassMax) { + log(aAST.getLineNo(), aAST.getColumnNo(), "ncss.class", + new Integer(count), new Integer(mClassMax)); + } + } + } + + /** + * @see com.puppycrawl.tools.checkstyle.api.Check + */ + public void finishTree(DetailAST aRootAST) + { + //pop counter from the stack + Counter counter = (Counter) mCounters.pop(); + + int count = counter.getCount(); + if (count > mFileMax) { + log(aRootAST.getLineNo(), aRootAST.getColumnNo(), "ncss.file", + new Integer(count), new Integer(mMethodMax)); + } + } + + /** + * Sets the maximum ncss for a file. + * + * @param aFileMax + * the maximum ncss + */ + public void setFileMaximum(int aFileMax) + { + mFileMax = aFileMax; + } + + /** + * Sets the maximum ncss for a class. + * + * @param aClassMax + * the maximum ncss + */ + public void setClassMaximum(int aClassMax) + { + mClassMax = aClassMax; + } + + /** + * Sets the maximum ncss for a method. + * + * @param aMethodMax + * the maximum ncss + */ + public void setMethodMaximum(int aMethodMax) + { + mMethodMax = aMethodMax; + } + + /** + * Checks if a token is countable for the ncss metric + * + * @param aAST + * the AST + * @return true if the token is countable + */ + private boolean isCountable(DetailAST aAST) + { + boolean countable = true; + + int tokenType = aAST.getType(); + + //check if an expression is countable + if (TokenTypes.EXPR == tokenType) { + countable = isExpressionCountable(aAST); + } + //check if an variable definition is countable + else if (TokenTypes.VARIABLE_DEF == tokenType) { + countable = isVariableDefCountable(aAST); + } + return countable; + } + + /** + * Checks if a variable definition is countable. + * + * @param aAST the AST + * @return true if the variable definition is countable, false otherwise + */ + private boolean isVariableDefCountable(DetailAST aAST) + { + boolean countable = false; + + //count variable defs only if they are direct child to a slist or + // object block + int parentType = aAST.getParent().getType(); + + if (TokenTypes.SLIST == parentType + || TokenTypes.OBJBLOCK == parentType) + { + DetailAST prevSibling = aAST.getPreviousSibling(); + + //is countable if no previous sibling is found or + //the sibling is no COMMA. + //This is done because multiple assignment on one line are countes + // as 1 + countable = prevSibling != null ? TokenTypes.COMMA != prevSibling + .getType() : true; + } + + return countable; + } + + /** + * Checks if an expression is countable for the ncss metric. + * + * @param aAST the AST + * @return true if the expression is countable, false otherwise + */ + private boolean isExpressionCountable(DetailAST aAST) + { + boolean countable = true; + + //count expressions only if they are direct child to a slist (method + // body, for loop...) + //or direct child of label,if,else,do,while,for + int parentType = aAST.getParent().getType(); + switch (parentType) { + case TokenTypes.SLIST : + case TokenTypes.LABELED_STAT : + case TokenTypes.LITERAL_FOR : + case TokenTypes.LITERAL_DO : + case TokenTypes.LITERAL_WHILE : + case TokenTypes.LITERAL_IF : + case TokenTypes.LITERAL_ELSE : + //don't count if or loop conditions + DetailAST prevSibling = aAST.getPreviousSibling(); + countable = prevSibling == null + || TokenTypes.LPAREN != prevSibling.getType(); + break; + default : + countable = false; + break; + } + return countable; + } + + /** + * @author Lars Ködderitzsch + * + * Class representing a counter, + */ + private class Counter + { + /** the counters internal integer */ + private int mIvCount; + + /** + * Increments the counter. + */ + public void increment() + { + mIvCount++; + } + + /** + * Gets the counters value + * + * @return the counter + */ + public int getCount() + { + return mIvCount; + } + } +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/metrics/messages.properties b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/metrics/messages.properties index dfac2be79..d3cf677db 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/metrics/messages.properties +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/metrics/messages.properties @@ -25,3 +25,6 @@ returnFromCatch=Return from catch is not allowed. returnFromFinally=Return from finally is not allowed. throwsCount=Throws count is {0,number,integer} (max allowed is {1,number,integer}). unusedVariable=Variable ''{0}'' is never used. +ncss.method=NCSS for this method is {0,number,integer} (max allowed is {1,number,integer}). +ncss.class=NCSS for this class is {0,number,integer} (max allowed is {1,number,integer}). +ncss.file=NCSS for this file is {0,number,integer} (max allowed is {1,number,integer}). diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/metrics/JavaNCSSCheckTestInput.java b/src/testinputs/com/puppycrawl/tools/checkstyle/metrics/JavaNCSSCheckTestInput.java new file mode 100755 index 000000000..e0fc96d1d --- /dev/null +++ b/src/testinputs/com/puppycrawl/tools/checkstyle/metrics/JavaNCSSCheckTestInput.java @@ -0,0 +1,78 @@ +// should give an ncss of 35 +package com.puppycrawl.tools.checkstyle.metrics; + +import java.awt.event.ItemEvent; +import java.awt.event.ItemListener; + + +//should give an ncss of 22 +public class JavaNCSSCheckTestInput { + + private Object mObject; + + //should count as 2 + private void testMethod1() { + + //should count as 1 + int x = 1, y = 2; + } + + //should count as 4 + private void testMethod2() { + + int abc = 0; + + //should count as 2 + testLabel: abc = 1; + } + + //should give an ncss of 12 + private void testMethod3() { + + int a = 0; + switch (a) { + case 1: //falls through + case 2: System.out.println("Hello"); break; + default: break; + } + + ItemListener lis = new ItemListener() { + + //should give an ncss of 2 + public void itemStateChanged(ItemEvent e) { + System.out.println("Hello"); + } + }; + } + + //should give an ncss of 2 + private class TestInnerClass { + + private Object test; + } +} + +//should give an ncss of 10 +class TestTopLevelNestedClass { + + private Object mObject; + + //should give an ncss of 8 + private void testMethod() { + + for (int i=0; i<10; i++) { + + if (i==0) { + + //should count as 1 + int x = 1, y = 2; + } + else { + int abc = 0; + + //should count as 2 + testLabel: abc = 1; + } + } + } +} diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/metrics/AllTests.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/metrics/AllTests.java index 23c566019..50043f85e 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/checks/metrics/AllTests.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/metrics/AllTests.java @@ -17,6 +17,7 @@ public class AllTests { suite.addTest(new TestSuite(ClassDataAbstractionCouplingCheckTest.class)); suite.addTest(new TestSuite(CyclomaticComplexityCheckTest.class)); suite.addTest(new TestSuite(NPathComplexityCheckTest.class)); + suite.addTest(new TestSuite(JavaNCSSCheckTest.class)); return suite; } diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/metrics/JavaNCSSCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/metrics/JavaNCSSCheckTest.java new file mode 100755 index 000000000..b1b31a079 --- /dev/null +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/metrics/JavaNCSSCheckTest.java @@ -0,0 +1,36 @@ +package com.puppycrawl.tools.checkstyle.checks.metrics; + +import java.io.File; + +import com.puppycrawl.tools.checkstyle.BaseCheckTestCase; +import com.puppycrawl.tools.checkstyle.DefaultConfiguration; + +/** + * Testcase for the JavaNCSS-Check. + * + * @author Lars Ködderitzsch + */ +public class JavaNCSSCheckTest extends BaseCheckTestCase { + + public void test() throws Exception { + DefaultConfiguration checkConfig = createCheckConfig(JavaNCSSCheck.class); + + checkConfig.addAttribute("methodMaximum", "0"); + checkConfig.addAttribute("classMaximum", "0"); + checkConfig.addAttribute("fileMaximum", "0"); + + String[] expected = { + "2:1: NCSS for this file is 35 (max allowed is 0).", + "9:1: NCSS for this class is 22 (max allowed is 0).", + "14:5: NCSS for this method is 2 (max allowed is 0).", + "21:5: NCSS for this method is 4 (max allowed is 0).", + "30:5: NCSS for this method is 12 (max allowed is 0).", + "42:13: NCSS for this method is 2 (max allowed is 0).", + "49:5: NCSS for this class is 2 (max allowed is 0).", + "56:1: NCSS for this class is 10 (max allowed is 0).", + "61:5: NCSS for this method is 8 (max allowed is 0).",}; + + verify(checkConfig, getPath("metrics" + File.separator + + "JavaNCSSCheckTestInput.java"), expected); + } +}