diff --git a/src/main/java/com/cflint/CFLint.java b/src/main/java/com/cflint/CFLint.java index 2fc4fa009..e5c98cec7 100644 --- a/src/main/java/com/cflint/CFLint.java +++ b/src/main/java/com/cflint/CFLint.java @@ -154,7 +154,6 @@ public CFLint(ConfigRuntime configuration,final CFLintScanner... bugsScanners) { bugs = new BugList(filter); } catch (IOException e1) { e1.printStackTrace(); - bugs = new BugList(null); } if(exceptionListeners.size() == 0){ addExceptionListener(new DefaultCFLintExceptionListener(bugs)); @@ -292,10 +291,32 @@ private void process(final Element elem, final String space, Context context) currentElement.push(elem); if (elem.getName().equalsIgnoreCase("cfcomponent")) { + inComponent = true; + handler.push("component"); + + for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { + try{ + structurePlugin.startComponent(context, bugs); + }catch(Exception e){ + e.printStackTrace(); + } + } + + context.setInComponent(true); context.setComponentName(elem.getAttributeValue("displayname")); } else if (elem.getName().equalsIgnoreCase("cffunction")) { context.setFunctionName(elem.getAttributeValue("name")); + inFunction = true; + handler.push("function"); + + for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { + try{ + structurePlugin.startFunction(context, bugs); + }catch(Exception e){ + e.printStackTrace(); + } + } } try{ @@ -379,47 +400,23 @@ else if (elem.getName().equalsIgnoreCase("cffunction")) { } if (elem.getName().equalsIgnoreCase("cffunction")) { - inFunction = true; - handler.push("function"); - - for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { - try{ - structurePlugin.startFunction(context, bugs); - }catch(Exception e){ - e.printStackTrace(); - bugs = new BugList(null); - } - } processStack(elem.getChildElements(), space + " ", context); for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { try{ structurePlugin.endFunction(context, bugs); }catch(Exception e){ e.printStackTrace(); - bugs = new BugList(null); } } inFunction = false; handler.pop(); } else if (elem.getName().equalsIgnoreCase("cfcomponent")) { - inComponent = true; - handler.push("component"); - for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { - try{ - structurePlugin.startComponent(context, bugs); - }catch(Exception e){ - e.printStackTrace(); - bugs = new BugList(null); - } - } - context.setInComponent(true); processStack(elem.getChildElements(), space + " ", context); for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { try{ structurePlugin.endComponent(context, bugs); }catch(Exception e){ e.printStackTrace(); - bugs = new BugList(null); } } @@ -479,11 +476,33 @@ private void process(final CFScriptStatement expression, final String filename, final Context context = new Context(filename, elem, functionName, inAssignment, handler); context.setInComponent(inComponent); - - if (expression instanceof CFFuncDeclStatement) { + if (expression instanceof CFCompDeclStatement) { + inComponent = true; + //do startComponent notifications + for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { + try{ + structurePlugin.startComponent(context, bugs); + }catch(Exception e){ + e.printStackTrace(); + } + } + } + else if (expression instanceof CFFuncDeclStatement) { final CFFuncDeclStatement function = (CFFuncDeclStatement) expression; functionName = function.getName().getName(); context.setFunctionName(functionName); + inFunction = true; + handler.push("function"); + for (final CFFunctionParameter param : function.getFormals()) { + handler.addArgument(param.getName()); + } + for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { + try{ + structurePlugin.startFunction(context, bugs); + }catch(Exception e){ + e.printStackTrace(); + } + } } for (final CFLintScanner plugin : extensions) { @@ -508,16 +527,6 @@ private void process(final CFScriptStatement expression, final String filename, } else if (expression instanceof CFExpressionStatement) { process(((CFExpressionStatement) expression).getExpression(), filename, elem, functionName); } else if (expression instanceof CFCompDeclStatement) { - inComponent = true; - //do startComponent notifications - for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { - try{ - structurePlugin.startComponent(context, bugs); - }catch(Exception e){ - e.printStackTrace(); - bugs = new BugList(null); - } - } //process the component declaration process(((CFCompDeclStatement) expression).getBody(), filename, elem, functionName); //do endComponent notifications @@ -526,7 +535,6 @@ private void process(final CFScriptStatement expression, final String filename, structurePlugin.endComponent(context, bugs); }catch(Exception e){ e.printStackTrace(); - bugs = new BugList(null); } } @@ -564,31 +572,12 @@ private void process(final CFScriptStatement expression, final String filename, } else if (expression instanceof CFFuncDeclStatement) { final CFFuncDeclStatement function = (CFFuncDeclStatement) expression; - inFunction = true; - handler.push("function"); -// if ("init".equalsIgnoreCase(function.getName())) { -// inFunction = false; -// } - - for (final CFFunctionParameter param : function.getFormals()) { - handler.addArgument(param.getName()); - } - for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { - try{ - structurePlugin.startFunction(context, bugs); - }catch(Exception e){ - e.printStackTrace(); - bugs = new BugList(null); - } - } - process(function.getBody(), filename, elem, function.getName()); for (final CFLintStructureListener structurePlugin : getStructureListeners(extensions)) { try{ structurePlugin.endFunction(context, bugs); }catch(Exception e){ e.printStackTrace(); - bugs = new BugList(null); } } inFunction = false; @@ -889,7 +878,6 @@ protected void fireStartedProcessing(final String srcidentifier) { structurePlugin.startFile(srcidentifier, bugs); }catch(Exception e){ e.printStackTrace(); - bugs = new BugList(null); } } for (final ScanProgressListener p : scanProgressListeners) { @@ -903,7 +891,6 @@ protected void fireFinishedProcessing(final String srcidentifier) { structurePlugin.endFile(srcidentifier, bugs); }catch(Exception e){ e.printStackTrace(); - bugs = new BugList(null); } } for (final ScanProgressListener p : scanProgressListeners) { diff --git a/src/main/java/com/cflint/plugins/core/UnusedArgumentChecker.java b/src/main/java/com/cflint/plugins/core/UnusedArgumentChecker.java new file mode 100644 index 000000000..95ddff2a3 --- /dev/null +++ b/src/main/java/com/cflint/plugins/core/UnusedArgumentChecker.java @@ -0,0 +1,95 @@ +package com.cflint.plugins.core; + +import java.util.Map; +import java.util.HashMap; + +import net.htmlparser.jericho.Element; + +import cfml.parsing.cfscript.CFAssignmentExpression; +import cfml.parsing.cfscript.CFExpression; +import cfml.parsing.cfscript.CFIdentifier; +import cfml.parsing.cfscript.CFFullVarExpression; +import cfml.parsing.cfscript.CFVarDeclExpression; +import cfml.parsing.cfscript.script.CFFuncDeclStatement; +import cfml.parsing.cfscript.script.CFFunctionParameter; +import cfml.parsing.cfscript.script.CFScriptStatement; + +import com.cflint.BugInfo; +import com.cflint.BugList; +import com.cflint.plugins.CFLintScannerAdapter; +import com.cflint.plugins.Context; + +public class UnusedArgumentChecker extends CFLintScannerAdapter { + final String severity = "INFO"; + + protected Map methodArguments = new HashMap(); + protected Map argumentLineNo = new HashMap(); + + @Override + public void element(final Element element, final Context context, final BugList bugs) { + if (element.getName().equals("cfargument")) { + final String name = element.getAttributeValue("name"); + methodArguments.put(name, false); + setArgumentLineNo(name, context.startLine()); + } + } + + @Override + public void expression(final CFScriptStatement expression, final Context context, final BugList bugs) { + if (expression instanceof CFFuncDeclStatement) { + final CFFuncDeclStatement function = (CFFuncDeclStatement) expression; + for (final CFFunctionParameter argument : function.getFormals()) { + final String name = argument.getName(); + methodArguments.put(name, false); + setArgumentLineNo(name, function.getLine()); // close enough? + } + } + } + + protected void setArgumentLineNo(final String argument, final Integer lineNo) { + if (argumentLineNo.get(argument) == null) { + argumentLineNo.put(argument, lineNo); + } + } + + @Override + public void expression(final CFExpression expression, final Context context, final BugList bugs) { + if (expression instanceof CFFullVarExpression) { + CFExpression variable = ((CFFullVarExpression) expression).getExpressions().get(0); + if (variable instanceof CFIdentifier) { + String name = ((CFIdentifier) variable).getName(); + if (methodArguments.get(name) != null) { + methodArguments.put(name, true); + } + } + } + else if (expression instanceof CFIdentifier) { + String name = ((CFIdentifier) expression).getName(); + if (methodArguments.get(name) != null) { + methodArguments.put(name, true); + } + } + } + + @Override + public void startFunction(Context context, BugList bugs) { + methodArguments.clear(); + } + + @Override + public void endFunction(Context context, BugList bugs) { + // TODO perhaps sort by line number? + for (Map.Entry method : methodArguments.entrySet()) { + Boolean used = method.getValue(); + if (!used) { + final String name = method.getKey(); + final Integer lineNo = argumentLineNo.get(name); + bugs.add(new BugInfo.BugInfoBuilder().setLine(lineNo).setMessageCode("UNUSED_METHOD_ARGUMENT") + .setSeverity(severity).setFilename(context.getFilename()) + .setMessage("Argument " + name + " is not used in function " + context.getFunctionName() + ", consider removing it.") + .build()); + } + } + } + +} diff --git a/src/main/resources/cflint.definition.json b/src/main/resources/cflint.definition.json index db26948f1..d0a1bbd3d 100644 --- a/src/main/resources/cflint.definition.json +++ b/src/main/resources/cflint.definition.json @@ -729,6 +729,17 @@ "parameter": [] }, { + "name": "UnusedArgumentChecker", + "className": "UnusedArgumentChecker", + "message": [ + { + "code": "UNUSED_METHOD_ARGUMENT", + "severity": "INFO" + } + ], + "parameter": [] + }, + { "name": "CFCompareVsAssignChecker", "className": "CFCompareVsAssignChecker", "message": [ diff --git a/src/main/resources/cflint.definition.xml b/src/main/resources/cflint.definition.xml index 7eff52579..bfc1c7a8a 100644 --- a/src/main/resources/cflint.definition.xml +++ b/src/main/resources/cflint.definition.xml @@ -333,6 +333,11 @@ WARNING Avoid leaving debug attribute on tags. + + + + INFO + diff --git a/src/test/java/com/cflint/TestUnusedArgumentChecker.java b/src/test/java/com/cflint/TestUnusedArgumentChecker.java new file mode 100644 index 000000000..7b4c1a7a1 --- /dev/null +++ b/src/test/java/com/cflint/TestUnusedArgumentChecker.java @@ -0,0 +1,230 @@ +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.UnusedArgumentChecker; + +public class TestUnusedArgumentChecker { + + private CFLint cfBugs; + + @Before + public void setUp() { + final ConfigRuntime conf = new ConfigRuntime(); + final PluginInfoRule pluginRule = new PluginInfoRule(); + pluginRule.setName("UnusedArgumentChecker"); + conf.getRules().add(pluginRule); + final PluginMessage pluginMessage = new PluginMessage("UNUSED_METHOD_ARGUMENT"); + pluginMessage.setSeverity("INFO"); + cfBugs = new CFLint(conf, new UnusedArgumentChecker()); + } + + @Test + public void testNoArgumentsInScript() throws ParseException, IOException { + final String scriptSrc = "\r\n" + + "component {\r\n" + + "function dummyFunction() {\r\n" + + "}\r\n" + + "}\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + final Map> result = cfBugs.getBugs().getBugList(); + assertEquals(0, result.size()); + } + + @Test + public void testArgumentsUsedInScript() throws ParseException, IOException { + final String scriptSrc = "\r\n" + + "component {\r\n" + + "function sum(a,b) {\r\n" + + "return a + b;\r\n" + + "}\r\n" + + "}\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + final Map> result = cfBugs.getBugs().getBugList(); + assertEquals(0, result.size()); + } + + @Test + public void testArgumentsNotUsedInScript() throws ParseException, IOException { + final String scriptSrc = "\r\n" + + "component {\r\n" + + "function sum(a,b) {\r\n" + + "c = a + b;" + + "return c;\r\n" + + "}\r\n" + + "}\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + final Map> result = cfBugs.getBugs().getBugList(); + assertEquals(0, result.size()); + } + + @Test + public void testArgumentsNotUsedInMethodScript() throws ParseException, IOException { + final String scriptSrc = "\r\n" + + "component {\r\n" + + "function sum(a,b) {\r\n" + + "return other(a,b);\r\n" + + "}\r\n" + + "}\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + final Map> result = cfBugs.getBugs().getBugList(); + assertEquals(0, result.size()); + } + + @Test + public void testArgumentsNotUsedInReturnScript() throws ParseException, IOException { + final String scriptSrc = "\r\n" + + "component {\r\n" + + "function sum(a,b) {\r\n" + + "return a;\r\n" + + "}\r\n" + + "}\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("UNUSED_METHOD_ARGUMENT", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + } + + @Test + public void testMultipleArgumentsNotUsedScript() throws ParseException, IOException { + final String scriptSrc = "\r\n" + + "component {\r\n" + + "function sum(a,b,c,d) {\r\n" + + "return a;\r\n" + + "}\r\n" + + "}\r\n" + + ""; + + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(3, result.size()); + assertEquals("UNUSED_METHOD_ARGUMENT", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("UNUSED_METHOD_ARGUMENT", result.get(1).getMessageCode()); + assertEquals(3, result.get(1).getLine()); + assertEquals("UNUSED_METHOD_ARGUMENT", result.get(2).getMessageCode()); + assertEquals(3, result.get(2).getLine()); + } + + @Test + public void testNoArgumentsInTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + "\r\n"; + + cfBugs.process(tagSrc, "test"); + final Map> result = cfBugs.getBugs().getBugList(); + assertEquals(0, result.size()); + } + + @Test + public void testArgumentsUsedInTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n"; + + cfBugs.process(tagSrc, "test"); + final Map> result = cfBugs.getBugs().getBugList(); + assertEquals(0, result.size()); + } + + @Test + public void testArgumentsNotUsedInTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "" + + "\r\n" + + "\r\n" + + "\r\n"; + + cfBugs.process(tagSrc, "test"); + final Map> result = cfBugs.getBugs().getBugList(); + assertEquals(0, result.size()); + } + + @Test + public void testArgumentsNotUsedInMethodTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n"; + + cfBugs.process(tagSrc, "test"); + final Map> result = cfBugs.getBugs().getBugList(); + assertEquals(0, result.size()); + } + + @Test + public void testArgumentsNotUsedInReturnTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n"; + + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("UNUSED_METHOD_ARGUMENT", result.get(0).getMessageCode()); + assertEquals(4, result.get(0).getLine()); + } + + @Test + public void testMultipleArgumentsNotUsedTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n" + + "\r\n"; + + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(3, result.size()); + assertEquals("UNUSED_METHOD_ARGUMENT", result.get(0).getMessageCode()); + assertEquals(6, result.get(0).getLine()); + assertEquals("UNUSED_METHOD_ARGUMENT", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("UNUSED_METHOD_ARGUMENT", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); + } + +}