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.

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

#550 arg types added, functions false positive fixed. #563

Merged
merged 1 commit into from
May 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions src/main/java/com/cflint/CFLint.java
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ public void scan(final File folderOrFile) {
}

protected void printException(final Exception e, final Element... elem) {
e.printStackTrace();
if (!quiet) {
if (elem != null && elem.length > 0 && elem[0] != null) {
final int line = elem[0].getSource().getRow(elem[0].getBegin());
Expand Down Expand Up @@ -1331,9 +1332,11 @@ public void reportRule(final Element elem, final Object currentExpression, final
//A bit of a hack to fix the offset issue
//This could be handled better at the source where line and offset are calc'd.
int idxOffSet = 1;
if(lineOffsets != null && msg.getLine()!=null && msg.getOffset() != null && msg.getOffset() >=lineOffsets[msg.getLine()] ){
idxOffSet=0;
}
try{
if(lineOffsets != null && msg.getLine()!=null && msg.getOffset() != null && msg.getOffset() >=lineOffsets[msg.getLine()] ){
idxOffSet=0;
}
}catch(ArrayIndexOutOfBoundsException e){}
if (expression instanceof CFExpression) {
final BugInfo bugInfo = bldr.build((CFExpression) expression, elem);
final Token token = ((CFExpression) expression).getToken();
Expand Down
78 changes: 44 additions & 34 deletions src/main/java/com/cflint/plugins/core/UnusedArgumentChecker.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Map.Entry;

import com.cflint.BugList;
import com.cflint.CF;
Expand All @@ -11,6 +12,7 @@

import cfml.parsing.cfscript.CFExpression;
import cfml.parsing.cfscript.CFFullVarExpression;
import cfml.parsing.cfscript.CFFunctionExpression;
import cfml.parsing.cfscript.CFIdentifier;
import cfml.parsing.cfscript.script.CFFuncDeclStatement;
import cfml.parsing.cfscript.script.CFFunctionParameter;
Expand All @@ -20,22 +22,32 @@
public class UnusedArgumentChecker extends CFLintScannerAdapter {

// Use linked hash map to preserve the order of the elements.
protected Map<String, Boolean> methodArguments = new LinkedHashMap<>();
protected Map<String, Integer> argumentLineNo = new HashMap<>();
protected Map<String, Integer> argumentOffset = new HashMap<>();

// protected Map<String, Boolean> methodArguments = new LinkedHashMap<>();
// protected Map<String, Integer> argumentLineNo = new HashMap<>();
// protected Map<String, Integer> argumentOffset = new HashMap<>();
protected Map<String, ArgInfo> currentArgs = new LinkedHashMap<>();

static class ArgInfo{
Boolean used = false;
Integer argumentLineNo;
Integer argumentOffset;
String type;
}

@Override
public void element(final Element element, final Context context, final BugList bugs) {
if (element.getName().equals(CF.CFARGUMENT)) {
final String name = element.getAttributeValue(CF.NAME) != null
? element.getAttributeValue(CF.NAME).toLowerCase() : "";
methodArguments.put(name, false);
setArgumentLineNo(name, context.startLine());
setArgumentOffset(name, element.getAttributeValue(CF.NAME) != null
? element.getAttributes().get(CF.NAME).getValueSegment().getBegin() : element.getBegin());
ArgInfo argInfo = new ArgInfo();
argInfo.argumentLineNo=context.startLine();
argInfo.argumentOffset=element.getAttributeValue(CF.NAME) != null
? element.getAttributes().get(CF.NAME).getValueSegment().getBegin() : element.getBegin();
argInfo.type=element.getAttributeValue(CF.TYPE);
currentArgs.put(name, argInfo);
final String code = element.getParentElement().toString();
if (isUsed(code, name)) {
methodArguments.put(name, true);
argInfo.used=true;
}
}
}
Expand All @@ -47,28 +59,18 @@ public void expression(final CFScriptStatement expression, final Context context
for (final CFFunctionParameter argument : function.getFormals()) {
final String name = argument.getName().toLowerCase();
// CF variable names are not case sensitive
methodArguments.put(name, false);
setArgumentLineNo(name, function.getLine());
setArgumentOffset(name, context.offset() + argument.getOffset() );
ArgInfo argInfo = new ArgInfo();
argInfo.argumentLineNo=function.getLine();
argInfo.argumentOffset=context.offset() + argument.getOffset();
argInfo.type=argument.getType();
currentArgs.put(name, argInfo);
if (isUsed(function.Decompile(0), name)) {
methodArguments.put(name, true);
argInfo.used=true;
}
}
}
}

protected void setArgumentLineNo(final String argument, final Integer lineNo) {
if (argumentLineNo.get(argument) == null) {
argumentLineNo.put(argument, lineNo);
}
}

protected void setArgumentOffset(final String argument, final Integer offset) {
if (argumentOffset.get(argument) == null) {
argumentOffset.put(argument, offset);
}
}

protected void useIdentifier(final CFFullVarExpression fullVarExpression) {
if (!fullVarExpression.getExpressions().isEmpty()) {
final CFExpression identifier1 = fullVarExpression.getExpressions().get(0);
Expand All @@ -88,8 +90,15 @@ protected void useIdentifier(final CFFullVarExpression fullVarExpression) {

protected void useIdentifier(final CFIdentifier identifier) {
final String name = identifier.getName().toLowerCase();
if (methodArguments.get(name) != null) {
methodArguments.put(name, true);
if (currentArgs.get(name) != null) {
currentArgs.get(name).used=true;
}
}

protected void useIdentifier(final CFFunctionExpression identifier) {
final String name = identifier.getName().toLowerCase();
if (currentArgs.get(name) != null && CF.FUNCTION.equalsIgnoreCase(currentArgs.get(name).type)) {
currentArgs.get(name).used=true;
}
}

Expand All @@ -99,24 +108,25 @@ public void expression(final CFExpression expression, final Context context, fin
useIdentifier((CFFullVarExpression) expression);
} else if (expression instanceof CFIdentifier) {
useIdentifier((CFIdentifier) expression);
} else if(expression instanceof CFFunctionExpression){
useIdentifier((CFFunctionExpression) expression);
}
}

@Override
public void startFunction(final Context context, final BugList bugs) {
methodArguments.clear();
argumentLineNo.clear();
currentArgs.clear();
}

@Override
public void endFunction(final Context context, final BugList bugs) {
// sort by line number
for (final Map.Entry<String, Boolean> method : methodArguments.entrySet()) {
for (final Entry<String, ArgInfo> method : currentArgs.entrySet()) {
final String name = method.getKey();
final Boolean used = method.getValue();
final int offset = argumentOffset.get(name);
if (!used) {
context.addMessage("UNUSED_METHOD_ARGUMENT", name, argumentLineNo.get(name), offset);
final ArgInfo argInfo = method.getValue();
final int offset = argInfo.argumentOffset;
if (!argInfo.used) {
context.addMessage("UNUSED_METHOD_ARGUMENT", name, argInfo.argumentLineNo, offset);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
component{
public string function myFunction(function doSomething){
return doSomething();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"version" : "",
"timestamp" : 1507944141,
"issues" : [ ],
"counts" : {
"totalFiles" : 0,
"totalLines" : 0,
"countByCode" : [ ],
"countBySeverity" : [ ]
}
}