From 16832adf11635eeccf886aed68a4102fbde0db28 Mon Sep 17 00:00:00 2001 From: Stephen Matta Date: Fri, 2 Oct 2015 21:03:57 -0400 Subject: [PATCH 01/13] Added -nooutput option Need to extend this to all types. Right now it's just for XML. --- src/main/java/com/cflint/main/CFLintMain.java | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/cflint/main/CFLintMain.java b/src/main/java/com/cflint/main/CFLintMain.java index 39634d681..3b7270083 100644 --- a/src/main/java/com/cflint/main/CFLintMain.java +++ b/src/main/java/com/cflint/main/CFLintMain.java @@ -57,12 +57,13 @@ public class CFLintMain { boolean showprogress= false; boolean progressUsesThread=true; private String configfile = null; + boolean noOutput = false; public static void main(final String[] args) throws ParseException, IOException, TransformerException, JAXBException { //PropertyConfigurator.configure("/log4j.properties"); //DOMConfigurator.configure(CFLintFilter.class.getResource("/log4j.xml").getFile()); //Logger.getLogger("net.htmlparser.jericho"); - + final Options options = new Options(); // add t option options.addOption("includeRule", true, "specify rules to include"); @@ -76,7 +77,7 @@ public static void main(final String[] args) throws ParseException, IOException, options.addOption("verbose", false, "verbose"); options.addOption("showprogress", false, "show progress bar"); options.addOption("singlethread", false, "show progress bar"); - + options.addOption("logerror", false, "log parsing errors as bugs"); options.addOption("e", false, "log parsing errors as bugs"); options.addOption("q", false, "quiet"); @@ -95,7 +96,8 @@ public static void main(final String[] args) throws ParseException, IOException, options.addOption("textfile", true, "specify the output text file (default: cflint-results.txt)"); options.addOption("extensions", true, "specify the extensions of the CF source files (default: .cfm,.cfc)"); options.addOption("configfile", true, "specify the location of the config file"); - + options.addOption("nooutput", false, "prevent file output"); + final CommandLineParser parser = new GnuParser(); final CommandLine cmd = parser.parse(options, args); @@ -159,7 +161,7 @@ public static void main(final String[] args) throws ParseException, IOException, if (cmd.hasOption("extensions")) { main.extensions = cmd.getOptionValue("extensions"); } - + if (cmd.hasOption("includeRule")) { main.includeCodes = cmd.getOptionValue("includeRule").split(","); } @@ -168,6 +170,7 @@ public static void main(final String[] args) throws ParseException, IOException, } main.showprogress=cmd.hasOption("showprogress") || (!cmd.hasOption("showprogress") && cmd.hasOption("ui")); main.progressUsesThread=!cmd.hasOption("singlethread"); + main.noOutput = cmd.hasOption("nooutput"); // for (final Option option : cmd.getOptions()) { // if(main.verbose){ // System.out.println("Option " + option.getOpt() + " => " + option.getValue()); @@ -233,7 +236,7 @@ private void ui() { if (indx == 4) { jsonOutput = true; } - + } } @@ -265,7 +268,7 @@ private void execute() throws IOException, TransformerException, JAXBException { filter = CFLintFilter.createFilter(new String(b),verbose); } } - + if (excludeCodes != null && excludeCodes.length > 0) { filter.excludeCode(excludeCodes); } @@ -283,16 +286,17 @@ private void execute() throws IOException, TransformerException, JAXBException { if(verbose){ System.out.println("Style:" + xmlstyle); } + Writer xmlwriter = (!noOutput && xmlOutFile != null) ? new FileWriter(xmlOutFile) : new OutputStreamWriter(System.out); if ("findbugs".equalsIgnoreCase(xmlstyle)) { - if(verbose){ + if(verbose && !noOutput){ System.out.println("Writing findbugs style to " + xmlOutFile); } - new XMLOutput().outputFindBugs(cflint.getBugs(), new FileWriter(xmlOutFile)); + new XMLOutput().outputFindBugs(cflint.getBugs(), xmlwriter); } else { if(verbose){ System.out.println("Writing " + xmlOutFile); } - new XMLOutput().output(cflint.getBugs(), new FileWriter(xmlOutFile)); + new XMLOutput().output(cflint.getBugs(), xmlwriter); } } if (textOutput) { @@ -303,7 +307,7 @@ private void execute() throws IOException, TransformerException, JAXBException { } Writer textwriter = textOutFile != null?new FileWriter(textOutFile):new OutputStreamWriter(System.out); new TextOutput().output(cflint.getBugs(), textwriter); - + } if (htmlOutput) { try { From 561e1280a6d611b0fff17abc4d7bf18a037423f2 Mon Sep 17 00:00:00 2001 From: Stephen Matta Date: Wed, 7 Oct 2015 11:30:26 -0400 Subject: [PATCH 02/13] Add stdin and stdout command line options Added -stdin and -stdout command line options so that CFLint can be used by processes that can't or don't want to write to disk. --- src/main/java/com/cflint/main/CFLintMain.java | 74 +++++++++++-------- 1 file changed, 43 insertions(+), 31 deletions(-) diff --git a/src/main/java/com/cflint/main/CFLintMain.java b/src/main/java/com/cflint/main/CFLintMain.java index 3b7270083..1288f5e85 100644 --- a/src/main/java/com/cflint/main/CFLintMain.java +++ b/src/main/java/com/cflint/main/CFLintMain.java @@ -10,6 +10,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Scanner; import javax.swing.JFileChooser; import javax.swing.JList; @@ -57,7 +58,8 @@ public class CFLintMain { boolean showprogress= false; boolean progressUsesThread=true; private String configfile = null; - boolean noOutput = false; + private Boolean stdIn = false; + private Boolean stdOut = false; public static void main(final String[] args) throws ParseException, IOException, TransformerException, JAXBException { //PropertyConfigurator.configure("/log4j.properties"); @@ -96,7 +98,8 @@ public static void main(final String[] args) throws ParseException, IOException, options.addOption("textfile", true, "specify the output text file (default: cflint-results.txt)"); options.addOption("extensions", true, "specify the extensions of the CF source files (default: .cfm,.cfc)"); options.addOption("configfile", true, "specify the location of the config file"); - options.addOption("nooutput", false, "prevent file output"); + options.addOption("stdin", false, "use stdin for file input"); + options.addOption("stdout", false, "output to stdout only"); final CommandLineParser parser = new GnuParser(); @@ -170,12 +173,8 @@ public static void main(final String[] args) throws ParseException, IOException, } main.showprogress=cmd.hasOption("showprogress") || (!cmd.hasOption("showprogress") && cmd.hasOption("ui")); main.progressUsesThread=!cmd.hasOption("singlethread"); - main.noOutput = cmd.hasOption("nooutput"); -// for (final Option option : cmd.getOptions()) { -// if(main.verbose){ -// System.out.println("Option " + option.getOpt() + " => " + option.getValue()); -// } -// } + main.stdIn = cmd.hasOption("stdin"); + main.stdOut = cmd.hasOption("stdout"); if (main.isValid()) { main.execute(); if (cmd.hasOption("ui")) { @@ -255,7 +254,7 @@ private void execute() throws IOException, TransformerException, JAXBException { try{ cflint.setAllowedExtensions(Arrays.asList(extensions.trim().split(","))); }catch(Exception e){ - System.out.println("Unable to use extensions (" + extensions + ") using default instead. " + e.getMessage()); + System.err.println("Unable to use extensions (" + extensions + ") using default instead. " + e.getMessage()); } } CFLintFilter filter = CFLintFilter.createFilter(verbose); @@ -278,52 +277,59 @@ private void execute() throws IOException, TransformerException, JAXBException { cflint.getBugs().setFilter(filter); for (final String scanfolder : folder) { cflint.scan(scanfolder); - // for(BugInfo bi: cflint.getBugs()){ - // System.out.println(bi); - // } } - if (xmlOutput) { - if(verbose){ - System.out.println("Style:" + xmlstyle); + if (stdIn) { + StringBuilder source = new StringBuilder(); + Scanner scanner = new Scanner(System.in); + while (scanner.hasNextLine()) { + String nextLine = scanner.nextLine(); + source.append(nextLine); + source.append(System.lineSeparator()); } - Writer xmlwriter = (!noOutput && xmlOutFile != null) ? new FileWriter(xmlOutFile) : new OutputStreamWriter(System.out); + scanner.close(); + cflint.process(source.toString(), "source.cfc"); + } + if (xmlOutput) { + Writer xmlwriter = stdOut ? new OutputStreamWriter(System.out) : new FileWriter(xmlOutFile); if ("findbugs".equalsIgnoreCase(xmlstyle)) { - if(verbose && !noOutput){ - System.out.println("Writing findbugs style to " + xmlOutFile); + if(verbose) { + display("Writing XML findbugs style" + (stdOut ? "." : " to " + xmlOutFile)); } new XMLOutput().outputFindBugs(cflint.getBugs(), xmlwriter); } else { - if(verbose){ - System.out.println("Writing " + xmlOutFile); + if(verbose) { + display("Writing XML" + (stdOut ? "." : " to " + xmlOutFile)); } new XMLOutput().output(cflint.getBugs(), xmlwriter); } } if (textOutput) { if(textOutFile != null){ - if(verbose){ - System.out.println("Writing " + textOutFile); + if(verbose) { + display("Writing text" + (stdOut ? "." : " to " + textOutFile)); } } - Writer textwriter = textOutFile != null?new FileWriter(textOutFile):new OutputStreamWriter(System.out); + Writer textwriter = stdOut ? new OutputStreamWriter(System.out) : new FileWriter(textOutFile); new TextOutput().output(cflint.getBugs(), textwriter); } if (htmlOutput) { try { - if(verbose){ - System.out.println("Writing " + htmlOutFile); + if(verbose) { + display("Writing HTML" + (stdOut ? "." : " to " + htmlOutFile)); } - new HTMLOutput(htmlStyle).output(cflint.getBugs(), new FileWriter(htmlOutFile)); + Writer htmlwriter = stdOut ? new OutputStreamWriter(System.out) : new FileWriter(htmlOutFile); + new HTMLOutput(htmlStyle).output(cflint.getBugs(), htmlwriter); } catch (final TransformerException e) { throw new IOException(e); } } if (jsonOutput) { - if(verbose){ - System.out.println("Writing " + jsonOutFile); + if(verbose) { + display("Writing JSON" + (stdOut ? "." : " to " + jsonOutFile)); } - new JSONOutput().output(cflint.getBugs(), new FileWriter(jsonOutFile)); + Writer jsonwriter = stdOut ? new OutputStreamWriter(System.out) : new FileWriter(jsonOutFile); + new JSONOutput().output(cflint.getBugs(), jsonwriter); } if (includeCodes != null) { cflint.getBugs().getFilter().includeCode(includeCodes); @@ -332,10 +338,16 @@ private void execute() throws IOException, TransformerException, JAXBException { cflint.getBugs().getFilter().excludeCode(excludeCodes); } } + + private void display(String text) { + if (verbose) { + System.out.println(text); + } + } private boolean isValid() { - if (folder.isEmpty()) { - System.err.println("Set -scanFolder"); + if (folder.isEmpty() && !stdIn) { + System.err.println("Set -scanFolder or -stdin"); return false; } return true; From 9c763e954320e8db14a3d5896840018ae45174d7 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Mon, 12 Oct 2015 00:41:01 +1100 Subject: [PATCH 03/13] add literal checker rule --- src/main/resources/cflint.definition.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/resources/cflint.definition.xml b/src/main/resources/cflint.definition.xml index befa1ab8a..5f9c09e87 100644 --- a/src/main/resources/cflint.definition.xml +++ b/src/main/resources/cflint.definition.xml @@ -179,4 +179,9 @@ INFO + + + WARNING + + From ca97849df133c66782f13e557744b9b75f9b7811 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Mon, 12 Oct 2015 00:46:23 +1100 Subject: [PATCH 04/13] Rule to check for repeated hard coded literal values --- .../cflint/plugins/core/LiteralChecker.java | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 src/main/java/com/cflint/plugins/core/LiteralChecker.java diff --git a/src/main/java/com/cflint/plugins/core/LiteralChecker.java b/src/main/java/com/cflint/plugins/core/LiteralChecker.java new file mode 100644 index 000000000..c0318739e --- /dev/null +++ b/src/main/java/com/cflint/plugins/core/LiteralChecker.java @@ -0,0 +1,76 @@ +package com.cflint.plugins.core; + +import java.util.Map; +import java.util.HashMap; + +import ro.fortsoft.pf4j.Extension; +import cfml.parsing.cfscript.CFExpression; +import cfml.parsing.cfscript.CFFunctionExpression; +import cfml.parsing.cfscript.CFLiteral; +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 LiteralChecker extends CFLintScannerAdapter { + final String severity = "WARNING"; + final protected int REPEAT_THRESHOLD = 3; + + protected int threshold = 3; + protected Map literals = new HashMap(); + protected Map done = new HashMap(); + + // May want to consider resetting literal map on new components but this way it + // detects duplicated literals across files which is useful + + @Override + public void expression(final CFExpression expression, final Context context, final BugList bugs) { + String repeatThreshold = getParameter("maximum"); + int threshold = REPEAT_THRESHOLD; + + if (repeatThreshold != null) { + threshold = Integer.parseInt(repeatThreshold); + } + + if (expression instanceof CFLiteral) { + CFLiteral literal = (CFLiteral) expression; + String name = literal.Decompile(0).replace("'",""); + int count = 1; + + if (isCommon(name)) { + return; + } + + if (literals.get(name) == null) { + literals.put(name, count); + done.put(name, false); + } + else { + count = literals.get(name); + count++; + literals.put(name, count); + } + + if (count > threshold && !done.get(name)) { + int lineNo = literal.getLine() + context.startLine() - 1; + magicValue(name, lineNo, context, bugs); + done.put(name, true); + } + } + } + + protected boolean isCommon(final String name) { + return name.equals("1") || name.equals("0") || name.equals("") || name.equals("true") || name.equals("false"); + } + + public void magicValue(final String name, final int lineNo, final Context context, final BugList bugs) { + bugs.add(new BugInfo.BugInfoBuilder().setLine(lineNo).setMessageCode("LITERAL_VALUE_USED_TOO_OFTEN") + .setSeverity(severity).setFilename(context.getFilename()) + .setMessage("Literal " + name + " occurs several times. Consider giving it a name and not hard coding values.") + .build()); + } +} \ No newline at end of file From 0095acda5e51b162edb667c62703b14ca575607b Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Mon, 12 Oct 2015 00:47:12 +1100 Subject: [PATCH 05/13] tests for literal checker --- .../java/com/cflint/TestLiteralChecker.java | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 src/test/java/com/cflint/TestLiteralChecker.java diff --git a/src/test/java/com/cflint/TestLiteralChecker.java b/src/test/java/com/cflint/TestLiteralChecker.java new file mode 100644 index 000000000..e2ef91758 --- /dev/null +++ b/src/test/java/com/cflint/TestLiteralChecker.java @@ -0,0 +1,65 @@ +package com.cflint; + +import static org.junit.Assert.assertEquals; + +import java.io.IOException; +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.LiteralChecker; + +public class TestLiteralChecker { + + private CFLint cfBugs; + + @Before + public void setUp() { + final ConfigRuntime conf = new ConfigRuntime(); + final PluginInfoRule pluginRule = new PluginInfoRule(); + pluginRule.setName("LiteralChecker"); + conf.getRules().add(pluginRule); + final PluginMessage pluginMessage = new PluginMessage("LITERAL_VALUE_USED_TOO_OFTEN"); + pluginMessage.setSeverity("WARNING"); + cfBugs = new CFLint(conf, new LiteralChecker()); + } + + @Test + public void testOK() throws ParseException, IOException { + final String scriptSrc = "\r\n" + + "var pi = 3.14;\r\n" + + "var code = \"CODE\";\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + final Map> result = cfBugs.getBugs().getBugList(); + assertEquals(0, result.size()); + } + + @Test + public void testTooManyHardCodevedValues() throws ParseException, IOException { + final String scriptSrc = "\r\n" + + "pi = 3.14;\r\n" + + "calc = \"AREA\";\r\n" + + "if (code == \"AREA\") {\r\n" + + "area = 3.14 * radius * radius;\r\n" + + "circumference = 2 * 3.14 * radius;\r\n" + + "volume = 4/3 * 3.14 * radius * radius * radius;\r\n" + +"}\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("LITERAL_VALUE_USED_TOO_OFTEN", result.get(0).getMessageCode()); + assertEquals(7, result.get(0).getLine()); + } + +} From 0b23ab37a0b555ce6376c1284e2a00478ee1d6c3 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Thu, 15 Oct 2015 15:25:53 +1100 Subject: [PATCH 06/13] Split warning into two one for global (i.e. constants across many files) or local (i.e. constants in the same file). --- .../cflint/plugins/core/LiteralChecker.java | 76 ++++++++++++++----- 1 file changed, 57 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/cflint/plugins/core/LiteralChecker.java b/src/main/java/com/cflint/plugins/core/LiteralChecker.java index c0318739e..57d9bc10b 100644 --- a/src/main/java/com/cflint/plugins/core/LiteralChecker.java +++ b/src/main/java/com/cflint/plugins/core/LiteralChecker.java @@ -7,9 +7,11 @@ import cfml.parsing.cfscript.CFExpression; import cfml.parsing.cfscript.CFFunctionExpression; import cfml.parsing.cfscript.CFLiteral; +import cfml.parsing.cfscript.script.CFCompDeclStatement; 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; @@ -19,10 +21,13 @@ public class LiteralChecker extends CFLintScannerAdapter { final String severity = "WARNING"; final protected int REPEAT_THRESHOLD = 3; + final protected int WARNING_THRESHOLD = 5; + + protected int threshold = REPEAT_THRESHOLD; + protected int warningThreshold = WARNING_THRESHOLD; - protected int threshold = 3; - protected Map literals = new HashMap(); - protected Map done = new HashMap(); + protected Map globalLiterals = new HashMap(); + protected Map functionListerals = new HashMap(); // May want to consider resetting literal map on new components but this way it // detects duplicated literals across files which is useful @@ -30,35 +35,61 @@ public class LiteralChecker extends CFLintScannerAdapter { @Override public void expression(final CFExpression expression, final Context context, final BugList bugs) { String repeatThreshold = getParameter("maximum"); - int threshold = REPEAT_THRESHOLD; + String maxWarnings = getParameter("maxWarnings"); + String warningScope = getParameter("warningScope"); if (repeatThreshold != null) { threshold = Integer.parseInt(repeatThreshold); } + if (maxWarnings != null) { + warningThreshold = Integer.parseInt(maxWarnings); + } + if (expression instanceof CFLiteral) { CFLiteral literal = (CFLiteral) expression; String name = literal.Decompile(0).replace("'",""); - int count = 1; if (isCommon(name)) { return; } - if (literals.get(name) == null) { - literals.put(name, count); - done.put(name, false); + int lineNo = literal.getLine() + context.startLine() - 1; + + if (warningScope == null || warningScope.equals("global")) { + literalCount(name, lineNo, globalLiterals, true, context, bugs); } - else { - count = literals.get(name); - count++; - literals.put(name, count); + else if (warningScope.equals("local")) { + literalCount(name, lineNo, functionListerals, false, context, bugs); } + } + } + + @Override + public void expression(final CFScriptStatement expression, final Context context, final BugList bugs) { + if (expression instanceof CFCompDeclStatement) { + functionListerals.clear(); + } + } - if (count > threshold && !done.get(name)) { - int lineNo = literal.getLine() + context.startLine() - 1; - magicValue(name, lineNo, context, bugs); - done.put(name, true); + protected void literalCount(final String name, final int lineNo, Map literals, boolean global, final Context context, final BugList bugs) { + int count = 1; + + if (literals.get(name) == null) { + literals.put(name, count); + } + else { + count = literals.get(name); + count++; + literals.put(name, count); + } + + if (count > threshold && (warningThreshold == -1 || (count - threshold) <= warningThreshold)) { + if (global) { + magicGlobalValue(name, lineNo, context, bugs); + } + else { + magicLocalValue(name, lineNo, context, bugs); } } } @@ -67,10 +98,17 @@ protected boolean isCommon(final String name) { return name.equals("1") || name.equals("0") || name.equals("") || name.equals("true") || name.equals("false"); } - public void magicValue(final String name, final int lineNo, final Context context, final BugList bugs) { - bugs.add(new BugInfo.BugInfoBuilder().setLine(lineNo).setMessageCode("LITERAL_VALUE_USED_TOO_OFTEN") + public void magicLocalValue(final String name, final int lineNo, final Context context, final BugList bugs) { + bugs.add(new BugInfo.BugInfoBuilder().setLine(lineNo).setMessageCode("LOCAL_LITERAL_VALUE_USED_TOO_OFTEN") + .setSeverity(severity).setFilename(context.getFilename()) + .setMessage("Literal " + name + " occurs several times in the same file. Consider giving it a name and not hard coding values.") + .build()); + } + + public void magicGlobalValue(final String name, final int lineNo, final Context context, final BugList bugs) { + bugs.add(new BugInfo.BugInfoBuilder().setLine(lineNo).setMessageCode("GLOBAL_LITERAL_VALUE_USED_TOO_OFTEN") .setSeverity(severity).setFilename(context.getFilename()) - .setMessage("Literal " + name + " occurs several times. Consider giving it a name and not hard coding values.") + .setMessage("Literal " + name + " occurs several times in one or more files. Consider giving it a name and not hard coding values.") .build()); } } \ No newline at end of file From 4210cae9805291b887717bbd6274198651cee9cf Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Thu, 15 Oct 2015 15:26:21 +1100 Subject: [PATCH 07/13] Added literal checker --- src/main/resources/cflint.definition.xml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/resources/cflint.definition.xml b/src/main/resources/cflint.definition.xml index 5f9c09e87..6b84fef06 100644 --- a/src/main/resources/cflint.definition.xml +++ b/src/main/resources/cflint.definition.xml @@ -180,7 +180,10 @@ - + + WARNING + + WARNING From 7546b80096dc2574891b6dc702df6f1ae11e0ab9 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Thu, 15 Oct 2015 15:27:01 +1100 Subject: [PATCH 08/13] split tests into two files --- .../com/cflint/TestLiteralGlobalChecker.java | 95 +++++++++++++++++++ ...cker.java => TestLiteralLocalChecker.java} | 12 ++- 2 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 src/test/java/com/cflint/TestLiteralGlobalChecker.java rename src/test/java/com/cflint/{TestLiteralChecker.java => TestLiteralLocalChecker.java} (79%) diff --git a/src/test/java/com/cflint/TestLiteralGlobalChecker.java b/src/test/java/com/cflint/TestLiteralGlobalChecker.java new file mode 100644 index 000000000..c4c2689a8 --- /dev/null +++ b/src/test/java/com/cflint/TestLiteralGlobalChecker.java @@ -0,0 +1,95 @@ +package com.cflint; + +import static org.junit.Assert.assertEquals; + +import java.io.IOException; +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.LiteralChecker; + +public class TestLiteralGlobalChecker { + + private CFLint cfBugs; + + @Before + public void setUp() { + final ConfigRuntime conf = new ConfigRuntime(); + final PluginInfoRule pluginRule = new PluginInfoRule(); + pluginRule.setName("LiteralChecker"); + conf.getRules().add(pluginRule); + final PluginMessage pluginMessage = new PluginMessage("GLOBAL_LITERAL_VALUE_USED_TOO_OFTEN"); + pluginMessage.setSeverity("WARNING"); + LiteralChecker checker = new LiteralChecker(); + checker.setParameter("maximum", "3"); + checker.setParameter("maxWarnings", "2"); + checker.setParameter("scope", "global"); + cfBugs = new CFLint(conf, checker); + } + + @Test + public void testOK() throws ParseException, IOException { + final String scriptSrc = "\r\n" + + "var pi = 3.14;\r\n" + + "var code = \"CODE\";\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + final Map> result = cfBugs.getBugs().getBugList(); + assertEquals(0, result.size()); + } + + @Test + public void testTooManyLocalHardCodevedValues() throws ParseException, IOException { + final String scriptSrc = "\r\n" + + "pi = 3.14;\r\n" + + "calc = \"AREA\";\r\n" + + "if (code == \"AREA\") {\r\n" + + "area = 3.14 * radius * radius;\r\n" + + "circumference = 2 * 3.14 * radius;\r\n" + + "volume = 4/3 * 3.14 * radius * radius * radius;\r\n" + +"}\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("GLOBAL_LITERAL_VALUE_USED_TOO_OFTEN", result.get(0).getMessageCode()); + assertEquals(7, result.get(0).getLine()); + } + + @Test + public void testTooManyGlobalHardCodevedValues() throws ParseException, IOException { + String scriptSrc = "\r\n" + + "pi = 3.14;\r\n" + + "calc = \"AREA\";\r\n" + + "if (code == \"AREA\") {\r\n" + + "circumference = 2 * 3.14 * radius;\r\n" + +"}\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + + scriptSrc = "\r\n" + + "area = 3.14 * radius * radius;\r\n" + + "volume = 4/3 * 3.14 * radius * radius * radius;\r\n" + +"}\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("GLOBAL_LITERAL_VALUE_USED_TOO_OFTEN", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + } + +} diff --git a/src/test/java/com/cflint/TestLiteralChecker.java b/src/test/java/com/cflint/TestLiteralLocalChecker.java similarity index 79% rename from src/test/java/com/cflint/TestLiteralChecker.java rename to src/test/java/com/cflint/TestLiteralLocalChecker.java index e2ef91758..2a6b3fcde 100644 --- a/src/test/java/com/cflint/TestLiteralChecker.java +++ b/src/test/java/com/cflint/TestLiteralLocalChecker.java @@ -16,7 +16,7 @@ import com.cflint.config.ConfigRuntime; import com.cflint.plugins.core.LiteralChecker; -public class TestLiteralChecker { +public class TestLiteralLocalChecker { private CFLint cfBugs; @@ -26,9 +26,13 @@ public void setUp() { final PluginInfoRule pluginRule = new PluginInfoRule(); pluginRule.setName("LiteralChecker"); conf.getRules().add(pluginRule); - final PluginMessage pluginMessage = new PluginMessage("LITERAL_VALUE_USED_TOO_OFTEN"); + final PluginMessage pluginMessage = new PluginMessage("LOCAL_LITERAL_VALUE_USED_TOO_OFTEN"); pluginMessage.setSeverity("WARNING"); - cfBugs = new CFLint(conf, new LiteralChecker()); + LiteralChecker checker = new LiteralChecker(); + checker.setParameter("maximum", "3"); + checker.setParameter("maxWarnings", "2"); + checker.setParameter("warningScope", "local"); + cfBugs = new CFLint(conf, checker); } @Test @@ -58,7 +62,7 @@ public void testTooManyHardCodevedValues() throws ParseException, IOException { cfBugs.process(scriptSrc, "test"); final List result = cfBugs.getBugs().getBugList().values().iterator().next(); assertEquals(1, result.size()); - assertEquals("LITERAL_VALUE_USED_TOO_OFTEN", result.get(0).getMessageCode()); + assertEquals("LOCAL_LITERAL_VALUE_USED_TOO_OFTEN", result.get(0).getMessageCode()); assertEquals(7, result.get(0).getLine()); } From 73687796901735b3fdb8005e020bac1f858d3eb1 Mon Sep 17 00:00:00 2001 From: ryaneberly Date: Thu, 15 Oct 2015 22:10:59 -0400 Subject: [PATCH 09/13] #104 --- src/main/java/com/cflint/CFLint.java | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/cflint/CFLint.java b/src/main/java/com/cflint/CFLint.java index 63b58d6ed..43ea5e189 100644 --- a/src/main/java/com/cflint/CFLint.java +++ b/src/main/java/com/cflint/CFLint.java @@ -21,14 +21,13 @@ import org.antlr.runtime.RecognitionException; import org.antlr.v4.runtime.Parser; import org.antlr.v4.runtime.Recognizer; +import org.antlr.v4.runtime.Token; import org.antlr.v4.runtime.atn.ATNConfigSet; import org.antlr.v4.runtime.dfa.DFA; import cfml.CFSCRIPTLexer; import cfml.parsing.CFMLParser; import cfml.parsing.CFMLSource; -import cfml.parsing.cfml.ErrorEvent; -import cfml.parsing.cfml.IErrorObserver; import cfml.parsing.cfscript.CFAssignmentExpression; import cfml.parsing.cfscript.CFBinaryExpression; import cfml.parsing.cfscript.CFExpression; @@ -775,7 +774,13 @@ public void syntaxError(Recognizer recognizer, Object offendingSymbol, int line, int charPositionInLine, String msg, org.antlr.v4.runtime.RecognitionException e) { final String file = currentFile == null ? "" : currentFile + "\r\n"; - //System.err.println(file + "----syntax error ---" + line + " : " + charPositionInLine); + String expression=null; + if(offendingSymbol instanceof Token){ + expression = ((Token) offendingSymbol).getText(); + if(expression.length() > 50){ + expression=expression.substring(1,40) + "..."; + } + } if(!currentElement.isEmpty()){ Element elem = currentElement.peek(); if(line == 1){ @@ -785,7 +790,17 @@ public void syntaxError(Recognizer recognizer, line = elem.getSource().getRow(elem.getBegin()) + line - 1; } } - fireCFLintException(e,PARSE_ERROR,currentFile,line,charPositionInLine,"",offendingSymbol==null?"":offendingSymbol.toString()); + if(recognizer instanceof Parser && ((Parser)recognizer).getExpectedTokens().contains(65)){ + bugs.add(new BugInfo.BugInfoBuilder().setMessageCode("PARSE_ERROR") + .setFilename(file).setMessage("End of statement(;) expected instead of " + expression).setSeverity("ERROR") + .setExpression(expression) + .setLine(line).setColumn(charPositionInLine) + .build()); + + }else{ + + fireCFLintException(e,PARSE_ERROR,file,line,charPositionInLine,"",msg); + } } public void reportAmbiguity(Parser recognizer, DFA dfa, int startIndex, int stopIndex, boolean exact, java.util.BitSet ambigAlts, From c5fa84087baca3d90c75df9957ac68a3022095e3 Mon Sep 17 00:00:00 2001 From: ryaneberly Date: Fri, 16 Oct 2015 21:25:50 -0400 Subject: [PATCH 10/13] add test --- .../java/com/cflint/TestParsingErrors.java | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 src/test/java/com/cflint/TestParsingErrors.java diff --git a/src/test/java/com/cflint/TestParsingErrors.java b/src/test/java/com/cflint/TestParsingErrors.java new file mode 100644 index 000000000..c8b94ef7b --- /dev/null +++ b/src/test/java/com/cflint/TestParsingErrors.java @@ -0,0 +1,57 @@ +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.ConfigRuntime; +import com.cflint.config.CFLintPluginInfo.PluginInfoRule; +import com.cflint.config.CFLintPluginInfo.PluginInfoRule.PluginMessage; +import com.cflint.plugins.core.VarScoper; + +public class TestParsingErrors { + + private CFLint cfBugs; + + @Before + public void setUp() { + ConfigRuntime conf = new ConfigRuntime(); + PluginInfoRule pluginRule = new PluginInfoRule(); + pluginRule.setName("VarScoper"); + conf.getRules().add(pluginRule); + PluginMessage pluginMessage = new PluginMessage("MISSING_VAR"); + pluginMessage.setSeverity("ERROR"); + pluginMessage + .setMessageText("Variable ${variable} is not declared with a var statement."); + pluginRule.getMessages().add(pluginMessage); + + cfBugs = new CFLint(conf, new VarScoper()); + } + + @Test + public void testMissingSemiColon() throws ParseException, IOException{ + final String cfcSrc = "\r\n" + + "\r\n" + + " \r\n" + + " var xx = 123\r\n" + + " yy = 123\r\n" + + " \r\n" + + "\r\n" + + ""; + cfBugs.process(cfcSrc,"test"); + assertEquals(1,cfBugs.getBugs().getFlatBugList().size()); + List result = cfBugs.getBugs().getFlatBugList(); + assertEquals(1,result.size()); + assertEquals("MISSING_VAR",result.get(0).getMessageCode()); + assertEquals("yy",result.get(0).getVariable()); + assertEquals(5,result.get(0).getLine()); + assertEquals("ERROR",result.get(0).getSeverity()); + assertEquals("Variable yy is not declared with a var statement.",result.get(0).getMessage()); + } +} From 8fada1ea3486ebe52a48955db06c8b37b0f0b40d Mon Sep 17 00:00:00 2001 From: ryaneberly Date: Mon, 19 Oct 2015 21:59:37 -0400 Subject: [PATCH 11/13] changed message code for missing semi-color to MISSING_SEMI --- src/main/java/com/cflint/CFLint.java | 3 ++- .../java/com/cflint/TestParsingErrors.java | 21 +++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/cflint/CFLint.java b/src/main/java/com/cflint/CFLint.java index 43ea5e189..01665db1e 100644 --- a/src/main/java/com/cflint/CFLint.java +++ b/src/main/java/com/cflint/CFLint.java @@ -156,6 +156,7 @@ public CFLint(ConfigRuntime configuration,final CFLintScanner... bugsScanners) { allowedExtensions.add(cfcExtension); allowedExtensions.add(cfmExtenstion); } + cfmlParser.setErrorReporter(this); } public void scan(final String folder) { @@ -791,7 +792,7 @@ public void syntaxError(Recognizer recognizer, } } if(recognizer instanceof Parser && ((Parser)recognizer).getExpectedTokens().contains(65)){ - bugs.add(new BugInfo.BugInfoBuilder().setMessageCode("PARSE_ERROR") + bugs.add(new BugInfo.BugInfoBuilder().setMessageCode("MISSING_SEMI") .setFilename(file).setMessage("End of statement(;) expected instead of " + expression).setSeverity("ERROR") .setExpression(expression) .setLine(line).setColumn(charPositionInLine) diff --git a/src/test/java/com/cflint/TestParsingErrors.java b/src/test/java/com/cflint/TestParsingErrors.java index c8b94ef7b..c388376a8 100644 --- a/src/test/java/com/cflint/TestParsingErrors.java +++ b/src/test/java/com/cflint/TestParsingErrors.java @@ -40,18 +40,27 @@ public void testMissingSemiColon() throws ParseException, IOException{ "\r\n" + " \r\n" + " var xx = 123\r\n" + - " yy = 123\r\n" + + " yy = 123;\r\n" + " \r\n" + "\r\n" + ""; cfBugs.process(cfcSrc,"test"); - assertEquals(1,cfBugs.getBugs().getFlatBugList().size()); - List result = cfBugs.getBugs().getFlatBugList(); + + assertEquals(2,cfBugs.getBugs().getFlatBugList().size()); + System.out.println(cfBugs.getBugs().getFlatBugList()); + final List result = cfBugs.getBugs().getBugList().get("MISSING_SEMI"); assertEquals(1,result.size()); - assertEquals("MISSING_VAR",result.get(0).getMessageCode()); - assertEquals("yy",result.get(0).getVariable()); + assertEquals("MISSING_SEMI",result.get(0).getMessageCode()); assertEquals(5,result.get(0).getLine()); assertEquals("ERROR",result.get(0).getSeverity()); - assertEquals("Variable yy is not declared with a var statement.",result.get(0).getMessage()); + assertEquals("End of statement(;) expected instead of yy",result.get(0).getMessage()); + + final List result2 = cfBugs.getBugs().getBugList().get("MISSING_VAR"); + assertEquals(1,result2.size()); + assertEquals("MISSING_VAR",result2.get(0).getMessageCode()); + assertEquals("yy",result2.get(0).getVariable()); + assertEquals(5,result2.get(0).getLine()); + assertEquals("ERROR",result2.get(0).getSeverity()); + assertEquals("Variable yy is not declared with a var statement.",result2.get(0).getMessage()); } } From 0cc25c61d9cd1df284590142ca840d59501377ca Mon Sep 17 00:00:00 2001 From: ryaneberly Date: Mon, 19 Oct 2015 22:08:39 -0400 Subject: [PATCH 12/13] fix test --- src/test/java/com/cflint/TestComponentLengthChecker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/cflint/TestComponentLengthChecker.java b/src/test/java/com/cflint/TestComponentLengthChecker.java index 62e47671f..8c94f6fe5 100644 --- a/src/test/java/com/cflint/TestComponentLengthChecker.java +++ b/src/test/java/com/cflint/TestComponentLengthChecker.java @@ -671,7 +671,7 @@ public void testBadFunction() throws ParseException, IOException{ "}\r\n" + "}"; cfBugs.process(cfcSrc,"test"); - final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + final List result = cfBugs.getBugs().getBugList().get("EXCESSIVE_COMPONENT_LENGTH"); assertEquals(1, result.size()); assertEquals("EXCESSIVE_COMPONENT_LENGTH", result.get(0).getMessageCode()); } From 567c2d20f2995918d419c2e493498edb210c5820 Mon Sep 17 00:00:00 2001 From: ryaneberly Date: Mon, 19 Oct 2015 22:15:46 -0400 Subject: [PATCH 13/13] fix test --- src/test/java/com/cflint/TestCFBugs_SimpleComplexity.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/cflint/TestCFBugs_SimpleComplexity.java b/src/test/java/com/cflint/TestCFBugs_SimpleComplexity.java index a4ba37c96..7470cb950 100644 --- a/src/test/java/com/cflint/TestCFBugs_SimpleComplexity.java +++ b/src/test/java/com/cflint/TestCFBugs_SimpleComplexity.java @@ -142,7 +142,7 @@ public void testComplexScriptBased() throws ParseException, IOException { + "}\r\n" + "}"; cfBugs.process(cfcSrc, "test"); - final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + final List result = cfBugs.getBugs().getBugList().get("FUNCTION_TOO_COMPLEX"); assertEquals(1, result.size()); assertEquals("FUNCTION_TOO_COMPLEX", result.get(0).getMessageCode()); assertEquals(2, result.get(0).getLine());