From 9538eca5f3e512586f84781e420a2881901bbd5a Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Mon, 5 Oct 2015 19:01:12 +1100 Subject: [PATCH 01/22] work in progress on checking variable names --- .../plugins/core/VariableNameChecker.java | 205 ++++++++++++++++++ .../com/cflint/TestCFBugs_VariableNames.java | 124 +++++++++++ 2 files changed, 329 insertions(+) create mode 100644 src/main/java/com/cflint/plugins/core/VariableNameChecker.java create mode 100644 src/test/java/com/cflint/TestCFBugs_VariableNames.java diff --git a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java new file mode 100644 index 000000000..457d88110 --- /dev/null +++ b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java @@ -0,0 +1,205 @@ +package com.cflint.plugins.core; + +import ro.fortsoft.pf4j.Extension; +import net.htmlparser.jericho.Element; +import net.htmlparser.jericho.Attributes; +import cfml.parsing.cfscript.CFExpression; +import cfml.parsing.cfscript.script.CFScriptStatement; + +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 = "WARNING"; + + final int MIN_VAR_LENGTH = 3; + final int MAX_VAR_LENGTH = 20; + final int WORD_LENGTH = 10; + final int MAX_VAR_WORDS = 4; + + public void expression(final CFExpression expression, final Context context, final BugList bugs) { + // TODO + } + + public void expression(final CFScriptStatement expression, final Context context, final BugList bugs) { + // TODO + } + + public void element(final Element element, final Context context, final BugList bugs) { + String tag = element.getName(); + if (tag.equals("cfset")) { + // TODO is there a better way to do this? + String content = element.getStartTag().getTagContent().toString(); + String parts[] = content.split("="); + String allVariables = parts[0]; + String filename = context.getFilename(); + int line = element.getSource().getRow(element.getBegin()); + + allVariables = allVariables.replace("var ",""); + allVariables = allVariables.replace("["," "); + allVariables = allVariables.replace("]"," "); + allVariables = allVariables.replace("."," "); + + String[] subVariables = allVariables.split(" "); + + System.out.println(allVariables); + + for (String variable : subVariables) { + if (Integer.parseInt(variable) == 0) { + checkNameForBugs(variable, filename, line, bugs); + } + } + } + } + + public void checkNameForBugs(String variable, String filename, int line, BugList bugs) { + System.out.println("Var:" + variable); + + if (isValid(variable)) { + System.out.println("Not valid"); + 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 (isUpperCase(variable)) { + System.out.println("all caps"); + 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 (tooShort(variable)) { + System.out.println("tooshort"); + bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_TOO_SHORT") + .setSeverity(severity).setFilename(filename) + .setMessage("Variable " + variable + " should be longer than " + MIN_VAR_LENGTH + " characters.") + .build()); + } + if (tooLong(variable)) { + System.out.println("toolong"); + bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_TOO_LONG") + .setSeverity(severity).setFilename(filename) + .setMessage("Variable " + variable + " should be short than " + MAX_VAR_LENGTH + " characters.") + .build()); + } + if (!isUpperCase(variable) && tooWordy(variable)) { + System.out.println("toowordy"); + bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_TOO_WORDY") + .setSeverity(severity).setFilename(filename) + .setMessage("Variable " + variable + " is too wordy.") + .build()); + } + if (isTemporary(variable)) { + System.out.println("istemporary"); + bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_IS_TEMPORARY") + .setSeverity(severity).setFilename(filename) + .setMessage("Variable sounds tempory " + variable + " and could be named better.") + .build()); + } + if (hasPrefixOrPostfix(variable)) { + System.out.println("hasprefix"); + bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_HAS_PREFIX") + .setSeverity(severity).setFilename(filename) + .setMessage("Variable has prefix " + variable + " and could be named better.") + .build()); + } + } + + protected boolean isValid(String variable) { + return validChars(variable) && (isCamelCase(variable) || usesUnderscores(variable)) && !endsInNumber(variable); + } + + protected boolean validChars(String variable) { + Pattern valid = Pattern.compile("[A-Za-z0-9_]+"); + return valid.matcher(variable).matches(); + } + + protected boolean isUpperCase(String variable) { + return variable.toUpperCase().equals(variable); + } + + protected boolean isCamelCase(String variable) { + Pattern valid = Pattern.compile("[a-z0-9]+([A-Z][a-z0-9]+)*"); + return valid.matcher(variable).matches(); + } + + protected boolean usesUnderscores(String variable) { + Pattern valid = Pattern.compile("[a-z0-9]+[a-z0-9_]+"); + return valid.matcher(variable).matches(); + } + + protected boolean endsInNumber(String variable) { + String sentence = variable.replaceAll("_", " "); + sentence = sentence.replaceAll("(\\p{Ll})(\\p{Lu})","$1 $2"); + String[] words = sentence.split(" "); + String lastWord = words[words.length-1]; + + System.out.println(words); + System.out.println(lastWord); + + if (Integer.parseInt(lastWord) > 0) { + return true; + } + + return false; + } + + protected boolean tooShort(String variable) { + return variable.length() < MIN_VAR_LENGTH; + } + + protected boolean tooLong(String variable) { + return variable.length() > MAX_VAR_LENGTH; + } + + protected boolean tooWordy(String variable) { + String[] words = variable.split("[A-Z_]"); + return words.length > MAX_VAR_WORDS; + } + + protected boolean isTemporary(String variable) { + String[] wordsToAvoid = {"temp", "tmp", "var", "func", "obj", "object", "str", "struct", "arr", "array"}; + String sentence = variable.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; + } + + protected boolean hasPrefixOrPostfix(String variable) { + String[] namesToAvoid = {"st", "str", "o", "obj", "b", "q", "arr", "a", "this", "my"}; + String sentence = variable.replaceAll("_", " "); + sentence = sentence.replaceAll("(\\p{Ll})(\\p{Lu})","$1 $2"); + String[] words = sentence.split(" "); + String firstWord = words[0]; + String lastWord = words[words.length-1]; + + for (String badName : namesToAvoid) { + if (firstWord.toLowerCase().equals(badName)) + { + return true; + } + if (lastWord.toLowerCase().equals(badName)) + { + return true; + } + } + + return false; + } +} \ No newline at end of file diff --git a/src/test/java/com/cflint/TestCFBugs_VariableNames.java b/src/test/java/com/cflint/TestCFBugs_VariableNames.java new file mode 100644 index 000000000..a3be95ff0 --- /dev/null +++ b/src/test/java/com/cflint/TestCFBugs_VariableNames.java @@ -0,0 +1,124 @@ +package com.cflint; + +import static org.junit.Assert.assertEquals; + +import java.io.IOException; +import java.util.Collection; +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.VariableNameChecker; + +public class TestCFBugs_VariableNames { + + private CFLint cfBugs; + + @Before + public void setUp() { + final ConfigRuntime conf = new ConfigRuntime(); + final PluginInfoRule pluginRule = new PluginInfoRule(); + pluginRule.setName("VariableNameChecker"); + conf.getRules().add(pluginRule); + PluginMessage pluginMessage = new PluginMessage("VAR_INVALID_NAME"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginMessage = new PluginMessage("VAR_ALLCAPS_NAME"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginMessage = new PluginMessage("VAR_TOO_SHORT"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginMessage = new PluginMessage("VAR_TOO_LONG"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginMessage = new PluginMessage("VAR_TOO_WORDY"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginMessage = new PluginMessage("VAR_IS_TEMPORARY"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginMessage = new PluginMessage("VAR_HAS_PREFIX"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + + cfBugs = new CFLint(conf, new VariableNameChecker()); + } + + @Test + public void testValidNames() throws ParseException, IOException { + final String cfcSrc = "\r\n" + + "\r\n" + + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + + "\r\n" + + ""; + cfBugs.process(cfcSrc, "test"); + Collection> result = cfBugs.getBugs().getBugList().values(); + assertEquals(0, result.size()); + } + + @Test + public void testUpercaseName() throws ParseException, IOException { + final String cfcSrc = "\r\n" + + "\r\n" + + " \r\n" + + " \r\n" + + "\r\n" + + ""; + cfBugs.process(cfcSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(2, result.size()); + assertEquals("VAR_ALLCAPS_NAME", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_ALLCAPS_NAME", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + } + + @Test + public void invalidCharsInName() throws ParseException, IOException { + final String cfcSrc = "\r\n" + + "\r\n" + + " \r\n" + + " \r\n" + + "\r\n" + + ""; + cfBugs.process(cfcSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(2, result.size()); + assertEquals("VAR_INVALID_NAME", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_INVALID_NAME", result.get(0).getMessageCode()); + assertEquals(4, result.get(0).getLine()); + } + + + @Test + public void nameEndsINNumber() throws ParseException, IOException { + final String cfcSrc = "\r\n" + + "\r\n" + + " \r\n" + + " \r\n" + + "\r\n" + + ""; + cfBugs.process(cfcSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(2, result.size()); + assertEquals("VAR_INVALID_NAME", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_INVALID_NAME", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + } + +} From d7ed91aeb5a82f6b952fb1302f5baddd08ded3a6 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Fri, 16 Oct 2015 15:26:29 +1100 Subject: [PATCH 02/22] Add more tests --- .../com/cflint/TestCFBugs_VariableNames.java | 192 +++++++++++++++--- 1 file changed, 162 insertions(+), 30 deletions(-) diff --git a/src/test/java/com/cflint/TestCFBugs_VariableNames.java b/src/test/java/com/cflint/TestCFBugs_VariableNames.java index a3be95ff0..572d9fd50 100644 --- a/src/test/java/com/cflint/TestCFBugs_VariableNames.java +++ b/src/test/java/com/cflint/TestCFBugs_VariableNames.java @@ -45,7 +45,7 @@ public void setUp() { pluginMessage = new PluginMessage("VAR_IS_TEMPORARY"); pluginMessage.setSeverity("INFO"); pluginRule.getMessages().add(pluginMessage); - pluginMessage = new PluginMessage("VAR_HAS_PREFIX"); + pluginMessage = new PluginMessage("VAR_HAS_PREFIX_OR_POSTFIX"); pluginMessage.setSeverity("INFO"); pluginRule.getMessages().add(pluginMessage); @@ -53,72 +53,204 @@ public void setUp() { } @Test - public void testValidNames() throws ParseException, IOException { - final String cfcSrc = "\r\n" + public void testValidNamesTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + "\r\n" - + " \r\n" - + " \r\n" - + " \r\n" - + " \r\n" - + " \r\n" - + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + "\r\n" + ""; - cfBugs.process(cfcSrc, "test"); + cfBugs.process(tagSrc, "test"); Collection> result = cfBugs.getBugs().getBugList().values(); assertEquals(0, result.size()); } @Test - public void testUpercaseName() throws ParseException, IOException { - final String cfcSrc = "\r\n" + public void testUpercaseNameTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + "\r\n" - + " \r\n" - + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + "\r\n" + ""; - cfBugs.process(cfcSrc, "test"); + cfBugs.process(tagSrc, "test"); final List result = cfBugs.getBugs().getBugList().values().iterator().next(); - assertEquals(2, result.size()); + assertEquals(4, result.size()); assertEquals("VAR_ALLCAPS_NAME", result.get(0).getMessageCode()); assertEquals(3, result.get(0).getLine()); assertEquals("VAR_ALLCAPS_NAME", result.get(1).getMessageCode()); assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_ALLCAPS_NAME", result.get(2).getMessageCode()); + assertEquals(6, result.get(2).getLine()); + assertEquals("VAR_ALLCAPS_NAME", result.get(3).getMessageCode()); + assertEquals(7, result.get(3).getLine()); } @Test - public void invalidCharsInName() throws ParseException, IOException { - final String cfcSrc = "\r\n" + public void invalidCharsInNameTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + "\r\n" - + " \r\n" - + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + "\r\n" + ""; - cfBugs.process(cfcSrc, "test"); + cfBugs.process(tagSrc, "test"); final List result = cfBugs.getBugs().getBugList().values().iterator().next(); - assertEquals(2, result.size()); + assertEquals(3, result.size()); assertEquals("VAR_INVALID_NAME", result.get(0).getMessageCode()); assertEquals(3, result.get(0).getLine()); - assertEquals("VAR_INVALID_NAME", result.get(0).getMessageCode()); - assertEquals(4, result.get(0).getLine()); + assertEquals("VAR_INVALID_NAME", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_INVALID_NAME", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); } @Test - public void nameEndsINNumber() throws ParseException, IOException { - final String cfcSrc = "\r\n" + public void nameEndsInNumberTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + "\r\n" - + " \r\n" - + " \r\n" + + " \r\n" + + " \r\n" + + " \r\n" + "\r\n" + ""; - cfBugs.process(cfcSrc, "test"); + cfBugs.process(tagSrc, "test"); final List result = cfBugs.getBugs().getBugList().values().iterator().next(); - assertEquals(2, result.size()); + assertEquals(3, result.size()); assertEquals("VAR_INVALID_NAME", result.get(0).getMessageCode()); assertEquals(3, result.get(0).getLine()); assertEquals("VAR_INVALID_NAME", result.get(1).getMessageCode()); assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_INVALID_NAME", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); + } + + @Test + public void nameTooShortTag() throws ParseException, IOException { + final String tagSrc = "\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("VAR_TOO_SHORT", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_TOO_SHORT", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_TOO_SHORT", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); + } + + @Test + public void nameTooLongTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + " \r\n" + + " \r\n" + + "\r\n" + + ""; + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(2, result.size()); + assertEquals("VAR_TOO_LONG", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_TOO_LONG", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + } + + @Test + public void nameTooWordyTag() throws ParseException, IOException { + final String tagSrc = "\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(2, result.size()); + assertEquals("VAR_TOO_WORDY", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_TOO_WORDY", result.get(1).getMessageCode()); + assertEquals(5, result.get(1).getLine()); + } + + @Test + public void nameIsTemporyTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\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(7, result.size()); + assertEquals("VAR_IS_TEMPORARY", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(3).getMessageCode()); + assertEquals(6, result.get(3).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(4).getMessageCode()); + assertEquals(7, result.get(4).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(5).getMessageCode()); + assertEquals(8, result.get(5).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(6).getMessageCode()); + assertEquals(9, result.get(6).getLine()); + } + + @Test + public void nameHasPrefixOrPostfixTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\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(7, result.size()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(3).getMessageCode()); + assertEquals(6, result.get(3).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(4).getMessageCode()); + assertEquals(7, result.get(4).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(5).getMessageCode()); + assertEquals(8, result.get(5).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(6).getMessageCode()); + assertEquals(9, result.get(6).getLine()); } } From 7089da71151ccfaa397fcd3a5ecc95f2c7f2b540 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Fri, 16 Oct 2015 15:26:54 +1100 Subject: [PATCH 03/22] Add variable name checker to flint definitions --- src/main/resources/cflint.definition.xml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/main/resources/cflint.definition.xml b/src/main/resources/cflint.definition.xml index f7bc54398..8c8b851e1 100644 --- a/src/main/resources/cflint.definition.xml +++ b/src/main/resources/cflint.definition.xml @@ -203,4 +203,27 @@ INFO + + + INFO + + + INFO + + + INFO + + + INFO + + + INFO + + + INFO + + + INFO + + From 8b2e6346c23a9c1b4727b3df7416584c650a8c40 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Fri, 16 Oct 2015 15:27:38 +1100 Subject: [PATCH 04/22] use expression method rather than element --- .../plugins/core/VariableNameChecker.java | 117 ++++++++---------- 1 file changed, 55 insertions(+), 62 deletions(-) diff --git a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java index 457d88110..09e0f0122 100644 --- a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java +++ b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java @@ -4,7 +4,9 @@ import net.htmlparser.jericho.Element; import net.htmlparser.jericho.Attributes; import cfml.parsing.cfscript.CFExpression; -import cfml.parsing.cfscript.script.CFScriptStatement; +import cfml.parsing.cfscript.CFFullVarExpression; +import cfml.parsing.cfscript.CFIdentifier; +import cfml.parsing.cfscript.CFVarDeclExpression; import com.cflint.BugInfo; import com.cflint.BugList; @@ -23,96 +25,87 @@ public class VariableNameChecker extends CFLintScannerAdapter { final int MAX_VAR_WORDS = 4; public void expression(final CFExpression expression, final Context context, final BugList bugs) { - // TODO - } + if (expression instanceof CFVarDeclExpression) { + String content = ((CFVarDeclExpression) expression).Decompile(0); + String parts[] = content.split(" "); + String varName = parts[1]; // var name = expression + int lineNo = ((CFVarDeclExpression) expression).getLine() + context.startLine() - 1; - public void expression(final CFScriptStatement expression, final Context context, final BugList bugs) { - // TODO - } - - public void element(final Element element, final Context context, final BugList bugs) { - String tag = element.getName(); - if (tag.equals("cfset")) { - // TODO is there a better way to do this? - String content = element.getStartTag().getTagContent().toString(); - String parts[] = content.split("="); - String allVariables = parts[0]; - String filename = context.getFilename(); - int line = element.getSource().getRow(element.getBegin()); - - allVariables = allVariables.replace("var ",""); - allVariables = allVariables.replace("["," "); - allVariables = allVariables.replace("]"," "); - allVariables = allVariables.replace("."," "); - - String[] subVariables = allVariables.split(" "); - - System.out.println(allVariables); - - for (String variable : subVariables) { - if (Integer.parseInt(variable) == 0) { - checkNameForBugs(variable, filename, line, bugs); + checkNameForBugs(varName, context.getFilename(), lineNo, bugs); + } + else if (expression instanceof CFFullVarExpression) { + String content = ((CFFullVarExpression) expression).Decompile(0); + content = content.replace(".", " "); + content = content.replace("[", " "); + content = content.replace("]", " "); + String parts[] = content.split(" "); + int lineNo = ((CFFullVarExpression) expression).getLine() + context.startLine() - 1; + + for (int i = 0; i < parts.length; i++) { + if (!parts[i].matches("\\d+")) { + checkNameForBugs(parts[i], context.getFilename(), lineNo, 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) { System.out.println("Var:" + variable); - if (isValid(variable)) { - System.out.println("Not valid"); + if (isInvalid(variable)) { + System.out.println("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 (isUpperCase(variable)) { - System.out.println("all caps"); 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 (tooShort(variable)) { - System.out.println("tooshort"); bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_TOO_SHORT") .setSeverity(severity).setFilename(filename) .setMessage("Variable " + variable + " should be longer than " + MIN_VAR_LENGTH + " characters.") .build()); } if (tooLong(variable)) { - System.out.println("toolong"); bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_TOO_LONG") .setSeverity(severity).setFilename(filename) - .setMessage("Variable " + variable + " should be short than " + MAX_VAR_LENGTH + " characters.") + .setMessage("Variable " + variable + " should be shorter than " + MAX_VAR_LENGTH + " characters.") .build()); } if (!isUpperCase(variable) && tooWordy(variable)) { - System.out.println("toowordy"); bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_TOO_WORDY") .setSeverity(severity).setFilename(filename) - .setMessage("Variable " + variable + " is too wordy.") + .setMessage("Variable " + variable + " is too wordy, can you think of a more concise name?") .build()); } if (isTemporary(variable)) { - System.out.println("istemporary"); bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_IS_TEMPORARY") .setSeverity(severity).setFilename(filename) - .setMessage("Variable sounds tempory " + variable + " and could be named better.") + .setMessage("Temporary variable " + variable + " could be named better.") .build()); } if (hasPrefixOrPostfix(variable)) { - System.out.println("hasprefix"); - bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_HAS_PREFIX") + System.out.println("hasPrefixOrPostfix : " + variable); + bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_HAS_PREFIX_OR_POSTFIX") .setSeverity(severity).setFilename(filename) - .setMessage("Variable has prefix " + variable + " and could be named better.") + .setMessage("Variable has prefix or postfix " + variable + " and could be named better.") .build()); } } - protected boolean isValid(String variable) { - return validChars(variable) && (isCamelCase(variable) || usesUnderscores(variable)) && !endsInNumber(variable); + protected boolean isInvalid(String variable) { + return !validChars(variable) || !(isSameCase(variable) || isCamelCase(variable) || usesUnderscores(variable)) || endsInNumber(variable); } protected boolean validChars(String variable) { @@ -124,6 +117,10 @@ protected boolean isUpperCase(String variable) { return variable.toUpperCase().equals(variable); } + protected boolean isSameCase(String variable) { + return variable.equals(variable.toLowerCase()) || variable.equals(variable.toUpperCase()); + } + protected boolean isCamelCase(String variable) { Pattern valid = Pattern.compile("[a-z0-9]+([A-Z][a-z0-9]+)*"); return valid.matcher(variable).matches(); @@ -135,15 +132,9 @@ protected boolean usesUnderscores(String variable) { } protected boolean endsInNumber(String variable) { - String sentence = variable.replaceAll("_", " "); - sentence = sentence.replaceAll("(\\p{Ll})(\\p{Lu})","$1 $2"); - String[] words = sentence.split(" "); - String lastWord = words[words.length-1]; - - System.out.println(words); - System.out.println(lastWord); + char lastLetter = variable.charAt(variable.length() - 1); - if (Integer.parseInt(lastWord) > 0) { + if (Character.isDigit(lastLetter)) { return true; } @@ -164,7 +155,7 @@ protected boolean tooWordy(String variable) { } protected boolean isTemporary(String variable) { - String[] wordsToAvoid = {"temp", "tmp", "var", "func", "obj", "object", "str", "struct", "arr", "array"}; + String[] wordsToAvoid = {"temp", "tmp", "var", "func", "obj", "object", "bool", "struct", "string", "array"}; String sentence = variable.replaceAll("_", " "); sentence = sentence.replaceAll("(\\p{Ll})(\\p{Lu})","$1 $2"); String[] words = sentence.split(" "); @@ -182,21 +173,23 @@ protected boolean isTemporary(String variable) { } protected boolean hasPrefixOrPostfix(String variable) { - String[] namesToAvoid = {"st", "str", "o", "obj", "b", "q", "arr", "a", "this", "my"}; + String[] namesToAvoid = {"s", "st", "str", "o", "obj", "b", "q", "a", "arr", "this", "my"}; String sentence = variable.replaceAll("_", " "); sentence = sentence.replaceAll("(\\p{Ll})(\\p{Lu})","$1 $2"); String[] words = sentence.split(" "); String firstWord = words[0]; String lastWord = words[words.length-1]; - for (String badName : namesToAvoid) { - if (firstWord.toLowerCase().equals(badName)) - { - return true; - } - if (lastWord.toLowerCase().equals(badName)) - { - return true; + if (words.length > 1) { + for (String badName : namesToAvoid) { + if (firstWord.toLowerCase().equals(badName)) + { + return true; + } + if (lastWord.toLowerCase().equals(badName)) + { + return true; + } } } From be6ddbe9c09da2d26645d09a231f54589863853b Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Fri, 16 Oct 2015 18:05:38 +1100 Subject: [PATCH 05/22] fix line number calculation where it pure script with no tags. --- src/main/java/com/cflint/plugins/Context.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/cflint/plugins/Context.java b/src/main/java/com/cflint/plugins/Context.java index 5381c1a82..358f6783e 100644 --- a/src/main/java/com/cflint/plugins/Context.java +++ b/src/main/java/com/cflint/plugins/Context.java @@ -144,7 +144,10 @@ public Context subContext(final Element elem){ } public int startLine() { - return element.getSource().getRow(element.getBegin()); + if(element != null && element.getSource() !=null) + return element.getSource().getRow(element.getBegin()); + else + return 1; // not zero } protected String componentFromFile(String filename) { From 05ea8ad396058708065d514254a3e8bf32baa51d Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Fri, 16 Oct 2015 18:06:00 +1100 Subject: [PATCH 06/22] remove debug --- .../java/com/cflint/plugins/core/VariableNameChecker.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java index 09e0f0122..6a4c4239f 100644 --- a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java +++ b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java @@ -56,10 +56,7 @@ else if (expression instanceof CFIdentifier) { } public void checkNameForBugs(String variable, String filename, int line, BugList bugs) { - System.out.println("Var:" + variable); - if (isInvalid(variable)) { - System.out.println("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.") @@ -96,7 +93,6 @@ public void checkNameForBugs(String variable, String filename, int line, BugList .build()); } if (hasPrefixOrPostfix(variable)) { - System.out.println("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.") From 030b9a46bdcf953745b19155de3babd0e9b9396f Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Fri, 16 Oct 2015 18:07:51 +1100 Subject: [PATCH 07/22] Add tests for scripts blocks --- .../com/cflint/TestCFBugs_VariableNames.java | 202 +++++++++++++++++- 1 file changed, 201 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/cflint/TestCFBugs_VariableNames.java b/src/test/java/com/cflint/TestCFBugs_VariableNames.java index 572d9fd50..13352c468 100644 --- a/src/test/java/com/cflint/TestCFBugs_VariableNames.java +++ b/src/test/java/com/cflint/TestCFBugs_VariableNames.java @@ -113,7 +113,6 @@ public void invalidCharsInNameTag() throws ParseException, IOException { assertEquals(5, result.get(2).getLine()); } - @Test public void nameEndsInNumberTag() throws ParseException, IOException { final String tagSrc = "\r\n" @@ -253,4 +252,205 @@ public void nameHasPrefixOrPostfixTag() throws ParseException, IOException { assertEquals(9, result.get(6).getLine()); } + + @Test + public void testValidNamesScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function test() {\r\n" + + " var firstName = \"Fred\";\r\n" + + " first_name = \"Fred\";\r\n" + + " firstname = \"Fred\";\r\n" + + " name.first = \"Fred\";\r\n" + + " name.last = \"Smith\";\r\n" + + " names[1] = \"Smith\";\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + Collection> result = cfBugs.getBugs().getBugList().values(); + assertEquals(0, result.size()); + } + + @Test + public void testUpercaseNameScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function test() {\r\n" + + " var FIRSTNAME = \"Fred\";\r\n" + + " LAST_NAME = \"Smith\";\r\n" + + " names = {};\r\n" + + " name.FIRST = \"Fred\";\r\n" + + " NAMES[1] = \"Fred\";\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(4, result.size()); + assertEquals("VAR_ALLCAPS_NAME", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_ALLCAPS_NAME", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_ALLCAPS_NAME", result.get(2).getMessageCode()); + assertEquals(6, result.get(2).getLine()); + assertEquals("VAR_ALLCAPS_NAME", result.get(3).getMessageCode()); + assertEquals(7, result.get(3).getLine()); + } + + @Test + public void invalidCharsInNameScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function test() {\r\n" + + " var $name = \"Fred\";\r\n" + + " last$name = \"Smith\";\r\n" + + " last.$name = \"Smith\";\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(3, result.size()); + assertEquals("VAR_INVALID_NAME", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_INVALID_NAME", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_INVALID_NAME", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); + } + + @Test + public void nameEndsInNumberScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function test() {\r\n" + + " name_1 = \"Fred\";\r\n" + + " name2 = \"Smith\";\r\n" + + " last.name1 = \"Fred\";\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(3, result.size()); + assertEquals("VAR_INVALID_NAME", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_INVALID_NAME", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_INVALID_NAME", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); + } + + @Test + public void nameTooShortScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function test() {\r\n" + + " a = \"Fred\";\r\n" + + " b = \"Smith\";\r\n" + + " last.a = \"Fred\";\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(3, result.size()); + assertEquals("VAR_TOO_SHORT", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_TOO_SHORT", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_TOO_SHORT", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); + } + + @Test + public void nameTooLongScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function test() {\r\n" + + " isaveryveryverylongvariablename = \"Fred\";\r\n" + + " isa.veryveryverylongvariablename = \"Fred\";\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(2, result.size()); + assertEquals("VAR_TOO_LONG", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_TOO_LONG", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + } + + @Test + public void nameTooWordyScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function test() {\r\n" + + " nameIsFarTooWordy = \"Fred\";\r\n" + + " nameIsOK = \"Fred\";\r\n" + + " name.isAlsoFarTooWordy = \"Fred\";\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(2, result.size()); + assertEquals("VAR_TOO_WORDY", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_TOO_WORDY", result.get(1).getMessageCode()); + assertEquals(5, result.get(1).getLine()); + } + + @Test + public void nameIsTemporyScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function test() {\r\n" + + " temp = \"Fred\";\r\n" + + " name.temp = \"Fred\";\r\n" + + " obj = \"Fred\";\r\n" + + " struct = \"Fred\";\r\n" + + " tempName = \"Fred\";\r\n" + + " nameObj = \"Fred\";\r\n" + + " nameString = \"Fred\";\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(7, result.size()); + assertEquals("VAR_IS_TEMPORARY", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(3).getMessageCode()); + assertEquals(6, result.get(3).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(4).getMessageCode()); + assertEquals(7, result.get(4).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(5).getMessageCode()); + assertEquals(8, result.get(5).getLine()); + assertEquals("VAR_IS_TEMPORARY", result.get(6).getMessageCode()); + assertEquals(9, result.get(6).getLine()); + } + + @Test + public void nameHasPrefixOrPostfixScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function test() {\r\n" + + " sName = \"Fred\";\r\n" + + " nameSt = {first:\"Fred\"};\r\n" + + " oName = {first:\"Fred\"};\r\n" + + " bOff = true;\r\n" + + " arrNames = [\"Fred\"];\r\n" + + " thisName = \"Fred\";\r\n" + + " myName = \"Fred\";\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(7, result.size()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(0).getMessageCode()); + assertEquals(3, result.get(0).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(1).getMessageCode()); + assertEquals(4, result.get(1).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(2).getMessageCode()); + assertEquals(5, result.get(2).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(3).getMessageCode()); + assertEquals(6, result.get(3).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(4).getMessageCode()); + assertEquals(7, result.get(4).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(5).getMessageCode()); + assertEquals(8, result.get(5).getLine()); + assertEquals("VAR_HAS_PREFIX_OR_POSTFIX", result.get(6).getMessageCode()); + assertEquals(9, result.get(6).getLine()); + } + } From e86dff3a4963f19e4071425ec4c1e535c4ee5685 Mon Sep 17 00:00:00 2001 From: ryaneberly Date: Fri, 16 Oct 2015 07:12:31 -0400 Subject: [PATCH 08/22] used more of the built into component, less decompile and string manipulation. added sample parmeter test --- .../plugins/core/VariableNameChecker.java | 33 +++++++++---------- .../com/cflint/TestCFBugs_VariableNames.java | 10 ++++-- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java index 6a4c4239f..7e1872f03 100644 --- a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java +++ b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java @@ -3,6 +3,7 @@ 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; @@ -26,25 +27,14 @@ public class VariableNameChecker extends CFLintScannerAdapter { public void expression(final CFExpression expression, final Context context, final BugList bugs) { if (expression instanceof CFVarDeclExpression) { - String content = ((CFVarDeclExpression) expression).Decompile(0); - String parts[] = content.split(" "); - String varName = parts[1]; // var name = expression - int lineNo = ((CFVarDeclExpression) expression).getLine() + context.startLine() - 1; - - checkNameForBugs(varName, context.getFilename(), lineNo, bugs); + final CFVarDeclExpression cfVarDeclExpression = (CFVarDeclExpression)expression; + int lineNo = expression.getLine() + context.startLine() - 1; + checkNameForBugs(cfVarDeclExpression.getName(), context.getFilename(), lineNo, bugs); } else if (expression instanceof CFFullVarExpression) { - String content = ((CFFullVarExpression) expression).Decompile(0); - content = content.replace(".", " "); - content = content.replace("[", " "); - content = content.replace("]", " "); - String parts[] = content.split(" "); - int lineNo = ((CFFullVarExpression) expression).getLine() + context.startLine() - 1; - - for (int i = 0; i < parts.length; i++) { - if (!parts[i].matches("\\d+")) { - checkNameForBugs(parts[i], context.getFilename(), lineNo, bugs); - } + final CFFullVarExpression cfFullVarExpression = (CFFullVarExpression)expression; + for(final CFExpression subexpression : cfFullVarExpression.getExpressions()){ + expression(subexpression,context,bugs); } } else if (expression instanceof CFIdentifier) { @@ -138,7 +128,14 @@ protected boolean endsInNumber(String variable) { } protected boolean tooShort(String variable) { - return variable.length() < MIN_VAR_LENGTH; + + int minVarLength = MIN_VAR_LENGTH; + if(getParameter("MinLength") !=null){ + try { + minVarLength= Integer.parseInt(getParameter("MinLength")); + }catch(Exception e){} + } + return variable.length() < minVarLength; } protected boolean tooLong(String variable) { diff --git a/src/test/java/com/cflint/TestCFBugs_VariableNames.java b/src/test/java/com/cflint/TestCFBugs_VariableNames.java index 13352c468..67d2780cc 100644 --- a/src/test/java/com/cflint/TestCFBugs_VariableNames.java +++ b/src/test/java/com/cflint/TestCFBugs_VariableNames.java @@ -36,6 +36,8 @@ public void setUp() { pluginMessage = new PluginMessage("VAR_TOO_SHORT"); pluginMessage.setSeverity("INFO"); pluginRule.getMessages().add(pluginMessage); + pluginRule.addParameter("MinLength", "3"); + pluginMessage = new PluginMessage("VAR_TOO_LONG"); pluginMessage.setSeverity("INFO"); pluginRule.getMessages().add(pluginMessage); @@ -49,7 +51,9 @@ public void setUp() { pluginMessage.setSeverity("INFO"); pluginRule.getMessages().add(pluginMessage); - cfBugs = new CFLint(conf, new VariableNameChecker()); + final VariableNameChecker checker = new VariableNameChecker(); + checker.setParameter("MinLength", "3"); + cfBugs = new CFLint(conf, checker); } @Test @@ -202,7 +206,7 @@ public void nameIsTemporyTag() throws ParseException, IOException { + "\r\n" + ""; cfBugs.process(tagSrc, "test"); - final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + final List result = cfBugs.getBugs().getBugList().get("VAR_IS_TEMPORARY"); assertEquals(7, result.size()); assertEquals("VAR_IS_TEMPORARY", result.get(0).getMessageCode()); assertEquals(3, result.get(0).getLine()); @@ -403,7 +407,7 @@ public void nameIsTemporyScript() throws ParseException, IOException { + "}\r\n" + "}"; cfBugs.process(scriptSrc, "test"); - final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + final List result = cfBugs.getBugs().getBugList().get("VAR_IS_TEMPORARY"); assertEquals(7, result.size()); assertEquals("VAR_IS_TEMPORARY", result.get(0).getMessageCode()); assertEquals(3, result.get(0).getLine()); From fa23dd279dfc90fd342e539536205f0d2e1ca75a Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 11:39:56 +1100 Subject: [PATCH 09/22] Added parameters change default form warning to info --- .../plugins/core/VariableNameChecker.java | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java index 7e1872f03..b55f435e8 100644 --- a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java +++ b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java @@ -18,11 +18,10 @@ @Extension public class VariableNameChecker extends CFLintScannerAdapter { - final String severity = "WARNING"; + final String severity = "INFO"; final int MIN_VAR_LENGTH = 3; final int MAX_VAR_LENGTH = 20; - final int WORD_LENGTH = 10; final int MAX_VAR_WORDS = 4; public void expression(final CFExpression expression, final Context context, final BugList bugs) { @@ -46,6 +45,28 @@ else if (expression instanceof CFIdentifier) { } public void checkNameForBugs(String variable, String filename, int line, BugList bugs) { + int minVarLength = MIN_VAR_LENGTH; + int maxVarLength = MAX_VAR_LENGTH; + int maxVarWords = MAX_VAR_WORDS; + + if (getParameter("MinLength") != null) { + try { + minVarLength = Integer.parseInt(getParameter("MinLength")); + } catch(Exception e) {} + } + + if (getParameter("MaxLength") != null) { + try { + minVarLength = Integer.parseInt(getParameter("MaxLength")); + } catch(Exception e) {} + } + + if (getParameter("MaxWords") != null) { + try { + minVarLength = Integer.parseInt(getParameter("MaxWords")); + } catch(Exception e) {} + } + if (isInvalid(variable)) { bugs.add(new BugInfo.BugInfoBuilder().setLine(line).setMessageCode("VAR_INVALID_NAME") .setSeverity(severity).setFilename(filename) @@ -128,12 +149,12 @@ protected boolean endsInNumber(String variable) { } protected boolean tooShort(String variable) { - int minVarLength = MIN_VAR_LENGTH; - if(getParameter("MinLength") !=null){ + + if (getParameter("MinLength") != null) { try { - minVarLength= Integer.parseInt(getParameter("MinLength")); - }catch(Exception e){} + minVarLength = Integer.parseInt(getParameter("MinLength")); + } catch(Exception e) {} } return variable.length() < minVarLength; } From 985c619032fc803857310cb4cf9fc19760277fd3 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 12:18:51 +1100 Subject: [PATCH 10/22] Moved code to checking if a name is valid to it's own class --- .../com/cflint/plugins/core/ValidName.java | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 src/main/java/com/cflint/plugins/core/ValidName.java diff --git a/src/main/java/com/cflint/plugins/core/ValidName.java b/src/main/java/com/cflint/plugins/core/ValidName.java new file mode 100644 index 000000000..312a47804 --- /dev/null +++ b/src/main/java/com/cflint/plugins/core/ValidName.java @@ -0,0 +1,115 @@ + +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; + + protected int minVarLength = MIN_VAR_LENGTH; + protected int maxVarLength = MAX_VAR_LENGTH; + protected int maxVarWords = MAX_VAR_WORDS; + + public ValidName() { + } + + public ValidName(int minVarLength, int maxVarLength, int maxVarWords) { + this.minVarLength = minVarLength; + this.maxVarLength = maxVarLength; + this.maxVarWords = maxVarWords; + } + + public boolean isInvalid(String variable) { + return !validChars(variable) || !(isSameCase(variable) || isCamelCase(variable) || usesUnderscores(variable)) || endsInNumber(variable); + } + + public boolean validChars(String variable) { + Pattern valid = Pattern.compile("[A-Za-z0-9_]+"); + return valid.matcher(variable).matches(); + } + + public boolean isUpperCase(String variable) { + return variable.toUpperCase().equals(variable); + } + + public boolean isSameCase(String variable) { + return variable.equals(variable.toLowerCase()) || variable.equals(variable.toUpperCase()); + } + + public boolean isCamelCase(String variable) { + Pattern valid = Pattern.compile("[a-z0-9]+([A-Z][a-z0-9]+)*"); + return valid.matcher(variable).matches(); + } + + public boolean usesUnderscores(String variable) { + Pattern valid = Pattern.compile("[a-z0-9]+[a-z0-9_]+"); + return valid.matcher(variable).matches(); + } + + public boolean endsInNumber(String variable) { + char lastLetter = variable.charAt(variable.length() - 1); + + if (Character.isDigit(lastLetter)) { + return true; + } + + return false; + } + + public boolean tooShort(String variable) { + return variable.length() < minVarLength; + } + + public boolean tooLong(String variable) { + return variable.length() > maxVarLength; + } + + public boolean tooWordy(String variable) { + String[] words = variable.split("[A-Z_]"); + return words.length > maxVarWords; + } + + public boolean isTemporary(String variable) { + String[] wordsToAvoid = {"temp", "tmp", "var", "func", "obj", "object", "bool", "struct", "string", "array"}; + String sentence = variable.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 variable) { + String[] namesToAvoid = {"s", "st", "str", "o", "obj", "b", "q", "a", "arr", "this", "my"}; + String sentence = variable.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; + } +} \ No newline at end of file From c16745bd7d9a91132ea4c65e0b0b2877cea2e593 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 12:19:16 +1100 Subject: [PATCH 11/22] Moved code to checking if a name is valid to it's own class --- .../plugins/core/VariableNameChecker.java | 129 ++---------------- 1 file changed, 14 insertions(+), 115 deletions(-) diff --git a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java index b55f435e8..700c7d0c6 100644 --- a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java +++ b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java @@ -19,10 +19,6 @@ @Extension public class VariableNameChecker extends CFLintScannerAdapter { final String severity = "INFO"; - - final int MIN_VAR_LENGTH = 3; - final int MAX_VAR_LENGTH = 20; - final int MAX_VAR_WORDS = 4; public void expression(final CFExpression expression, final Context context, final BugList bugs) { if (expression instanceof CFVarDeclExpression) { @@ -45,9 +41,9 @@ else if (expression instanceof CFIdentifier) { } public void checkNameForBugs(String variable, String filename, int line, BugList bugs) { - int minVarLength = MIN_VAR_LENGTH; - int maxVarLength = MAX_VAR_LENGTH; - int maxVarWords = MAX_VAR_WORDS; + int minVarLength = ValidName.MIN_VAR_LENGTH; + int maxVarLength = ValidName.MAX_VAR_LENGTH; + int maxVarWords = ValidName.MAX_VAR_WORDS; if (getParameter("MinLength") != null) { try { @@ -67,146 +63,49 @@ public void checkNameForBugs(String variable, String filename, int line, BugList } catch(Exception e) {} } - if (isInvalid(variable)) { + 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 (isUpperCase(variable)) { + 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 (tooShort(variable)) { + 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 " + MIN_VAR_LENGTH + " characters.") + .setMessage("Variable " + variable + " should be longer than " + minVarLength + " characters.") .build()); } - if (tooLong(variable)) { + 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 " + MAX_VAR_LENGTH + " characters.") + .setMessage("Variable " + variable + " should be shorter than " + maxVarLength + " characters.") .build()); } - if (!isUpperCase(variable) && tooWordy(variable)) { + 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 (isTemporary(variable)) { + 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 (hasPrefixOrPostfix(variable)) { + 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()); } } - - protected boolean isInvalid(String variable) { - return !validChars(variable) || !(isSameCase(variable) || isCamelCase(variable) || usesUnderscores(variable)) || endsInNumber(variable); - } - - protected boolean validChars(String variable) { - Pattern valid = Pattern.compile("[A-Za-z0-9_]+"); - return valid.matcher(variable).matches(); - } - - protected boolean isUpperCase(String variable) { - return variable.toUpperCase().equals(variable); - } - - protected boolean isSameCase(String variable) { - return variable.equals(variable.toLowerCase()) || variable.equals(variable.toUpperCase()); - } - - protected boolean isCamelCase(String variable) { - Pattern valid = Pattern.compile("[a-z0-9]+([A-Z][a-z0-9]+)*"); - return valid.matcher(variable).matches(); - } - - protected boolean usesUnderscores(String variable) { - Pattern valid = Pattern.compile("[a-z0-9]+[a-z0-9_]+"); - return valid.matcher(variable).matches(); - } - - protected boolean endsInNumber(String variable) { - char lastLetter = variable.charAt(variable.length() - 1); - - if (Character.isDigit(lastLetter)) { - return true; - } - - return false; - } - - protected boolean tooShort(String variable) { - int minVarLength = MIN_VAR_LENGTH; - - if (getParameter("MinLength") != null) { - try { - minVarLength = Integer.parseInt(getParameter("MinLength")); - } catch(Exception e) {} - } - return variable.length() < minVarLength; - } - - protected boolean tooLong(String variable) { - return variable.length() > MAX_VAR_LENGTH; - } - - protected boolean tooWordy(String variable) { - String[] words = variable.split("[A-Z_]"); - return words.length > MAX_VAR_WORDS; - } - - protected boolean isTemporary(String variable) { - String[] wordsToAvoid = {"temp", "tmp", "var", "func", "obj", "object", "bool", "struct", "string", "array"}; - String sentence = variable.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; - } - - protected boolean hasPrefixOrPostfix(String variable) { - String[] namesToAvoid = {"s", "st", "str", "o", "obj", "b", "q", "a", "arr", "this", "my"}; - String sentence = variable.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; - } } \ No newline at end of file From 8e72adc31b032af13b9287f603137712a262bee6 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 12:19:56 +1100 Subject: [PATCH 12/22] rearrange a little to group properties to rules --- src/test/java/com/cflint/TestCFBugs_VariableNames.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/cflint/TestCFBugs_VariableNames.java b/src/test/java/com/cflint/TestCFBugs_VariableNames.java index 67d2780cc..65601b9e8 100644 --- a/src/test/java/com/cflint/TestCFBugs_VariableNames.java +++ b/src/test/java/com/cflint/TestCFBugs_VariableNames.java @@ -37,13 +37,14 @@ public void setUp() { pluginMessage.setSeverity("INFO"); pluginRule.getMessages().add(pluginMessage); pluginRule.addParameter("MinLength", "3"); - + pluginRule.addParameter("MaxLength", "20"); pluginMessage = new PluginMessage("VAR_TOO_LONG"); pluginMessage.setSeverity("INFO"); pluginRule.getMessages().add(pluginMessage); pluginMessage = new PluginMessage("VAR_TOO_WORDY"); pluginMessage.setSeverity("INFO"); pluginRule.getMessages().add(pluginMessage); + pluginRule.addParameter("MaxWords", "4"); pluginMessage = new PluginMessage("VAR_IS_TEMPORARY"); pluginMessage.setSeverity("INFO"); pluginRule.getMessages().add(pluginMessage); From f09f42effdb4de1c139c36161288c2d7a3f93c6b Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 14:11:49 +1100 Subject: [PATCH 13/22] Fix function name not being set --- src/main/java/com/cflint/CFLint.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/cflint/CFLint.java b/src/main/java/com/cflint/CFLint.java index 63b58d6ed..24e777003 100644 --- a/src/main/java/com/cflint/CFLint.java +++ b/src/main/java/com/cflint/CFLint.java @@ -416,12 +416,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{ From 18b228fcf8eb7c9432c43891f7a4fa35ec00e689 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 14:12:48 +1100 Subject: [PATCH 14/22] Made code a bit more generic and renamed variable to name. Add default for method name rules. --- .../com/cflint/plugins/core/ValidName.java | 68 ++++++++++--------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/cflint/plugins/core/ValidName.java b/src/main/java/com/cflint/plugins/core/ValidName.java index 312a47804..18cb1afcc 100644 --- a/src/main/java/com/cflint/plugins/core/ValidName.java +++ b/src/main/java/com/cflint/plugins/core/ValidName.java @@ -8,48 +8,52 @@ public class ValidName { public static final int MAX_VAR_LENGTH = 20; public static final int MAX_VAR_WORDS = 4; - protected int minVarLength = MIN_VAR_LENGTH; - protected int maxVarLength = MAX_VAR_LENGTH; - protected int maxVarWords = MAX_VAR_WORDS; + 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 minVarLength, int maxVarLength, int maxVarWords) { - this.minVarLength = minVarLength; - this.maxVarLength = maxVarLength; - this.maxVarWords = maxVarWords; + public ValidName(int minLength, int maxLength, int maxWords) { + this.minLength = minLength; + this.maxLength = maxLength; + this.maxWords = maxWords; } - public boolean isInvalid(String variable) { - return !validChars(variable) || !(isSameCase(variable) || isCamelCase(variable) || usesUnderscores(variable)) || endsInNumber(variable); + public boolean isInvalid(String name) { + return !validChars(name) || !(isSameCase(name) || isCamelCase(name) || usesUnderscores(name)) || endsInNumber(name); } - public boolean validChars(String variable) { + public boolean validChars(String name) { Pattern valid = Pattern.compile("[A-Za-z0-9_]+"); - return valid.matcher(variable).matches(); + return valid.matcher(name).matches(); } - public boolean isUpperCase(String variable) { - return variable.toUpperCase().equals(variable); + public boolean isUpperCase(String name) { + return name.toUpperCase().equals(name); } - public boolean isSameCase(String variable) { - return variable.equals(variable.toLowerCase()) || variable.equals(variable.toUpperCase()); + public boolean isSameCase(String name) { + return name.equals(name.toLowerCase()) || name.equals(name.toUpperCase()); } - public boolean isCamelCase(String variable) { + public boolean isCamelCase(String name) { Pattern valid = Pattern.compile("[a-z0-9]+([A-Z][a-z0-9]+)*"); - return valid.matcher(variable).matches(); + return valid.matcher(name).matches(); } - public boolean usesUnderscores(String variable) { + public boolean usesUnderscores(String name) { Pattern valid = Pattern.compile("[a-z0-9]+[a-z0-9_]+"); - return valid.matcher(variable).matches(); + return valid.matcher(name).matches(); } - public boolean endsInNumber(String variable) { - char lastLetter = variable.charAt(variable.length() - 1); + public boolean endsInNumber(String name) { + char lastLetter = name.charAt(name.length() - 1); if (Character.isDigit(lastLetter)) { return true; @@ -58,22 +62,22 @@ public boolean endsInNumber(String variable) { return false; } - public boolean tooShort(String variable) { - return variable.length() < minVarLength; + public boolean tooShort(String name) { + return name.length() < minLength; } - public boolean tooLong(String variable) { - return variable.length() > maxVarLength; + public boolean tooLong(String name) { + return name.length() > maxLength; } - public boolean tooWordy(String variable) { - String[] words = variable.split("[A-Z_]"); - return words.length > maxVarWords; + public boolean tooWordy(String name) { + String[] words = name.split("[A-Z_]"); + return words.length > maxWords; } - public boolean isTemporary(String variable) { + public boolean isTemporary(String name) { String[] wordsToAvoid = {"temp", "tmp", "var", "func", "obj", "object", "bool", "struct", "string", "array"}; - String sentence = variable.replaceAll("_", " "); + String sentence = name.replaceAll("_", " "); sentence = sentence.replaceAll("(\\p{Ll})(\\p{Lu})","$1 $2"); String[] words = sentence.split(" "); @@ -89,9 +93,9 @@ public boolean isTemporary(String variable) { return false; } - public boolean hasPrefixOrPostfix(String variable) { + public boolean hasPrefixOrPostfix(String name) { String[] namesToAvoid = {"s", "st", "str", "o", "obj", "b", "q", "a", "arr", "this", "my"}; - String sentence = variable.replaceAll("_", " "); + String sentence = name.replaceAll("_", " "); sentence = sentence.replaceAll("(\\p{Ll})(\\p{Lu})","$1 $2"); String[] words = sentence.split(" "); String firstWord = words[0]; From 0b5455a7a4ade730805fec731c2b2359547af481 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 14:13:15 +1100 Subject: [PATCH 15/22] Rules for method names --- .../plugins/core/MethodNameChecker.java | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 src/main/java/com/cflint/plugins/core/MethodNameChecker.java diff --git a/src/main/java/com/cflint/plugins/core/MethodNameChecker.java b/src/main/java/com/cflint/plugins/core/MethodNameChecker.java new file mode 100644 index 000000000..ef1a88fe4 --- /dev/null +++ b/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()); + } + } +} \ No newline at end of file From 0441720cba9e5a2f4abd5dbf455d3dd2aa8dad08 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 14:13:46 +1100 Subject: [PATCH 16/22] fix parameter setting code --- .../java/com/cflint/plugins/core/VariableNameChecker.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java index 700c7d0c6..552f2b99a 100644 --- a/src/main/java/com/cflint/plugins/core/VariableNameChecker.java +++ b/src/main/java/com/cflint/plugins/core/VariableNameChecker.java @@ -53,13 +53,13 @@ public void checkNameForBugs(String variable, String filename, int line, BugList if (getParameter("MaxLength") != null) { try { - minVarLength = Integer.parseInt(getParameter("MaxLength")); + maxVarLength = Integer.parseInt(getParameter("MaxLength")); } catch(Exception e) {} } if (getParameter("MaxWords") != null) { try { - minVarLength = Integer.parseInt(getParameter("MaxWords")); + maxVarWords = Integer.parseInt(getParameter("MaxWords")); } catch(Exception e) {} } From 902859a6a73a1499b83663b768590faf65bc6ba4 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 14:14:08 +1100 Subject: [PATCH 17/22] added method name rules --- src/main/resources/cflint.definition.xml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/main/resources/cflint.definition.xml b/src/main/resources/cflint.definition.xml index 8c8b851e1..a67f88eb6 100644 --- a/src/main/resources/cflint.definition.xml +++ b/src/main/resources/cflint.definition.xml @@ -226,4 +226,27 @@ INFO + + + INFO + + + INFO + + + INFO + + + INFO + + + INFO + + + INFO + + + INFO + + From 402d2bb20356aaf08a5a57cd77ec3218c36d823a Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 14:14:38 +1100 Subject: [PATCH 18/22] no need to set parameter twice --- src/test/java/com/cflint/TestCFBugs_VariableNames.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/com/cflint/TestCFBugs_VariableNames.java b/src/test/java/com/cflint/TestCFBugs_VariableNames.java index 65601b9e8..91fadf49a 100644 --- a/src/test/java/com/cflint/TestCFBugs_VariableNames.java +++ b/src/test/java/com/cflint/TestCFBugs_VariableNames.java @@ -53,7 +53,6 @@ public void setUp() { pluginRule.getMessages().add(pluginMessage); final VariableNameChecker checker = new VariableNameChecker(); - checker.setParameter("MinLength", "3"); cfBugs = new CFLint(conf, checker); } From 9c4c2fc253e5d161ce68ef3264ea1e69a316d57e Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Sun, 18 Oct 2015 14:14:55 +1100 Subject: [PATCH 19/22] Added rules for method names --- .../com/cflint/TestCFBugs_MethodNames.java | 290 ++++++++++++++++++ 1 file changed, 290 insertions(+) create mode 100644 src/test/java/com/cflint/TestCFBugs_MethodNames.java diff --git a/src/test/java/com/cflint/TestCFBugs_MethodNames.java b/src/test/java/com/cflint/TestCFBugs_MethodNames.java new file mode 100644 index 000000000..664b7bf04 --- /dev/null +++ b/src/test/java/com/cflint/TestCFBugs_MethodNames.java @@ -0,0 +1,290 @@ +package com.cflint; + +import static org.junit.Assert.assertEquals; + +import java.io.IOException; +import java.util.Collection; +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.MethodNameChecker; + +public class TestCFBugs_MethodNames { + + private CFLint cfBugs; + + @Before + public void setUp() { + final ConfigRuntime conf = new ConfigRuntime(); + final PluginInfoRule pluginRule = new PluginInfoRule(); + pluginRule.setName("MethodNameChecker"); + conf.getRules().add(pluginRule); + PluginMessage pluginMessage = new PluginMessage("METHOD_INVALID_NAME"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginMessage = new PluginMessage("METHOD_ALLCAPS_NAME"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginMessage = new PluginMessage("METHOD_TOO_SHORT"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginRule.addParameter("MinLength", "3"); + pluginRule.addParameter("MaxLength", "20"); + pluginMessage = new PluginMessage("METHOD_TOO_LONG"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginMessage = new PluginMessage("METHOD_TOO_WORDY"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginRule.addParameter("MaxWords", "4"); + pluginMessage = new PluginMessage("METHOD_IS_TEMPORARY"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + pluginMessage = new PluginMessage("METHOD_HAS_PREFIX_OR_POSTFIX"); + pluginMessage.setSeverity("INFO"); + pluginRule.getMessages().add(pluginMessage); + + final MethodNameChecker checker = new MethodNameChecker(); + cfBugs = new CFLint(conf, checker); + } + + @Test + public void testValidNamesTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + ""; + cfBugs.process(tagSrc, "test"); + Collection> result = cfBugs.getBugs().getBugList().values(); + assertEquals(0, result.size()); + } + + @Test + public void testUpercaseNameTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + ""; + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_ALLCAPS_NAME", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void invalidCharsInNameTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + ""; + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_INVALID_NAME", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameEndsInNumberTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + ""; + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_INVALID_NAME", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameTooShortTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + ""; + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_TOO_SHORT", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameTooLongTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + ""; + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_TOO_LONG", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameTooWordyTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + ""; + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_TOO_WORDY", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameIsTemporyTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + ""; + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().get("METHOD_IS_TEMPORARY"); + assertEquals(1, result.size()); + assertEquals("METHOD_IS_TEMPORARY", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameHasPrefixOrPostfixTag() throws ParseException, IOException { + final String tagSrc = "\r\n" + + "\r\n" + + "\r\n" + + ""; + cfBugs.process(tagSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_HAS_PREFIX_OR_POSTFIX", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + + @Test + public void testValidNamesScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function test() {\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + Collection> result = cfBugs.getBugs().getBugList().values(); + assertEquals(0, result.size()); + } + + @Test + public void testUpercaseNameScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function UPPERCASE() {\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_ALLCAPS_NAME", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void invalidCharsInNameScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function method$name() {\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_INVALID_NAME", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameEndsInNumberScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function method23() {\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_INVALID_NAME", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameTooShortScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function a() {\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_TOO_SHORT", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameTooLongScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function isaveryveryverylongmethodname() {\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_TOO_LONG", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameTooWordyScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function isFarTooWordyMethodName() {\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_TOO_WORDY", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameIsTemporyScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function temp() {\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().get("METHOD_IS_TEMPORARY"); + assertEquals(1, result.size()); + assertEquals("METHOD_IS_TEMPORARY", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + + @Test + public void nameHasPrefixOrPostfixScript() throws ParseException, IOException { + final String scriptSrc = "component {\r\n" + + "function thisMethod() {\r\n" + + "}\r\n" + + "}"; + cfBugs.process(scriptSrc, "test"); + final List result = cfBugs.getBugs().getBugList().values().iterator().next(); + assertEquals(1, result.size()); + assertEquals("METHOD_HAS_PREFIX_OR_POSTFIX", result.get(0).getMessageCode()); + assertEquals(2, result.get(0).getLine()); + } + +} From 36b33e2961cb766b9b0b9c3ad8a6e4a775958b8c Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Tue, 20 Oct 2015 16:55:05 +1100 Subject: [PATCH 20/22] Tests for valid names --- src/test/java/com/cflint/TestValidNames.java | 158 +++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 src/test/java/com/cflint/TestValidNames.java diff --git a/src/test/java/com/cflint/TestValidNames.java b/src/test/java/com/cflint/TestValidNames.java new file mode 100644 index 000000000..e0fcdd05d --- /dev/null +++ b/src/test/java/com/cflint/TestValidNames.java @@ -0,0 +1,158 @@ +package com.cflint; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; + +import java.io.IOException; + +import org.junit.Before; +import org.junit.Test; + +import com.cflint.plugins.core.ValidName; + +public class TestValidNames { + + private ValidName name; + + @Before + public void setUp() { + name = new ValidName(3, 20, 3); + } + + @Test + public void testInvalid() { + assertFalse(name.isInvalid("camelCase")); + assertFalse(name.isInvalid("product_id")); + assertFalse(name.isInvalid("size")); + assertFalse(name.isInvalid("lowercase")); + assertFalse(name.isInvalid("UPPERCASE")); + assertFalse(name.isInvalid("99_bottles_of_beer")); + + assertTrue(name.isInvalid("UppperCaseCamel")); + assertTrue(name.isInvalid("endsInNumber99")); + } + + @Test + public void testValidChars() { + assertTrue(name.validChars("camelCase")); + assertTrue(name.validChars("product_id")); + assertTrue(name.validChars("99_bottles_of_beer")); + + assertFalse(name.validChars("no$items")); + assertFalse(name.validChars("no-items")); + assertFalse(name.validChars("no+items")); + } + + @Test + public void testIsCamelCaseLower() { + assertTrue(name.isCamelCaseLower("product")); + assertTrue(name.isCamelCaseLower("camelCase")); + assertTrue(name.isCamelCaseLower("product4G")); + assertTrue(name.isCamelCaseLower("productID")); + assertTrue(name.isCamelCaseLower("requestViaHTTPS")); + assertTrue(name.isCamelCaseLower("myURLRequest")); + + assertFalse(name.isCamelCaseLower("product_id")); + assertFalse(name.isCamelCaseLower("UpperCase")); + assertFalse(name.isCamelCaseLower("Uppercase")); + assertFalse(name.isCamelCaseLower("ProductID")); + assertFalse(name.isCamelCaseLower("PRODUCTID")); + } + + @Test + public void testIsCamelCaseUpper() { + assertTrue(name.isCamelCaseUpper("Product")); + assertTrue(name.isCamelCaseUpper("PamelCase")); + assertTrue(name.isCamelCaseUpper("Product4G")); + assertTrue(name.isCamelCaseUpper("ProductID")); + assertTrue(name.isCamelCaseUpper("RequestViaHTTPS")); + assertTrue(name.isCamelCaseUpper("MyURLRequest")); + + assertFalse(name.isCamelCaseUpper("product")); + assertFalse(name.isCamelCaseUpper("Product_id")); + assertFalse(name.isCamelCaseUpper("Product_Id")); + assertFalse(name.isCamelCaseUpper("PRODUCTID")); + } + + @Test + public void testUsesUnderscores() { + assertTrue(name.usesUnderscores("product_name")); + assertTrue(name.usesUnderscores("_atstart")); + assertTrue(name.usesUnderscores("atend_")); + assertTrue(name.usesUnderscores("lots_of_under_scores")); + + assertFalse(name.usesUnderscores("productName")); + assertFalse(name.usesUnderscores("productname")); + } + + @Test + public void testEndsInNumber() { + assertTrue(name.endsInNumber("endsInNumber99")); + assertTrue(name.endsInNumber("temp2")); + + assertFalse(name.endsInNumber("productName")); + assertFalse(name.endsInNumber("phone4G")); + assertFalse(name.endsInNumber("99beers")); + } + + @Test + public void testTooShort() { + assertTrue(name.tooShort("a")); + assertTrue(name.tooShort("to")); + + assertFalse(name.tooShort("the")); + assertFalse(name.tooShort("chars")); + assertFalse(name.tooShort("averylongname")); + } + + @Test + public void testTooLong() { + assertTrue(name.tooLong("123456789012345678901")); + assertTrue(name.tooLong("aVeryLongNameThatIsTooLong")); + + assertFalse(name.tooLong("shortName")); + assertFalse(name.tooLong("short_name")); + assertFalse(name.tooLong("12345678901234567890")); + } + + + @Test + public void testTooWordy() { + assertTrue(name.tooWordy("thisIsAVeryWordyName")); + assertTrue(name.tooWordy("alsoFarTooWordy")); + + assertFalse(name.tooWordy("ProductName")); + assertFalse(name.tooWordy("aSimpleName")); + assertFalse(name.tooWordy("simple")); + } + + @Test + public void testIsTemporary() { + assertTrue(name.isTemporary("temp")); + assertTrue(name.isTemporary("tmp")); + assertTrue(name.isTemporary("myVar")); + assertTrue(name.isTemporary("aFunc")); + assertTrue(name.isTemporary("nameObj")); + assertTrue(name.isTemporary("name_obj")); + assertTrue(name.isTemporary("tmp_array")); + + assertFalse(name.isTemporary("temperature")); + assertFalse(name.isTemporary("productName")); + assertFalse(name.isTemporary("first_name")); + } + + @Test + public void testHasPrefixOrPostfix() { + assertTrue(name.hasPrefixOrPostfix("stName")); + assertTrue(name.hasPrefixOrPostfix("oPerson")); + assertTrue(name.hasPrefixOrPostfix("qGet")); + assertTrue(name.hasPrefixOrPostfix("nameStr")); + assertTrue(name.hasPrefixOrPostfix("bValid")); + + assertFalse(name.isTemporary("temperature")); + assertFalse(name.isTemporary("strength")); + assertFalse(name.isTemporary("argument")); + assertFalse(name.isTemporary("first_name")); + } +} From 536346ebf5d8ee00f0531d719654559ba066d2a9 Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Tue, 20 Oct 2015 16:55:34 +1100 Subject: [PATCH 21/22] improve regular expressions and fix up word count --- .../com/cflint/plugins/core/ValidName.java | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/cflint/plugins/core/ValidName.java b/src/main/java/com/cflint/plugins/core/ValidName.java index 18cb1afcc..1c8aa63f0 100644 --- a/src/main/java/com/cflint/plugins/core/ValidName.java +++ b/src/main/java/com/cflint/plugins/core/ValidName.java @@ -26,11 +26,11 @@ public ValidName(int minLength, int maxLength, int maxWords) { } public boolean isInvalid(String name) { - return !validChars(name) || !(isSameCase(name) || isCamelCase(name) || usesUnderscores(name)) || endsInNumber(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_]+"); + Pattern valid = Pattern.compile("^[A-Za-z0-9_]+$"); return valid.matcher(name).matches(); } @@ -42,16 +42,21 @@ public boolean isSameCase(String name) { return name.equals(name.toLowerCase()) || name.equals(name.toUpperCase()); } - public boolean isCamelCase(String name) { - Pattern valid = Pattern.compile("[a-z0-9]+([A-Z][a-z0-9]+)*"); + 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 usesUnderscores(String name) { - Pattern valid = Pattern.compile("[a-z0-9]+[a-z0-9_]+"); + 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); @@ -71,12 +76,20 @@ public boolean tooLong(String name) { } public boolean tooWordy(String name) { - String[] words = name.split("[A-Z_]"); - return words.length > maxWords; + 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"}; + 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(" "); From 84e5241b89f2d633d9d8a3452e302d778f8603da Mon Sep 17 00:00:00 2001 From: Justin Mclean Date: Tue, 20 Oct 2015 17:06:37 +1100 Subject: [PATCH 22/22] Add JSON config for variable and method names --- src/main/resources/cflint.definition.json | 96 +++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/src/main/resources/cflint.definition.json b/src/main/resources/cflint.definition.json index 572e96894..eb44e8d5d 100644 --- a/src/main/resources/cflint.definition.json +++ b/src/main/resources/cflint.definition.json @@ -449,6 +449,102 @@ } ], "parameter": [] + }, + { + "name": "VariableNameChecker", + "className": "VariableNameChecker", + "message": [ + { + "code": "VAR_INVALID_NAME", + "severity": "INFO" + }, + { + "code": "VAR_ALLCAPS_NAME", + "severity": "INFO" + }, + { + "code": "VAR_TOO_SHORT", + "severity": "INFO" + }, + { + "code": "VAR_TOO_LONG", + "severity": "INFO" + }, + { + "code": "VAR_TOO_WORDY", + "severity": "INFO" + }, + { + "code": "VAR_IS_TEMPORARY", + "severity": "INFO" + }, + { + "code": "VAR_HAS_PREFIX_OR_POSTFIX", + "severity": "INFO" + } + ], + "parameter": [ + { + "name": "MinLength", + "value": "3" + }, + { + "name": "MaxLength", + "value": "20" + }, + { + "name": "MaxWords", + "value": "4" + } + ] + }, + { + "name": "MethodNameChecker", + "className": "MethodNameChecker", + "message": [ + { + "code": "METHOD_INVALID_NAME", + "severity": "INFO" + }, + { + "code": "METHOD_ALLCAPS_NAME", + "severity": "INFO" + }, + { + "code": "METHOD_TOO_SHORT", + "severity": "INFO" + }, + { + "code": "METHOD_TOO_LONG", + "severity": "INFO" + }, + { + "code": "METHOD_TOO_WORDY", + "severity": "INFO" + }, + { + "code": "METHOD_IS_TEMPORARY", + "severity": "INFO" + }, + { + "code": "METHOD_HAS_PREFIX_OR_POSTFIX", + "severity": "INFO" + } + ], + "parameter": [ + { + "name": "MinLength", + "value": "3" + }, + { + "name": "MaxLength", + "value": "25" + }, + { + "name": "MaxWords", + "value": "5" + } + ] } ] } \ No newline at end of file