Rule to work out simple complexity measure for functions

src/main/java/com/cflint/plugins/core/SimpleComplexityChecker.java

@@ -0,0 +1,93 @@
+package com.cflint.plugins.core;
+
+import net.htmlparser.jericho.Element;
+import cfml.parsing.cfscript.CFFunctionExpression;
+import cfml.parsing.cfscript.script.CFIfStatement;
+import cfml.parsing.cfscript.script.CFForStatement;
+import cfml.parsing.cfscript.script.CFForInStatement;
+import cfml.parsing.cfscript.script.CFSwitchStatement;
+import cfml.parsing.cfscript.script.CFTryCatchStatement;
+import cfml.parsing.cfscript.script.CFCompDeclStatement;
+import cfml.parsing.cfscript.script.CFFuncDeclStatement;
+import cfml.parsing.cfscript.script.CFFunctionParameter;
+import cfml.parsing.cfscript.script.CFScriptStatement;
+import cfml.parsing.cfscript.script.CFWhileStatement;
+import cfml.parsing.cfscript.script.CFDoWhileStatement;
+
+import com.cflint.BugInfo;
+import com.cflint.BugList;
+import com.cflint.plugins.CFLintScannerAdapter;
+import com.cflint.plugins.Context;
+import com.cflint.tools.CFTool;
+
+public class SimpleComplexityChecker extends CFLintScannerAdapter {
+	final String severity = "WARNING";
+	final protected int COMPLEXITY_THRESHOLD = 10;
+
+	protected int complexity = 0;
+	protected boolean alreadyTooComplex = false;
+	int functionLineNo = 1;
+
+	@Override
+	public void expression(final CFScriptStatement expression, final Context context, final BugList bugs) {
+		CFFuncDeclStatement function = null;
+
+		if (expression instanceof CFFuncDeclStatement) {
+			function = (CFFuncDeclStatement) expression;
+			functionLineNo = function.getLine();
+			complexity = 0;
+			alreadyTooComplex = false;
+		}
+		// Not using instanceof to avoid double counting
+		else if (expression.getClass().equals(CFIfStatement.class) ||
+			expression.getClass().equals(CFForStatement.class) ||
+			expression.getClass().equals(CFForInStatement.class) ||
+			expression.getClass().equals(CFSwitchStatement.class) ||
+			expression.getClass().equals(CFTryCatchStatement.class) ||
+			expression.getClass().equals(CFWhileStatement.class) ||
+			expression.getClass().equals(CFDoWhileStatement.class)) {
+			complexity++;
+			// TODO +1 for each case statment in a switch
+			checkComplexity(context.getFunctionName(), functionLineNo, context, bugs);
+		}
+	}
+
+	@Override
+	public void element(final Element element, final Context context, final BugList bugs) {
+		final String name = element.getName();
+
+		if (name.equalsIgnoreCase("cffunction")) {
+			functionLineNo = element.getSource().getRow(element.getBegin());
+			complexity = 0;
+			alreadyTooComplex = false;
+		}
+		else {
+			if (name.equalsIgnoreCase("cfif") || name.equalsIgnoreCase("cfelse") || name.equalsIgnoreCase("cfelseif")
+				|| name.equalsIgnoreCase("cfloop")  || name.equalsIgnoreCase("cfwhile") || name.equalsIgnoreCase("cfoutput") // TODO could check for query=
+				|| name.equalsIgnoreCase("cfcase") || name.equalsIgnoreCase("cfdefaultcase")
+				|| name.equalsIgnoreCase("cftry") || name.equalsIgnoreCase("cfcatch")) {
+				complexity++;
+				checkComplexity(context.getFunctionName(), functionLineNo, context, bugs);
+			}
+		}
+	}
+
+	protected void checkComplexity(String name, int lineNo, Context context, BugList bugs) {
+		String complexityThreshold = getParameter("maximum");
+		int threshold = COMPLEXITY_THRESHOLD;
+
+		if (complexityThreshold != null) {
+			threshold = Integer.parseInt(complexityThreshold);
+		}
+
+		if (!alreadyTooComplex && complexity > threshold) {
+			alreadyTooComplex = true;
+
+			bugs.add(new BugInfo.BugInfoBuilder().setLine(lineNo).setMessageCode("FUNCTION_TOO_COMPLEX")
+				.setSeverity(severity).setFilename(context.getFilename())
+				.setMessage("Function " + name + " is too complex. Consider breaking the function into smaller functions.")
+				.build());
+		}
+	}
+
+}

src/main/resources/cflint.definition.xml

@@ -155,12 +155,18 @@
 		<message code="EXCESSIVE_ARGUMENTS">
 			<severity>WARNING</severity>
 		</message>
-		<parameter name="maximum" />
+		<parameter name="maximum" value="10" />
 	</ruleImpl>
 	<ruleImpl name="TooManyFunctionsChecker" className="TooManyArgumentsChecker">
 		<message code="EXCESSIVE_FUNCTIONS">
 			<severity>WARNING</severity>
 		</message>
-		<parameter name="maximum" />
+		<parameter name="maximum" value="10" />
+	</ruleImpl>
+	<ruleImpl name="SimpleComplexityChecker" className="SimpleComplexityChecker">
+		<message code="FUNCTION_TOO_COMPLEX">
+			<severity>WARNING</severity>
+		</message>
+		<parameter name="maximum" value="10" />
 	</ruleImpl>
 </CFLint-Plugin>

src/test/java/com/cflint/TestCFBugs_SimpleComplexity.java

@@ -0,0 +1,201 @@
+package com.cflint;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import cfml.parsing.reporting.ParseException;
+
+import com.cflint.config.CFLintPluginInfo.PluginInfoRule;
+import com.cflint.config.CFLintPluginInfo.PluginInfoRule.PluginMessage;
+import com.cflint.config.ConfigRuntime;
+import com.cflint.plugins.core.SimpleComplexityChecker;
+
+public class TestCFBugs_SimpleComplexity {
+
+	private CFLint cfBugs;
+
+	@Before
+	public void setUp() {
+		final ConfigRuntime conf = new ConfigRuntime();
+		final PluginInfoRule pluginRule = new PluginInfoRule();
+		pluginRule.setName("SimpleComplexityChecker");
+		conf.getRules().add(pluginRule);
+		PluginMessage pluginMessage = new PluginMessage("FUNCTION_TOO_COMPLEX");
+		pluginMessage.setSeverity("WARNING");
+		pluginRule.getMessages().add(pluginMessage);
+
+		cfBugs = new CFLint(conf, new SimpleComplexityChecker());
+	}
+
+	@Test
+	public void testNotComplexTagBased() throws ParseException, IOException {
+		final String cfcSrc = "<cfcomponent>\r\n"
+		 + "<cffunction name=\"functionOne\">\r\n"
+		 + "<cfif a is 1>\r\n"
+		 + "<cfset b = 1>\r\n"
+		 + "<cfelse>\r\n"
+		 + "<cfset b = 2>\r\n"
+		 + "</cfif>\r\n"
+		 + "</cffunction>\r\n"
+		 + "</cfcomponent>";
+		cfBugs.process(cfcSrc, "test");
+		Collection<List<BugInfo>> result = cfBugs.getBugs().getBugList().values();
+		assertEquals(0, result.size());
+	}
+
+	@Test
+	public void testComplexTagBased() throws ParseException, IOException {
+		final String cfcSrc = "<cfcomponent>\r\n"
+		 + "<cffunction name=\"functionTwo\">\r\n"
+		 + "<cfif a is 1>\r\n"
+		 + "<cfset b = 1>\r\n"
+		 + "<cfelseif a is 2>\r\n"
+		 + "<cfset b = 2>\r\n"
+		 + "<cfelseif a is 3>\r\n"
+		 + "<cfset b = 3>\r\n"
+		 + "<cfelseif a is 4>\r\n"
+		 + "<cfset b = 4>\r\n"
+		 + "<cfelseif a is 5>\r\n"
+		 + "<cfset b = 5>\r\n"
+		 + "<cfelseif a is 6>\r\n"
+		 + "<cfset b = 6>\r\n"
+		 + "<cfelseif a is 7>\r\n"
+		 + "<cfset b = 7>\r\n"
+		 + "<cfelseif a is 8>\r\n"
+		 + "<cfset b = 8>\r\n"
+		 + "<cfelseif a is 9>\r\n"
+		 + "<cfset b = 9>\r\n"
+		 + "<cfelseif a is 10>\r\n"
+		 + "<cfset b = 10>\r\n"
+		 + "<cfelseif a is 11>\r\n"
+		 + "<cfset b = 11>\r\n"	 		 		 		 		 		 		 
+		 + "</cfif>\r\n"
+		 + "</cffunction>\r\n"
+		 + "</cfcomponent>";
+		cfBugs.process(cfcSrc, "test");
+		final List<BugInfo> result = cfBugs.getBugs().getBugList().values().iterator().next();
+		assertEquals(1, result.size());
+		assertEquals("FUNCTION_TOO_COMPLEX", result.get(0).getMessageCode());
+		assertEquals(2, result.get(0).getLine());
+	}
+
+	@Test
+	public void testNonComplexNameScriptBased() throws ParseException, IOException {
+		final String cfcSrc = "component {\r\n"
+		 + "function functionThree() {\r\n"
+		 + "if (a == 1) {"
+		 + "b = 1;\r\n"
+		 + "}\r\n"
+		 + "else {\r\n"
+		 + "b = 2;\r\n"	 		 		 		 		 		 		 
+		 + "}\r\n"
+		 + "}\r\n"
+		 + "}";
+		cfBugs.process(cfcSrc, "test");
+		Collection<List<BugInfo>> result = cfBugs.getBugs().getBugList().values();
+		assertEquals(0, result.size());
+	}
+
+	@Test
+	public void testComplexScriptBased() throws ParseException, IOException {
+		final String cfcSrc = "component {\r\n"
+		 + "function functionFour() {\r\n"
+		 + "if (a == 1) {"
+		 + "b = 1;\r\n"
+		 + "}\r\n"
+		 + "else if (a == 2) {\r\n"
+		 + "b = 2;\r\n" 		 		 		 		 		 		 
+		 + "}\r\n"
+		 + "else if (a == 3) {\r\n"
+		 + "b = 3;\r\n" 		 		 		 		 		 		 
+		 + "}\r\n"
+		 + "else if (a == 4) {\r\n"
+		 + "b = 4;\r\n" 		 		 		 		 		 		 
+		 + "}\r\n"
+		 + "else if (a == 5) {\r\n"
+		 + "b = 5;\r\n" 		 		 		 		 		 		 
+		 + "}\r\n"
+		 + "else if (a == 6) {\r\n"
+		 + "b = 6;\r\n" 		 		 		 		 		 		 
+		 + "}\r\n"
+		 + "else if (a == 7) {\r\n"
+		 + "b = 7;\r\n" 		 		 		 		 		 		 
+		 + "}\r\n"	
+		 + "else if (a == 8) {\r\n"
+		 + "b = 8;\r\n" 		 		 		 		 		 		 
+		 + "}\r\n"
+		 + "else if (a == 9) {\r\n"
+		 + "b = 9;\r\n" 		 		 		 		 		 		 
+		 + "}\r\n"
+		 + "else if (a == 10) {\r\n"
+		 + "b = 10;\r\n" 
+		 + "else if (a == 11) {\r\n"
+		 + "b = 11;\r\n" 		 		 		 		 		 		 
+		 + "}\r\n"	
+		 + "}\r\n" 		 		 		 
+		 + "}";
+		cfBugs.process(cfcSrc, "test");
+		final List<BugInfo> result = cfBugs.getBugs().getBugList().values().iterator().next();
+		assertEquals(1, result.size());
+		assertEquals("FUNCTION_TOO_COMPLEX", result.get(0).getMessageCode());
+		assertEquals(2, result.get(0).getLine());
+	}
+
+	@Test
+	public void testComplexSwitchScriptBased() throws ParseException, IOException {
+		final String cfcSrc = "component {\r\n"
+		 + "function functionFive() {\r\n"
+		 + "switch(a) {\r\n"
+		 + "case 1:\r\n"
+		 + "b = 1;\r\n"
+		 + "break;\r\n"
+		 + "case 2:\r\n"
+		 + "b = 2;\r\n"
+		 + "break;\r\n"
+		 + "case 3:\r\n"
+		 + "b = 3;\r\n"
+		 + "break;\r\n"
+		 + "case 4:\r\n"
+		 + "b = 4;\r\n"
+		 + "break;\r\n"
+		 + "case 5:\r\n"
+		 + "b = 5;\r\n"
+		 + "break;\r\n"
+		 + "case 6:\r\n"
+		 + "b = 6;\r\n"
+		 + "break;\r\n"
+		 + "case 7:\r\n"
+		 + "b = 7;\r\n"
+		 + "break;\r\n"
+		 + "case 8:\r\n"
+		 + "b = 8;\r\n"
+		 + "break;\r\n"
+		 + "case 9:\r\n"
+		 + "b = 9;\r\n"
+		 + "break;\r\n"
+		 + "case 10:\r\n"
+		 + "b = 10;\r\n"
+		 + "break;\r\n"
+		 + "case 11:\r\n"
+		 + "b = 11;\r\n"
+		 + "break;\r\n"		 	 		 		 		 		 		 
+		 + "}\r\n"	
+		 + "}\r\n" 		 		 		 
+		 + "}";
+		cfBugs.process(cfcSrc, "test");
+		Collection<List<BugInfo>> result = cfBugs.getBugs().getBugList().values();
+		assertEquals(0, result.size());
+		// TODO currently this is seen as simple code change test once fixed
+		//assertEquals("FUNCTION_TOO_COMPLEX", result.get(0).getMessageCode());
+		//assertEquals(1, result.get(0).getLine());
+	}
+
+
+}