Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule to check for unused arguments in methods #127

Merged
merged 6 commits into from
Nov 8, 2015
Merged

Rule to check for unused arguments in methods #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6e0820b
Add tests for rule to check for unused function arguments
justinmclean Nov 1, 2015
377a7ce
Add rule to check for unused function / method arguments
justinmclean Nov 1, 2015
30059b6
Move startFunction and startComponent to before the component or func…
justinmclean Nov 1, 2015
a93179e
addd unused arguments to config files
justinmclean Nov 5, 2015
9512cba
Don't remove all bugs when an exception occurs.
justinmclean Nov 8, 2015
6da0622
Merge branch 'dev' into unusedArguments
justinmclean Nov 8, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 46 additions & 59 deletions src/main/java/com/cflint/CFLint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
95 changes: 95 additions & 0 deletions src/main/java/com/cflint/plugins/core/UnusedArgumentChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<String, Boolean> methodArguments = new HashMap<String, Boolean>();
protected Map<String, Integer> argumentLineNo = new HashMap<String, Integer>();

@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<String, Boolean> 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());
}
}
}

}
11 changes: 11 additions & 0 deletions src/main/resources/cflint.definition.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,17 @@
"parameter": []
},
{
"name": "UnusedArgumentChecker",
"className": "UnusedArgumentChecker",
"message": [
{
"code": "UNUSED_METHOD_ARGUMENT",
"severity": "INFO"
}
],
"parameter": []
},
{
"name": "CFCompareVsAssignChecker",
"className": "CFCompareVsAssignChecker",
"message": [
Expand Down
5 changes: 5 additions & 0 deletions src/main/resources/cflint.definition.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,11 @@
<severity>WARNING</severity>
<messageText>Avoid leaving debug attribute on tags.</messageText>
</message>
</ruleImpl>
<ruleImpl name="UnusedArgumentChecker" className="UnusedArgumentChecker">
<message code="UNUSED_LOCAL_VARIABLE">
<severity>INFO</severity>
</message>
</ruleImpl>
<ruleImpl name="CFCompareVsAssignChecker" className="CFCompareVsAssignChecker">
<message code="COMPARE_INSTEAD_OF_ASSIGN">
Expand Down
Loading