Merge pull request #92 from cflint/booleanExpression

Check for explicit boolean expression checking for true and false

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

@@ -0,0 +1,68 @@
+package com.cflint.plugins.core;
+
+import net.htmlparser.jericho.Element;
+import cfml.parsing.cfscript.CFExpression;
+import cfml.parsing.cfscript.CFBinaryExpression;
+import cfml.parsing.cfscript.script.CFScriptStatement;
+import cfml.parsing.cfscript.script.CFIfStatement;
+import cfml.parsing.cfscript.script.CFExpressionStatement;
+
+import com.cflint.BugInfo;
+import com.cflint.BugList;
+import com.cflint.plugins.CFLintScannerAdapter;
+import com.cflint.plugins.Context;
+
+public class BooleanExpressionChecker extends CFLintScannerAdapter {
+	final String severity = "INFO";
+
+	protected int lastLineNo = -1;
+
+	@Override	
+	public void expression(final CFExpression expression, final Context context, final BugList bugs) {
+		if (expression instanceof CFBinaryExpression) {
+			String code = expression.Decompile(0).toLowerCase();
+
+			if (hasExplicitBooleanCheck(code)) {
+				int lineNo = ((CFBinaryExpression) expression).getLine() + context.startLine() - 1;
+
+				// Only report issue once per line
+				if (lastLineNo != lineNo) {
+					booleanExpression(lineNo, context, bugs);
+					lastLineNo = lineNo;
+				}
+			}
+		}
+	}
+
+	@Override
+	public void element(final Element element, final Context context, final BugList bugs) {
+		String tag = element.getName();
+
+		if (tag.equals("cfif")) { 
+			String content = element.getStartTag().getTagContent().toString();
+
+			if (hasExplicitBooleanCheck(content)) {
+				int lineNo = element.getSource().getRow(element.getBegin());
+
+				// Only report issue once per line
+				if (lastLineNo != lineNo) {
+					booleanExpression(lineNo, context, bugs);
+					lastLineNo = lineNo;
+				}
+			}
+		}
+	}
+
+	protected boolean hasExplicitBooleanCheck(final String code) {
+		return code.contains("== true") || code.contains("eq true") || code.contains("is true") || code.contains("!= true")
+			|| code.contains("== false") || code.contains("eq false") || code.contains("is false") || code.contains("!= false");
+	}
+
+	public void booleanExpression(final int lineNo, final Context context, final BugList bugs) {
+		bugs.add(new BugInfo.BugInfoBuilder().setLine(lineNo).setMessageCode("EXPLICIT_BOOLEAN_CHECK")
+			.setSeverity(severity).setFilename(context.getFilename())
+			.setMessage("Explicit check of boolean expession at " + lineNo + " is not needed.")
+			.build());
+	}
+
+}

src/main/resources/cflint.definition.xml

@@ -198,4 +198,9 @@
 			<severity>WARNING</severity>
 		</message>
 	</ruleImpl>
+	<ruleImpl name="BooleanExpressionChecker" className="BooleanExpressionChecker">
+		<message code="EXPLICIT_BOOLEAN_CHECK">
+			<severity>INFO</severity>
+		</message>
+	</ruleImpl>
 </CFLint-Plugin>

src/test/java/com/cflint/TestBooleanExpressionChecker.java

@@ -0,0 +1,65 @@
+package com.cflint;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import java.util.List;
+
+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.BooleanExpressionChecker;
+
+public class TestBooleanExpressionChecker {
+
+	private CFLint cfBugs;
+
+	@Before
+	public void setUp() {
+		final ConfigRuntime conf = new ConfigRuntime();
+		final PluginInfoRule pluginRule = new PluginInfoRule();
+		pluginRule.setName("BooleanExpressionChecker");
+		conf.getRules().add(pluginRule);
+		final PluginMessage pluginMessage = new PluginMessage("EXPLICIT_BOOLEAN_CHECK");
+		pluginMessage.setSeverity("INFO");
+		cfBugs = new CFLint(conf, new BooleanExpressionChecker());
+	}
+
+	@Test
+	public void testBooleanExpressionInScript() throws ParseException, IOException {
+		final String scriptSrc = "<cfscript>\r\n"
+			+ "if (a && b == true) {\r\n"
+			+ "	c = 1;\r\n"
+			+ "}\r\n"
+			+ "else if (a or b is false) {\r\n"
+			+ "	c = 1;\r\n"
+			+ "}\r\n"
+			+ "</cfscript>";
+
+		cfBugs.process(scriptSrc, "test");
+		final List<BugInfo> result = cfBugs.getBugs().getBugList().values().iterator().next();
+		assertEquals(2, result.size());
+		assertEquals("EXPLICIT_BOOLEAN_CHECK", result.get(0).getMessageCode());
+		assertEquals(2, result.get(0).getLine());
+		assertEquals("EXPLICIT_BOOLEAN_CHECK", result.get(1).getMessageCode());
+		assertEquals(5, result.get(1).getLine());
+	}
+
+	@Test
+	public void testBooleanExpressionInTag() throws ParseException, IOException {
+		final String tagSrc = "<cfset a = 23>\r\n"
+			+ "<cfset a = not (b and c) is false>";
+
+		cfBugs.process(tagSrc, "test");
+		final List<BugInfo> result = cfBugs.getBugs().getBugList().values().iterator().next();
+		assertEquals(1, result.size());
+		assertEquals("EXPLICIT_BOOLEAN_CHECK", result.get(0).getMessageCode());
+		assertEquals(2, result.get(0).getLine());
+	}
+
+}