Merge pull request #167 from cflint/dev

merge Dev
cflint · May 10, 2016 · 2764cf7 · 2764cf7
2 parents 8aeacd9 + e8a616b
commit 2764cf7
Show file tree

Hide file tree

Showing 10 changed files with 83 additions and 78 deletions.
diff --git a/build.gradle b/build.gradle
@@ -42,8 +42,9 @@ dependencies {
     compile group: 'org.antlr', name: 'antlr4-runtime', version:'4.5.2-1'
     compile group: 'org.antlr', name: 'stringtemplate', version:'4.0.2'
     compile group: 'org.antlr', name: 'antlr4', version:'4.5.2-1'
-    compile group: 'net.htmlparser.jericho', name: 'jericho-html', version:'3.3'
+    compile group: 'net.htmlparser.jericho', name: 'jericho-html', version:'3.4'
     compile group: 'log4j', name: 'log4j', version:'1.2.17'
+    compile group: 'org.apache.logging.log4j', name: 'log4j-core', version:'2.5'
     compile group: 'commons-cli', name: 'commons-cli', version:'1.2'
     compile group: 'ro.fortsoft.pf4j', name: 'pf4j', version:'0.6'
     compile group: 'org.slf4j', name: 'slf4j-log4j12', version:'1.7.5'

diff --git a/pom.xml b/pom.xml
@@ -106,8 +106,7 @@
 		<dependency>
 			<groupId>net.htmlparser.jericho</groupId>
 			<artifactId>jericho-html</artifactId>
-			<version>3.3</version>
-			<type>jar</type>
+			<version>3.4</version>
 			<!-- scope>runtime</scope-->
 		</dependency>
 		<dependency>
@@ -123,6 +122,11 @@
 			<version>1.2.17</version>
 			<type>jar</type>
 		</dependency>
+ 		<dependency>
+ 			<groupId>org.apache.logging.log4j</groupId>
+ 			<artifactId>log4j-core</artifactId>
+ 			<version>2.5</version>
+ 		</dependency>		
 		<dependency>
 			<groupId>org.javolution</groupId>
 			<artifactId>javolution</artifactId>

diff --git a/src/main/java/com/cflint/JSONOutput.java b/src/main/java/com/cflint/JSONOutput.java
@@ -49,8 +49,10 @@ public void output(final BugList bugList, final Writer writer, final boolean sho
 				jg.writeStringField("message", code);
 				jg.writeStringField("category", "CFLINT");
 				jg.writeStringField("abbrev", abbrev(code));
+				jg.writeFieldName("locations");
+				jg.writeStartArray();
 				do {
-					jg.writeObjectFieldStart("location");
+					jg.writeStartObject();
 					jg.writeStringField("file",notNull(bugInfo.getFilename()));
 					jg.writeStringField("fileName",filename(bugInfo.getFilename()));
 					jg.writeStringField("function",filename(bugInfo.getFunction()));
@@ -63,6 +65,7 @@ public void output(final BugList bugList, final Writer writer, final boolean sho
 					prevbugInfo = bugInfo;
 					bugInfo = iterator.hasNext() ? iterator.next() : null;
 				} while (isGrouped(prevbugInfo, bugInfo));
+				jg.writeEndArray();
 				jg.writeEndObject();
 			}
 		}

diff --git a/src/main/java/com/cflint/config/ConfigRuntime.java b/src/main/java/com/cflint/config/ConfigRuntime.java
@@ -39,6 +39,12 @@ public ConfigRuntime(final CFLintConfig config, final CFLintPluginInfo pluginInf
 					// Include the rule if at least one of the messages is included.
 					for (PluginMessage msg : rule.getMessages()) {
 						if (config.getIncludes().contains(msg)) {
+							for (PluginMessage cfgMsg : config.getIncludes()) {
+								if (cfgMsg.equals(msg)) {
+									merge(cfgMsg, msg);
+								}
+							}
+
 							getRules().add(rule);
 							break;
 						}
@@ -61,6 +67,22 @@ public ConfigRuntime(final CFLintConfig config, final CFLintPluginInfo pluginInf
 		}
 	}
 
+	/*
+	 * Apply the configuration to the existing rule. Overlay it.
+	 */
+	private void merge(PluginMessage cfgMsg, PluginMessage msg) {
+		if (!isEmpty(cfgMsg.getMessageText())) {
+			msg.setMessageText(cfgMsg.getMessageText());
+		}
+		if (!isEmpty(cfgMsg.getSeverity())) {
+			msg.setSeverity(cfgMsg.getSeverity());
+		}
+	}
+
+	private boolean isEmpty(String messageText) {
+		return messageText == null || messageText.trim().length() == 0;
+	}
+
 	public boolean isIncludeMessage(final String messageCode) {
 		return isIncludeMessage(new PluginMessage(messageCode));
 	}

diff --git a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java
@@ -1,6 +1,5 @@
 package com.cflint.plugins.core;
 
-import com.cflint.BugInfo;
 import com.cflint.BugList;
 import com.cflint.plugins.CFLintScannerAdapter;
 import com.cflint.plugins.Context;
@@ -20,7 +19,7 @@ public void expression(final CFExpression expression, final Context context, fin
 		if (expression instanceof CFVarDeclExpression) {
 			final CFVarDeclExpression cfVarDeclExpression = (CFVarDeclExpression)expression;
 			int lineNo = expression.getLine() + context.startLine() - 1;
-			checkNameForBugs(cfVarDeclExpression.getName(), context.getFilename(), context.getFunctionName(), lineNo, bugs);
+			checkNameForBugs(context,cfVarDeclExpression.getName(), context.getFilename(), context.getFunctionName(), lineNo, bugs);
 		}
 		else if (expression instanceof CFFullVarExpression) {
 			final CFFullVarExpression cfFullVarExpression = (CFFullVarExpression)expression;
@@ -32,11 +31,11 @@ else if (expression instanceof CFIdentifier) {
 			String varName = ((CFIdentifier) expression).getName();
 			int lineNo = ((CFIdentifier) expression).getLine() + context.startLine() - 1;
 
-			checkNameForBugs(varName, context.getFilename(), context.getFunctionName(), lineNo, bugs);
+			checkNameForBugs(context, varName, context.getFilename(), context.getFunctionName(), lineNo, bugs);
 		}
 	}
 
-	public void checkNameForBugs(String variable, String filename, String functionName, int line, BugList bugs) {
+	public void checkNameForBugs(final Context context, String variable, String filename, String functionName, int line, BugList bugs) {
 		int minVarLength = ValidName.MIN_VAR_LENGTH;
 		int maxVarLength = ValidName.MAX_VAR_LENGTH;
 		int maxVarWords = ValidName.MAX_VAR_WORDS;
@@ -63,68 +62,31 @@ public void checkNameForBugs(String variable, String filename, String functionNa
 		ValidName name = new ValidName(minVarLength, maxVarLength, maxVarWords);
 
 		if (name.isInvalid(variable)) {
-			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_INVALID_NAME")
-				.setSeverity(severity).setFilename(filename)
-				.setMessage(VARIABLE + variable + " is not a valid name. Please use CamelCase or underscores.")
-				.setFunction(functionName)
-				.setVariable(variable)
-				.build());
+			context.addMessage("VAR_INVALID_NAME", variable);
 		}
 		if (!scope.isCFScoped(variable) && name.isUpperCase(variable)) {
-			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_ALLCAPS_NAME")
-				.setSeverity(severity).setFilename(filename)
-				.setMessage(VARIABLE + variable + " should not be upper case.")
-				.setFunction(functionName)
-				.setVariable(variable)
-				.build());
+			context.addMessage("VAR_ALLCAPS_NAME", variable);
 		}
-		if (scope.isCFScoped(variable) &&  name.isUpperCase(variable)) {
-			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("SCOPE_ALLCAPS_NAME")
-				.setSeverity(severity).setFilename(filename)
-				.setMessage("Scope " + variable + " should not be upper case.")
-				.setFunction(functionName)
-				.setVariable(variable)
-				.build());
+		if (scope.isCFScoped(variable)
+				&& name.isUpperCase(variable)
+				&& (getParameter("IgnoreUpperCaseScopes") == null || !getParameter(
+						"IgnoreUpperCaseScopes").contains(variable))) {
+			context.addMessage("SCOPE_ALLCAPS_NAME", variable);
 		}
 		if (name.tooShort(variable)) {
-			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_TOO_SHORT")
-				.setSeverity(severity).setFilename(filename)
-				.setMessage(VARIABLE + variable + " should be longer than " + minVarLength + " characters.")
-				.setFunction(functionName)
-				.setVariable(variable)
-				.build());
+			context.addMessage("VAR_TOO_SHORT", variable);
 		}
 		if (name.tooLong(variable)) {
-			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_TOO_LONG")
-				.setSeverity(severity).setFilename(filename)
-				.setMessage(VARIABLE + variable + " should be shorter than " + maxVarLength + " characters.")
-				.setFunction(functionName)
-				.setVariable(variable)
-				.build());
+			context.addMessage("VAR_TOO_LONG", variable);
 		}
 		if (!name.isUpperCase(variable) && name.tooWordy(variable)) {
-			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_TOO_WORDY")
-				.setSeverity(severity).setFilename(filename)
-				.setMessage(VARIABLE + variable + " is too wordy, can you think of a more concise name?")
-				.setFunction(functionName)
-				.setVariable(variable)
-				.build());
+			context.addMessage("VAR_TOO_WORDY", variable);
 		}
 		if (name.isTemporary(variable)) {
-			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_IS_TEMPORARY")
-				.setSeverity(severity).setFilename(filename)
-				.setMessage("Temporary variable " + variable + " could be named better.")
-				.setFunction(functionName)
-				.setVariable(variable)
-				.build());
+			context.addMessage("VAR_IS_TEMPORARY", variable);
 		}
 		if (name.hasPrefixOrPostfix(variable)) {
-			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_HAS_PREFIX_OR_POSTFIX")
-				.setSeverity(severity).setFilename(filename)
-				.setMessage("Variable has prefix or postfix " + variable + " and could be named better.")
-				.setFunction(functionName)
-				.setVariable(variable)
-				.build());
+			context.addMessage("VAR_HAS_PREFIX_OR_POSTFIX", variable);
 		}
 	}
 }
diff --git a/src/main/resources/cflint.definition.json b/src/main/resources/cflint.definition.json
@@ -506,35 +506,43 @@
       "message": [
         {
           "code": "VAR_INVALID_NAME",
-          "severity": "INFO"
+          "severity": "INFO",
+          "messageText" : "Variable ${variable} is not a valid name. Please use CamelCase or underscores."
         },
         {
           "code": "VAR_ALLCAPS_NAME",
-          "severity": "INFO"
+          "severity": "INFO",
+          "messageText" : "Variable ${variable} should not be upper case."
         },
         {
           "code": "SCOPE_ALLCAPS_NAME",
-          "severity": "INFO"
+          "severity": "INFO",
+          "messageText": "Scope ${variable} should not be upper case."
         },
         {
           "code": "VAR_TOO_SHORT",
-          "severity": "INFO"
+          "severity": "INFO",
+          "messageText": "Variable ${variable} should be longer than ${MinLength} characters."
         },
         {
           "code": "VAR_TOO_LONG",
-          "severity": "INFO"
+          "severity": "INFO",
+          "messageText" : "Variable ${variable} should be shorter than ${MaxLength} characters."
         },
         {
           "code": "VAR_TOO_WORDY",
-          "severity": "INFO"
+          "severity": "INFO",
+          "messageText" : "Variable ${variable} is too wordy, can you think of a more concise name?"
         },
         {
           "code": "VAR_IS_TEMPORARY",
-          "severity": "INFO"
+          "severity": "INFO",
+          "messageText" : "Temporary variable ${variable} could be named better."
         },
         {
           "code": "VAR_HAS_PREFIX_OR_POSTFIX",
-          "severity": "INFO"
+          "severity": "INFO",
+          "messageText" : "Variable has prefix or postfix ${variable} and could be named better."
         }
       ],
       "parameter": [
@@ -549,6 +557,10 @@
         {
           "name": "MaxWords",
           "value": "4"
+        },
+        {
+          "name": "IgnoreUpperCaseScopes",
+          "value": "CGI,URL"
         }
       ]
     },

diff --git a/src/test/java/com/cflint/TestJSONOutput.java b/src/test/java/com/cflint/TestJSONOutput.java
@@ -28,7 +28,7 @@ public void testOutput() throws IOException {
 		BugInfo bugInfo = new BugInfo.BugInfoBuilder().setFunction("testf").setMessageCode("PARSE_ERROR").setFilename("c:\\temp\\test.cfc").build();
 		bugList.add(bugInfo);
 		outputer.output(bugList, writer, false);
-		String expectedText = "[{\"severity\":\"\",\"id\":\"PARSE_ERROR\",\"message\":\"PARSE_ERROR\",\"category\":\"CFLINT\",\"abbrev\":\"PE\",\"location\":{\"file\":\"c:\\\\temp\\\\test.cfc\",\"fileName\":\"test.cfc\",\"function\":\"testf\",\"column\":\"0\",\"line\":\"0\",\"message\":\"\",\"variable\":\"\",\"expression\":\"\"}}]";
+		String expectedText = "[{\"severity\":\"\",\"id\":\"PARSE_ERROR\",\"message\":\"PARSE_ERROR\",\"category\":\"CFLINT\",\"abbrev\":\"PE\",\"locations\":[{\"file\":\"c:\\\\temp\\\\test.cfc\",\"fileName\":\"test.cfc\",\"function\":\"testf\",\"column\":\"0\",\"line\":\"0\",\"message\":\"\",\"variable\":\"\",\"expression\":\"\"}]}]";
 //		assertEquals(JSONValue.parse(expectedText),JSONValue.parse(writer.toString()));
 		assertEquals(expectedText,writer.toString());
 	}
@@ -38,7 +38,7 @@ public void testStats() throws IOException {
 		BugInfo bugInfo = new BugInfo.BugInfoBuilder().setFunction("testf").setMessageCode("PARSE_ERROR").setFilename("c:\\temp\\test.cfc").build();
 		bugList.add(bugInfo);
 		outputer.output(bugList, writer, true);
-		String expectedText = "[{\"severity\":\"\",\"id\":\"PARSE_ERROR\",\"message\":\"PARSE_ERROR\",\"category\":\"CFLINT\",\"abbrev\":\"PE\",\"location\":{\"file\":\"c:\\\\temp\\\\test.cfc\",\"fileName\":\"test.cfc\",\"function\":\"testf\",\"column\":\"0\",\"line\":\"0\",\"message\":\"\",\"variable\":\"\",\"expression\":\"\"}},{\"code\":\"PARSE_ERROR\",\"count\":\"1\"}]";
+		String expectedText = "[{\"severity\":\"\",\"id\":\"PARSE_ERROR\",\"message\":\"PARSE_ERROR\",\"category\":\"CFLINT\",\"abbrev\":\"PE\",\"locations\":[{\"file\":\"c:\\\\temp\\\\test.cfc\",\"fileName\":\"test.cfc\",\"function\":\"testf\",\"column\":\"0\",\"line\":\"0\",\"message\":\"\",\"variable\":\"\",\"expression\":\"\"}]},{\"code\":\"PARSE_ERROR\",\"count\":\"1\"}]";
 		assertEquals(expectedText,writer.toString());
 	}
 

diff --git a/src/test/java/com/cflint/integration/TestFiles.java b/src/test/java/com/cflint/integration/TestFiles.java
@@ -84,8 +84,9 @@ public void test() throws IOException, URISyntaxException, JAXBException, Transf
 				expectedText = actualTree;
 				writeExpectFile(expectedFile, actualTree);
 			}
-			assertEquals(actualTree.replaceAll("\\\\","/").replaceAll("/+","/").replaceAll("\r\n", "\n"), 
-					expectedText.replaceAll("\\\\","/").replaceAll("/+","/").replaceAll("\r\n", "\n"));
+			assertEquals( 
+					expectedText.replaceAll("\\\\","/").replaceAll("/+","/").replaceAll("\r\n", "\n"),
+					actualTree.replaceAll("\\\\","/").replaceAll("/+","/").replaceAll("\r\n", "\n"));
 		}
 	}
 

diff --git a/src/test/resources/com/cflint/tests/CompareInsteadOfAssign/Compare1.expected.txt b/src/test/resources/com/cflint/tests/CompareInsteadOfAssign/Compare1.expected.txt
@@ -4,30 +4,30 @@
   "message" : "COMPARE_INSTEAD_OF_ASSIGN",
   "category" : "CFLINT",
   "abbrev" : "CI",
-  "location" : {
-    "file" : "src\\test\\resources\\com\\cflint\\tests\\CompareInsteadOfAssign\\Compare1.cfm",
+  "locations" : [ {
+    "file" : "src/test/resources/com/cflint/tests/CompareInsteadOfAssign/Compare1.cfm",
     "fileName" : "Compare1.cfm",
     "function" : "",
     "column" : "5",
     "line" : "5",
     "message" : "CWE-482: Comparing instead of Assigning",
     "variable" : "==",
     "expression" : "x.x == 2"
-  }
+  } ]
 }, {
   "severity" : "WARNING",
   "id" : "COMPARE_INSTEAD_OF_ASSIGN",
   "message" : "COMPARE_INSTEAD_OF_ASSIGN",
   "category" : "CFLINT",
   "abbrev" : "CI",
-  "location" : {
-    "file" : "src\\test\\resources\\com\\cflint\\tests\\CompareInsteadOfAssign\\Compare1.cfm",
+  "locations" : [ {
+    "file" : "src/test/resources/com/cflint/tests/CompareInsteadOfAssign/Compare1.cfm",
     "fileName" : "Compare1.cfm",
     "function" : "",
     "column" : "5",
     "line" : "9",
     "message" : "CWE-482: Comparing instead of Assigning",
     "variable" : "EQ",
     "expression" : "x.x EQ 6"
-  }
+  } ]
 } ]
diff --git a/src/test/resources/com/cflint/tests/UnusedArgument/UnusedArgument_152b.expected.txt b/src/test/resources/com/cflint/tests/UnusedArgument/UnusedArgument_152b.expected.txt
@@ -4,14 +4,14 @@
   "message" : "UNUSED_METHOD_ARGUMENT",
   "category" : "CFLINT",
   "abbrev" : "UM",
-  "location" : {
-    "file" : "src\\test\\resources\\com\\cflint\\tests\\UnusedArgument\\UnusedArgument_152b.cfc",
+  "locations" : [ {
+    "file" : "src/test/resources/com/cflint/tests/UnusedArgument/UnusedArgument_152b.cfc",
     "fileName" : "UnusedArgument_152b.cfc",
     "function" : "helloWorld",
     "column" : "0",
     "line" : "6",
     "message" : "Argument xyzzy is not used in function helloWorld, consider removing it.",
     "variable" : "xyzzy",
     "expression" : ""
-  }
+  } ]
 } ]