Merge pull request #125 from cflint/testing

Introduce file driven tests.

build.gradle

@@ -55,3 +55,9 @@ dependencies {
     runtime group: 'org.slf4j', name: 'slf4j-api', version:'1.7.5'
     runtime group: 'org.javolution', name: 'javolution', version:'5.2.6'
 }
+
+test {
+    testLogging {
+        exceptionFormat = 'full'
+    }
+}

pom.xml

@@ -166,8 +166,6 @@
 		  <artifactId>jackson-module-jaxb-annotations</artifactId>
 		  <version>2.6.3</version>
 		</dependency>
-
-
 		<!-- dependency>
 			<groupId>com.googlecode.json-simple</groupId>
 			<artifactId>json-simple</artifactId>

src/main/java/com/cflint/CFLint.java

@@ -37,6 +37,7 @@
 import cfml.parsing.cfscript.CFIdentifier;
 import cfml.parsing.cfscript.CFLiteral;
 import cfml.parsing.cfscript.CFNestedExpression;
+import cfml.parsing.cfscript.CFStatement;
 import cfml.parsing.cfscript.CFUnaryExpression;
 import cfml.parsing.cfscript.CFVarDeclExpression;
 import cfml.parsing.cfscript.script.CFCompDeclStatement;
@@ -809,7 +810,13 @@ protected void reportRule(final Element elem, final Object expression, final Con
 			bldr.setSeverity("WARNING");
 			bldr.setMessage(msgcode);
 		}
-		if(elem != null){
+		if(expression instanceof CFStatement){
+			bldr.setExpression(((CFStatement) expression).Decompile(0));
+		}
+		else if(expression instanceof CFScriptStatement){
+			bldr.setExpression(((CFScriptStatement) expression).Decompile(0));
+		}
+		else if(elem != null){
 			bldr.setExpression(elem.toString());
 		}
 		bldr.setRuleParameters(ruleInfo.getParameters());

src/main/java/com/cflint/JSONOutput.java

@@ -18,12 +18,24 @@
 import com.fasterxml.jackson.core.JsonGenerator;
 
 public class JSONOutput {
+
+	boolean prettyPrint = true;
+
+	public boolean isPrettyPrint() {
+		return prettyPrint;
+	}
+
+	public void setPrettyPrint(boolean prettyPrint) {
+		this.prettyPrint = prettyPrint;
+	}
 
 	@SuppressWarnings("unchecked")
 	public void output(final BugList bugList, final Writer writer) throws IOException {
 		// final StringBuilder sb = new StringBuilder();
 		JsonFactory jsonF = new JsonFactory();
 		JsonGenerator jg = jsonF.createGenerator(writer);
+		if(prettyPrint)
+			jg.useDefaultPrettyPrinter();
 		jg.writeStartArray();
 		for (final Entry<String, List<BugInfo>> bugEntry : bugList.getBugList().entrySet()) {
 			final Iterator<BugInfo> iterator = bugEntry.getValue().iterator();

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

@@ -0,0 +1,57 @@
+package com.cflint.plugins.core;
+
+import java.util.Arrays;
+import java.util.List;
+
+import ro.fortsoft.pf4j.Extension;
+import net.htmlparser.jericho.Element;
+import cfml.CFSCRIPTLexer;
+import cfml.parsing.cfscript.CFBinaryExpression;
+import cfml.parsing.cfscript.CFExpression;
+import cfml.parsing.cfscript.script.CFExpressionStatement;
+import cfml.parsing.cfscript.script.CFScriptStatement;
+
+import com.cflint.BugInfo;
+import com.cflint.BugList;
+import com.cflint.plugins.CFLintScannerAdapter;
+import com.cflint.plugins.Context;
+
+@Extension
+public class CFCompareVsAssignChecker extends CFLintScannerAdapter {
+	final String severity = "INFO";
+
+	List<Integer> TOKENS = Arrays.asList(CFSCRIPTLexer.EQUALSEQUALSOP,
+			CFSCRIPTLexer.LT,
+			CFSCRIPTLexer.LTE,
+			CFSCRIPTLexer.GT,
+			CFSCRIPTLexer.GTE,
+			CFSCRIPTLexer.OR,
+			CFSCRIPTLexer.OROPERATOR,
+			CFSCRIPTLexer.EQV,
+			CFSCRIPTLexer.XOR,
+			CFSCRIPTLexer.AND,
+			CFSCRIPTLexer.ANDOPERATOR,
+			CFSCRIPTLexer.EQ,
+			CFSCRIPTLexer.NEQ,
+			CFSCRIPTLexer.CONTAINS);
+
+	@Override
+	public void expression(final CFScriptStatement expression, final Context context, final BugList bugs) {	
+		if(expression instanceof CFExpressionStatement){
+			final CFExpressionStatement exprStatement = (CFExpressionStatement)expression;
+			if(exprStatement.getExpression() instanceof CFBinaryExpression){
+				CFBinaryExpression binaryExpression = (CFBinaryExpression) exprStatement.getExpression();
+				if(TOKENS.contains(binaryExpression.getToken().getType())){
+					context.addMessage("COMPARE_INSTEAD_OF_ASSIGN", binaryExpression.getToken().getText());
+				}
+			}
+		}
+	}
+
+	protected void noNeedtoUseCreateObject(final int lineNo, final Context context, final BugList bugs) {
+		bugs.add(new BugInfo.BugInfoBuilder().setLine(lineNo).setMessageCode("AVOID_USING_CREATEOBJECT")
+			.setSeverity(severity).setFilename(context.getFilename())
+			.setMessage("CreateObject statement at line " + lineNo + ". Use createObject(path_to_component) or even better new path_to_component().")
+			.build());
+	}
+}

src/main/resources/cflint.definition.json

@@ -727,6 +727,18 @@
         }
       ],
       "parameter": []
+    },
+    {
+      "name": "CFCompareVsAssignChecker",
+      "className": "CFCompareVsAssignChecker",
+      "message": [
+        {
+          "code": "COMPARE_INSTEAD_OF_ASSIGN",
+           "messageText": "CWE-482: Comparing instead of Assigning",
+          "severity": "WARNING"
+        }
+      ],
+      "parameter": []
     }
   ]
 }

src/main/resources/cflint.definition.xml

@@ -334,4 +334,11 @@
 			<messageText>Avoid leaving debug attribute on tags.</messageText>
 		</message>
 	</ruleImpl>
+	<ruleImpl name="CFCompareVsAssignChecker" className="CFCompareVsAssignChecker">
+		<message code="COMPARE_INSTEAD_OF_ASSIGN">
+			<severity>WARNING</severity>
+			<messageText>CWE-482: Comparing instead of Assigning</messageText>
+		</message>
+	</ruleImpl>
+
 </CFLint-Plugin>

src/test/java/com/cflint/TestCFBugs_ComponentNames.java

@@ -159,7 +159,7 @@ public void nameHasPrefixOrPostfixTag() throws ParseException, IOException {
 		final String tagSrc = "<cfcomponent>\r\n"
 		 + "</cfcomponent>";
 		cfBugs.process(tagSrc, "MyComp.cfc");
-		final List<BugInfo> result = cfBugs.getBugs().getBugList().values().iterator().next();
+		final List<BugInfo> result = cfBugs.getBugs().getBugList().get("COMPONENT_HAS_PREFIX_OR_POSTFIX");
 		assertEquals(1, result.size());
 		assertEquals("COMPONENT_HAS_PREFIX_OR_POSTFIX", result.get(0).getMessageCode());
 		assertEquals(1, result.get(0).getLine());

src/test/java/com/cflint/TestJSONOutput.java

@@ -27,6 +27,7 @@ public class TestJSONOutput {
 	@Before
 	public void setUp(){
 		outputer = new JSONOutput();
+		outputer.setPrettyPrint(false);
 		bugList = new BugList(null);
 		writer = new StringWriter();
 	}

src/test/java/com/cflint/integration/TestFiles.java

@@ -0,0 +1,180 @@
+package com.cflint.integration;
+
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.ResourceBundle;
+
+import javax.xml.bind.JAXBException;
+import javax.xml.transform.TransformerException;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import com.cflint.BugInfo;
+import com.cflint.CFLint;
+import com.cflint.JSONOutput;
+import com.cflint.config.CFLintConfig;
+import com.cflint.config.ConfigUtils;
+
+
+/**
+ * Run a test over each *.cf* file in src/test/resources/com/cflint/tests
+ * 
+ * @author ryaneberly
+ *
+ */
+@RunWith(Parameterized.class)
+public class TestFiles {
+
+	File sourceFile;
+
+	boolean autoReplaceFailed = false;
+	static String singleTestName = null;
+	static {
+		try {
+			singleTestName = ResourceBundle.getBundle("com.cflint.test").getString("RunSingleTest");
+		} catch (Exception e) {
+		}
+	}
+
+	public TestFiles(File sourceFile,String testName) {
+		super();
+		this.sourceFile = sourceFile;
+		try {
+			autoReplaceFailed = "Y".equalsIgnoreCase(ResourceBundle.getBundle("com.cflint.test").getString(
+					"AutoReplaceFailedTestResults"));
+		} catch (Exception e) {
+		}
+	}
+
+	@Test
+	public void test() throws IOException, URISyntaxException, JAXBException, TransformerException {
+		final String inputString = loadFile(sourceFile);
+		final File expectedFile = new File(sourceFile.getPath().replaceAll("\\.cf.", ".expected.txt"));
+		final String expectedFileText = expectedFile.exists() ? loadFile(expectedFile) : null;
+		String expectedText = expectedFileText ;
+
+		final CFLintConfig config = loadPluginInfo(sourceFile.getParentFile());
+		CFLint cflint = new CFLint(config );
+
+		cflint.process(inputString, sourceFile.getPath());
+		List<BugInfo> result = cflint.getBugs().getFlatBugList();
+		StringWriter writer = new StringWriter();
+		new JSONOutput().output(cflint.getBugs(), writer);
+
+		String actualTree=writer.toString();
+		if (expectedText == null || expectedText.trim().length() == 0) {
+			writeExpectFile(expectedFile,  actualTree);
+			System.out.println("Tree written to " + expectedFile);
+		} else {
+			if (autoReplaceFailed && !actualTree.equals(expectedText)) {
+				System.out.println("Replaced content of " + expectedFile);
+				expectedText = actualTree;
+				writeExpectFile(expectedFile, actualTree);
+			}
+			assertEquals(actualTree.replaceAll("\\\\","/").replaceAll("/+","/").replaceAll("\r\n", "\n"), 
+					expectedText.replaceAll("\\\\","/").replaceAll("/+","/").replaceAll("\r\n", "\n"));
+		}
+	}
+
+	private void writeExpectFile(File expectedFile, String actualTree) throws IOException {
+		final FileOutputStream fos = new FileOutputStream(expectedFile, false);
+		fos.write(actualTree.getBytes());
+		fos.close();
+
+	}
+
+
+
+
+	private String getTree(String expectedFileText) {
+		if (expectedFileText != null && expectedFileText.contains("/*===TREE===*/")) {
+			int startIdx = expectedFileText.indexOf("/*===TREE===*/") + 14;
+			while (expectedFileText.charAt(startIdx) == '\r' || expectedFileText.charAt(startIdx) == '\n') {
+				startIdx++;
+			}
+			int endIndex = expectedFileText.indexOf("/*======*/", startIdx);
+			if (endIndex > startIdx) {
+				while (expectedFileText.charAt(endIndex - 1) == '\r' || expectedFileText.charAt(endIndex - 1) == '\n') {
+					endIndex--;
+				}
+				return expectedFileText.substring(startIdx, endIndex);
+			}
+		}
+		if (expectedFileText != null && expectedFileText.contains("/*===")) {
+			return null;
+		}
+		return expectedFileText;
+	}
+
+	@Parameterized.Parameters(name = "{1}")
+	public static Collection<Object[]> primeNumbers() throws URISyntaxException, IOException {
+		final ArrayList<Object[]> retval = new ArrayList<Object[]>();
+		final List<File> listing = new ArrayList<File>();
+		final File baseFolder = new File("src/test/resources/com/cflint/tests");
+		fillResourceListing(baseFolder, listing);
+		for (File s : listing) {
+			retval.add(new Object[] { s, baseFolder.toPath().relativize(s.toPath() ).toString() });
+		}
+		return retval;
+	}
+
+	private static void fillResourceListing(File file, List<File> retval) {
+		if (file != null) {
+			if (file.isDirectory()) {
+				for (File subfile : file.listFiles()) {
+					fillResourceListing(subfile, retval);
+				}
+			} else if (file.getName().toLowerCase().endsWith(".cfc") || file.getName().toLowerCase().endsWith(".cfm")) {
+				if (singleTestName == null || singleTestName.equals(file.getName())) {
+					retval.add(file);
+				} else if (singleTestName.equals("*LAST")) {
+					if (retval.size() == 0 || file.lastModified() > retval.get(0).lastModified()) {
+						retval.clear();
+						retval.add(file);
+					}
+				}
+			}
+		}
+	}
+	public static String loadFile(File file) throws IOException {
+		InputStream is = new FileInputStream(file);
+		byte[] b = new byte[is.available()];
+		is.read(b);
+		is.close();
+		return new String(b);
+	}
+
+	public static CFLintConfig loadPluginInfo(File folder) throws IOException {
+		try{
+		final InputStream jsonInputStream = new FileInputStream(folder.getPath() + "/.cflintrc");
+			if (jsonInputStream != null) {
+				return ConfigUtils.unmarshalJson(jsonInputStream, CFLintConfig.class);
+			}
+		}catch(FileNotFoundException fnfe){}
+
+		final InputStream inputStream = new FileInputStream(folder.getPath() + "/.cflintrc.xml");
+		if (inputStream != null) {
+			try {
+				return ConfigUtils.unmarshal(inputStream, CFLintConfig.class);
+			} catch (JAXBException e) {
+				throw new IOException(e);
+			}
+		}
+		return null;
+	}
+
+}

src/test/resources/com/cflint/test.properties

@@ -0,0 +1,4 @@
+#Change the AutoReplaceFailedTestResults to Y to pass the test and replace the expected results
+AutoReplaceFailedTestResults=N
+#Uncomment the following line to run only the last updated test file.
+#RunSingleTest=*LAST

src/test/resources/com/cflint/tests/CompareInsteadOfAssign/.cflintrc.xml

@@ -0,0 +1 @@
+<config><includes code="COMPARE_INSTEAD_OF_ASSIGN"/></config>

src/test/resources/com/cflint/tests/CompareInsteadOfAssign/Compare1.cfm

@@ -0,0 +1,10 @@
+<!--- contains comparison operations that don't 'do' anything --->
+<cfscript>
+x = {};
+x.x = 1;
+x.x == 2;
+if (x.x == 2){
+  x=123;
+}
+x.x EQ 6;
+</cfscript>