Merge pull request #109 from cflint/naming

Provide some rules around naming of variables and methods

src/main/java/com/cflint/CFLint.java

@@ -425,12 +425,19 @@ public String stripLineComments(final String cfscript) {
 
 	private void process(final CFScriptStatement expression, final String filename, final Element elem,
 			final CFIdentifier functionName) {
-		process(expression,filename,elem,functionName.Decompile(0));
+		process(expression,filename,elem,functionName.getName());
 	}
 	private void process(final CFScriptStatement expression, final String filename, final Element elem,
-			final String functionName) {
+			String functionName) {
 		final Context context = new Context(filename, elem, functionName, inAssignment, handler);
+
 		context.setInComponent(inComponent);
+
+		if (expression instanceof CFFuncDeclStatement) {
+			final CFFuncDeclStatement function = (CFFuncDeclStatement) expression;
+			functionName = function.getName().getName();
+			context.setFunctionName(functionName);
+		}
 
 		for (final CFLintScanner plugin : extensions) {
 			try{

src/main/java/com/cflint/plugins/Context.java

@@ -147,7 +147,7 @@ public int startLine() {
 		if(element != null && element.getSource() !=null)
 			return element.getSource().getRow(element.getBegin());
 		else 
-			return 1;
+			return 1; // not zero
 	}
 
 	protected String componentFromFile(String filename) {

src/main/java/com/cflint/plugins/core/MethodNameChecker.java

@@ -0,0 +1,106 @@
+package com.cflint.plugins.core;
+
+import ro.fortsoft.pf4j.Extension;
+import net.htmlparser.jericho.Element;
+import net.htmlparser.jericho.Attributes;
+import cfml.parsing.cfscript.script.CFScriptStatement;
+import cfml.parsing.cfscript.script.CFFuncDeclStatement;
+
+
+import com.cflint.BugInfo;
+import com.cflint.BugList;
+import com.cflint.plugins.CFLintScannerAdapter;
+import com.cflint.plugins.Context;
+
+import java.util.regex.Pattern;
+
+@Extension
+public class MethodNameChecker extends CFLintScannerAdapter {
+	final String severity = "INFO";
+
+	@Override	
+	public void expression(final CFScriptStatement expression, final Context context, final BugList bugs) {
+		if (expression instanceof CFFuncDeclStatement) {
+			final CFFuncDeclStatement method = (CFFuncDeclStatement) expression;
+			int lineNo = method.getLine() + context.startLine() - 1;
+			checkNameForBugs(context.getFunctionName(), context.getFilename(), lineNo, bugs);
+		}
+	}
+
+	@Override
+	public void element(final Element element, final Context context, final BugList bugs) {
+		if (element.getName().equals("cffunction")) {
+			int lineNo = element.getSource().getRow(element.getBegin());
+			checkNameForBugs(context.getFunctionName(), context.getFilename(), lineNo, bugs);
+		}
+	}
+
+	public void checkNameForBugs(String method, String filename, int line, BugList bugs) {
+		int minMethodLength = ValidName.MIN_METHOD_LENGTH;
+		int maxMethodLength = ValidName.MAX_METHOD_LENGTH;
+		int maxMethodWords = ValidName.MAX_METHOD_WORDS;
+
+		if (getParameter("MinLength") != null) {
+			try {
+				minMethodLength = Integer.parseInt(getParameter("MinLength"));
+			} catch(Exception e) {}
+		}
+
+		if (getParameter("MaxLength") != null) {
+			try {
+				maxMethodLength = Integer.parseInt(getParameter("MaxLength"));
+			} catch(Exception e) {}
+		}
+
+		if (getParameter("MaxWords") != null) {
+			try {
+				maxMethodWords = Integer.parseInt(getParameter("MaxWords"));
+			} catch(Exception e) {}
+		}
+
+		ValidName name = new ValidName(minMethodLength, maxMethodLength, maxMethodWords);
+
+		if (name.isInvalid(method)) {
+			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("METHOD_INVALID_NAME")
+				.setSeverity(severity).setFilename(filename)
+				.setMessage("Method name " + method + " is not a valid name. Please use CamelCase or underscores.")
+				.build());
+		}
+		if (name.isUpperCase(method)) {
+			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("METHOD_ALLCAPS_NAME")
+				.setSeverity(severity).setFilename(filename)
+				.setMessage("Method name " + method + " should not be upper case.")
+				.build());
+		}
+		if (name.tooShort(method)) {
+			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("METHOD_TOO_SHORT")
+				.setSeverity(severity).setFilename(filename)
+				.setMessage("Method name " + method + " should be longer than " + minMethodLength + " characters.")
+				.build());
+		}
+		if (name.tooLong(method)) {
+			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("METHOD_TOO_LONG")
+				.setSeverity(severity).setFilename(filename)
+				.setMessage("Method name " + method + " should be shorter than " + maxMethodLength + " characters.")
+				.build());
+		}
+		if (!name.isUpperCase(method) && name.tooWordy(method)) {
+			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("METHOD_TOO_WORDY")
+				.setSeverity(severity).setFilename(filename)
+				.setMessage("Method name " + method + " is too wordy, can you think of a more concise name?")
+				.build());
+		}
+		if (name.isTemporary(method)) {
+			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("METHOD_IS_TEMPORARY")
+				.setSeverity(severity).setFilename(filename)
+				.setMessage("Method name " + method + " could be named better.")
+				.build());
+		}
+		if (name.hasPrefixOrPostfix(method)) {
+			bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("METHOD_HAS_PREFIX_OR_POSTFIX")
+				.setSeverity(severity).setFilename(filename)
+				.setMessage("Method name has prefix or postfix " + method + " and could be named better.")
+				.build());
+		}
+	}
+}

src/main/java/com/cflint/plugins/core/ValidName.java

@@ -0,0 +1,132 @@
+
+package com.cflint.plugins.core;
+
+import java.util.regex.Pattern;
+
+public class ValidName {
+	public static final int MIN_VAR_LENGTH = 3;
+	public static final int MAX_VAR_LENGTH = 20;
+	public static final int MAX_VAR_WORDS = 4;
+
+	public static final int MIN_METHOD_LENGTH = 3;
+	public static final int MAX_METHOD_LENGTH = 25;
+	public static final int MAX_METHOD_WORDS = 5;
+
+	protected int minLength = MIN_VAR_LENGTH;
+	protected int maxLength = MAX_VAR_LENGTH;
+	protected int maxWords = MAX_VAR_WORDS;
+
+	public ValidName() {
+	}
+
+	public ValidName(int minLength, int maxLength, int maxWords) {
+		this.minLength = minLength;
+		this.maxLength = maxLength;
+		this.maxWords = maxWords;
+	}
+
+	public boolean isInvalid(String name) {
+		return !validChars(name) || endsInNumber(name) || !(isSameCase(name) || isCamelCaseLower(name) || usesUnderscores(name));
+	}
+
+	public boolean validChars(String name) {
+		 Pattern valid = Pattern.compile("^[A-Za-z0-9_]+$");
+		 return valid.matcher(name).matches();
+	}
+
+	public boolean isUpperCase(String name) {
+		return name.toUpperCase().equals(name);
+	}
+
+	public boolean isSameCase(String name) {
+		return name.equals(name.toLowerCase()) || name.equals(name.toUpperCase());
+	}
+
+	public boolean isCamelCaseLower(String name) {
+		// [A-Z0-9]{2,5} catch names like productID, phone4G, requestURL etc etc
+		Pattern valid = Pattern.compile("^[a-z0-9]+([A-Z]{1,5}[a-z0-9]+)*([A-Z0-9]{2,5}){0,1}$");
+		return valid.matcher(name).matches();
+	}
+
+	public boolean isCamelCaseUpper(String name) {
+		Pattern valid = Pattern.compile("^([A-Z]{1,5}[a-z0-9]+)+([A-Z0-9]{2,5}){0,1}$");
+		return valid.matcher(name).matches();
+	}
+
+	public boolean usesUnderscores(String name) {
+		return name.indexOf('_') != -1;
+	}
+
+	public boolean endsInNumber(String name) {
+		char lastLetter = name.charAt(name.length() - 1);
+
+		if (Character.isDigit(lastLetter)) {
+			return true;
+		}
+
+		return false;
+	}
+
+	public boolean tooShort(String name) {
+		return name.length() < minLength;
+	}
+
+	public boolean tooLong(String name) {
+		return name.length() > maxLength;
+	}
+
+	public boolean tooWordy(String name) {
+		String[] words = name.split("[A-Z_]+");
+		int count = 0;
+
+		for (int i = 0; i < words.length; i++) {
+			if (words[i] != null && !words[i].equals("")) {
+				count++;
+			}
+		}
+
+		return count > maxWords;
+	}
+
+	public boolean isTemporary(String name) {
+		String[] wordsToAvoid = {"temp", "tmp", "var", "func", "obj", "object", "bool", "struct", "string", "array", "comp"};
+		String sentence = name.replaceAll("_", " ");
+		sentence = sentence.replaceAll("(\\p{Ll})(\\p{Lu})","$1 $2");
+		String[] words = sentence.split(" ");
+
+		for (String badWord : wordsToAvoid) {
+			for (String word : words) {
+				if (word.toLowerCase().equals(badWord))
+				{
+					return true;
+				}
+			}
+		}
+
+		return false;
+	}
+
+	public boolean hasPrefixOrPostfix(String name) {
+		String[] namesToAvoid = {"s", "st", "str", "o", "obj", "b", "q", "a", "arr", "this", "my"};
+		String sentence = name.replaceAll("_", " ");
+		sentence = sentence.replaceAll("(\\p{Ll})(\\p{Lu})","$1 $2");
+		String[] words = sentence.split(" ");
+		String firstWord = words[0];
+		String lastWord = words[words.length-1];
+
+		if (words.length > 1) {
+			for (String badName : namesToAvoid) {
+				if (firstWord.toLowerCase().equals(badName))
+				{
+					return true;
+				}
+				if (lastWord.toLowerCase().equals(badName))
+				{
+					return true;
+				}
+			}
+		}
+
+		return false;
+	}
+}

src/main/java/com/cflint/plugins/core/VariableNameChecker.java

@@ -0,0 +1,111 @@
+package com.cflint.plugins.core;
+
+import ro.fortsoft.pf4j.Extension;
+import net.htmlparser.jericho.Element;
+import net.htmlparser.jericho.Attributes;
+import cfml.parsing.cfscript.CFAssignmentExpression;
+import cfml.parsing.cfscript.CFExpression;
+import cfml.parsing.cfscript.CFFullVarExpression;
+import cfml.parsing.cfscript.CFIdentifier;
+import cfml.parsing.cfscript.CFVarDeclExpression;
+
+import com.cflint.BugInfo;
+import com.cflint.BugList;
+import com.cflint.plugins.CFLintScannerAdapter;
+import com.cflint.plugins.Context;
+
+import java.util.regex.Pattern;
+
+@Extension
+public class VariableNameChecker extends CFLintScannerAdapter {
+	final String severity = "INFO";
+
+	public void expression(final CFExpression expression, final Context context, final BugList bugs) {
+		if (expression instanceof CFVarDeclExpression) {
+			final CFVarDeclExpression cfVarDeclExpression = (CFVarDeclExpression)expression;
+			int lineNo = expression.getLine() + context.startLine() - 1;
+			checkNameForBugs(cfVarDeclExpression.getName(), context.getFilename(), lineNo, bugs);
+		}
+		else if (expression instanceof CFFullVarExpression) {
+			final CFFullVarExpression cfFullVarExpression = (CFFullVarExpression)expression;
+			for(final CFExpression subexpression : cfFullVarExpression.getExpressions()){
+				expression(subexpression,context,bugs);
+			}
+		}
+		else if (expression instanceof CFIdentifier) {
+			String varName = ((CFIdentifier) expression).getName();
+			int lineNo = ((CFIdentifier) expression).getLine() + context.startLine() - 1;
+
+			checkNameForBugs(varName, context.getFilename(), lineNo, bugs);
+		}
+	}
+
+	public void checkNameForBugs(String variable, String filename, int line, BugList bugs) {
+		int minVarLength = ValidName.MIN_VAR_LENGTH;
+		int maxVarLength = ValidName.MAX_VAR_LENGTH;
+		int maxVarWords = ValidName.MAX_VAR_WORDS;
+
+		if (getParameter("MinLength") != null) {
+			try {
+				minVarLength = Integer.parseInt(getParameter("MinLength"));
+			} catch(Exception e) {}
+		}
+
+		if (getParameter("MaxLength") != null) {
+			try {
+				maxVarLength = Integer.parseInt(getParameter("MaxLength"));
+			} catch(Exception e) {}
+		}
+
+		if (getParameter("MaxWords") != null) {
+			try {
+				maxVarWords = Integer.parseInt(getParameter("MaxWords"));
+			} catch(Exception e) {}
+		}
+
+		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.")
+				.build());
+		}
+		if (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.")
+				.build());
+		}
+		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.")
+				.build());
+		}
+		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.")
+				.build());
+		}
+		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?")
+				.build());
+		}
+		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.")
+				.build());
+		}
+		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.")
+				.build());
+		}
+	}
+}