diff --git a/build.hxml b/build.hxml index 0a861ba5..959fb22f 100644 --- a/build.hxml +++ b/build.hxml @@ -13,6 +13,6 @@ -x TestMain --next --cmd neko run -s src -s test -p resources/static-analysis.txt -c resources/checkstyle.json -e resources/checkstyle-exclude.json +-cmd neko run -s src -s test -p resources/static-analysis.txt -cmd neko run --default-config resources/default-config.json -cmd neko run -c resources/default-config.json \ No newline at end of file diff --git a/resources/checkstyle-exclude.json b/checkstyle-exclude.json similarity index 100% rename from resources/checkstyle-exclude.json rename to checkstyle-exclude.json diff --git a/resources/checkstyle.json b/checkstyle.json similarity index 99% rename from resources/checkstyle.json rename to checkstyle.json index a5f1be9b..3346f62e 100644 --- a/resources/checkstyle.json +++ b/checkstyle.json @@ -7,6 +7,12 @@ }, "type": "Anonymous" }, + { + "props": { + "severity": "INFO" + }, + "type": "ArrayAccess" + }, { "props": { "severity": "INFO" diff --git a/package.json b/package.json index 4e72d5ac..142a580e 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,6 @@ "license": "MIT", "devDependencies": { "grunt": "^0.4.5", - "grunt-contrib-copy": "^0.7.0", "grunt-haxe": "^0.1.12", "grunt-shell": "^1.1.1", "grunt-zip": "^0.16.0" diff --git a/resources/default-config.json b/resources/default-config.json index e4595e39..4d3ea13f 100644 --- a/resources/default-config.json +++ b/resources/default-config.json @@ -9,6 +9,14 @@ }, "type": "Anonymous" }, + { + "props": { + "spaceBefore": false, + "spaceInside": false, + "severity": "IGNORE" + }, + "type": "ArrayAccess" + }, { "props": { "severity": "IGNORE" diff --git a/src/checkstyle/Checker.hx b/src/checkstyle/Checker.hx index c31a3cd5..a99dd3d0 100644 --- a/src/checkstyle/Checker.hx +++ b/src/checkstyle/Checker.hx @@ -49,9 +49,7 @@ class Checker { public function getTokenTree():TokenTree { if (tokens == null) return null; - if (tokenTree == null) { - tokenTree = TokenTreeBuilder.buildTokenTree(tokens); - } + if (tokenTree == null) tokenTree = TokenTreeBuilder.buildTokenTree(tokens); return tokenTree; } @@ -75,13 +73,15 @@ class Checker { public function getLinePos(off:Int):LinePos { for (i in 0...linesIdx.length) { - if (linesIdx[i].l <= off && linesIdx[i].r >= off) { - return { line:i, ofs: off - linesIdx[i].l }; - } + if (linesIdx[i].l <= off && linesIdx[i].r >= off) return { line:i, ofs: off - linesIdx[i].l }; } throw "Bad offset"; } + public function getString(off:Int, off2:Int):String { + return file.content.substr(off, off2 - off); + } + function findLineSeparator() { var code = file.content; for (i in 0 ... code.length) { @@ -121,9 +121,7 @@ class Checker { function makeASTs() { asts = [makeAST(baseDefines)]; - for (combination in defineCombinations) { - asts.push(makeAST(combination.concat(baseDefines))); - } + for (combination in defineCombinations) asts.push(makeAST(combination.concat(baseDefines))); } function makeAST(defines:Array):Ast { @@ -162,9 +160,7 @@ class Checker { function loadFileContent(lintFile:LintFile) { // unittests set content before running Checker // real checks load content here - if (lintFile.content == null) { - lintFile.content = File.getContent(lintFile.name); - } + if (lintFile.content == null) lintFile.content = File.getContent(lintFile.name); } function unloadFileContent(lintFile:LintFile) { @@ -270,13 +266,12 @@ class Checker { function getErrorMessage(e:Dynamic, fileName:String, step:String):LintMessage { return { fileName:fileName, - message:step + " failed: " + - e + "\nStacktrace: " + CallStack.toString(CallStack.exceptionStack()), line:1, startColumn:0, endColumn:0, severity:ERROR, - moduleName:"Checker" + moduleName:"Checker", + message:step + " failed: " + e + "\nStacktrace: " + CallStack.toString(CallStack.exceptionStack()) }; } } diff --git a/src/checkstyle/ChecksInfo.hx b/src/checkstyle/ChecksInfo.hx index 89368717..6344380d 100644 --- a/src/checkstyle/ChecksInfo.hx +++ b/src/checkstyle/ChecksInfo.hx @@ -19,9 +19,9 @@ class ChecksInfo { var desc = getCheckDescription(cl); checkInfos[names[i]] = { name: names[i], - description: (i == 0) ? desc : desc + " [DEPRECATED, use " + names[0] + " instead]", clazz: cl, - isAlias: i > 0 + isAlias: i > 0, + description: (i == 0) ? desc : desc + " [DEPRECATED, use " + names[0] + " instead]" }; } } diff --git a/src/checkstyle/Main.hx b/src/checkstyle/Main.hx index 90a0d6ad..a080b0cb 100644 --- a/src/checkstyle/Main.hx +++ b/src/checkstyle/Main.hx @@ -114,27 +114,26 @@ class Main { function loadExcludeConfig(excludeConfigPath:String) { var config:Config = Json.parse(File.getContent(excludeConfigPath)); var excludes = Reflect.fields(config); - for (e in excludes) { - createExcludeMapElement(e); - var excludeValues:Array = Reflect.field(config, e); - if (excludeValues != null && excludeValues.length > 0) { - for (val in excludeValues) { - for (p in paths) { - var path = p + "/" + val.split(".").join("/"); - if (e == "all") allExcludes.push(path); - else { - if (!p.contains(":")) excludesMap.get(e).push(path); - } - } - } - } + for (exclude in excludes) { + createExcludeMapElement(exclude); + var excludeValues:Array = Reflect.field(config, exclude); + if (excludeValues == null || excludeValues.length == 0) continue; + for (val in excludeValues) updateExcludes(exclude, val); } start(); } - function createExcludeMapElement(name:String) { - if (excludesMap.get(name) == null) excludesMap.set(name, []); + function createExcludeMapElement(exclude:String) { + if (excludesMap.get(exclude) == null) excludesMap.set(exclude, []); + } + + function updateExcludes(exclude:String, val:String) { + for (p in paths) { + var path = p + "/" + val.split(".").join("/"); + if (exclude == "all") allExcludes.push(path); + else excludesMap.get(exclude).push(path); + } } function createCheck(checkConf:CheckConfig):Check { diff --git a/src/checkstyle/checks/Check.hx b/src/checkstyle/checks/Check.hx index 7fa4c346..8167c6b4 100644 --- a/src/checkstyle/checks/Check.hx +++ b/src/checkstyle/checks/Check.hx @@ -176,17 +176,11 @@ class Check { switch (td.decl){ case EAbstract(d): case EClass(d): - if ((pos <= td.pos.max) && (pos >= td.pos.min)) { - return d.flags.contains(HExtern); - } + if ((pos <= td.pos.max) && (pos >= td.pos.min)) return d.flags.contains(HExtern); case EEnum(d): - if ((pos <= td.pos.max) && (pos >= td.pos.min)) { - return d.flags.contains(EExtern); - } + if ((pos <= td.pos.max) && (pos >= td.pos.min)) return d.flags.contains(EExtern); case ETypedef(d): - if ((pos <= td.pos.max) && (pos >= td.pos.min)) { - return d.flags.contains(EExtern); - } + if ((pos <= td.pos.max) && (pos >= td.pos.min)) return d.flags.contains(EExtern); switch (d.data) { case TAnonymous(fields): for (field in fields) { diff --git a/src/checkstyle/checks/EmptyPackageCheck.hx b/src/checkstyle/checks/EmptyPackageCheck.hx index f1470cc3..657a7c8f 100644 --- a/src/checkstyle/checks/EmptyPackageCheck.hx +++ b/src/checkstyle/checks/EmptyPackageCheck.hx @@ -18,13 +18,9 @@ class EmptyPackageCheck extends Check { var root:TokenTree = checker.getTokenTree(); var packageTokens = root.filter([Kwd(KwdPackage)], ALL); if (enforceEmptyPackage) { - if (packageTokens.length == 0) { - log("Missing package declaration", 1, 0, 0); - } - } - else { - checkPackageNames(packageTokens); + if (packageTokens.length == 0) log("Missing package declaration", 1, 0, 0); } + else checkPackageNames(packageTokens); } function checkPackageNames(entries:Array) { diff --git a/src/checkstyle/checks/coding/ArrayAccessCheck.hx b/src/checkstyle/checks/coding/ArrayAccessCheck.hx new file mode 100644 index 00000000..923bfc15 --- /dev/null +++ b/src/checkstyle/checks/coding/ArrayAccessCheck.hx @@ -0,0 +1,47 @@ +package checkstyle.checks.coding; + +import haxe.macro.Expr; +import checkstyle.utils.ExprUtils; + +@name("ArrayAccess") +@desc("Spacing check on array access") +class ArrayAccessCheck extends Check { + + public var spaceBefore:Bool; + public var spaceInside:Bool; + + public function new() { + super(AST); + spaceBefore = false; + spaceInside = false; + } + + override function actualRun() { + var lastExpr = null; + + ExprUtils.walkFile(checker.ast, function(e:Expr) { + if (lastExpr == null) { + lastExpr = e; + return; + } + + switch (e.expr) { + case EArray(e1, e2): + if (!spaceBefore) { + var e1length = e1.pos.max - e1.pos.min; + var eString = checker.getString(e.pos.min, e.pos.max); + if (eString.substr(e1length, 1) == " ") logPos('Space between array and [', e.pos); + } + + if (!spaceInside) { + var eString = checker.getString(e.pos.min, e.pos.max); + if (checker.file.content.substr(e2.pos.min - 1, 1) == " ") logPos('Space between [ and index', e.pos); + if (checker.file.content.substr(e2.pos.max, 1) == " ") logPos('Space between index and ]', e.pos); + } + default: + } + + lastExpr = e; + }); + } +} \ No newline at end of file diff --git a/src/checkstyle/checks/whitespace/TabForAligningCheck.hx b/src/checkstyle/checks/whitespace/TabForAligningCheck.hx index 0832fe0c..59ca90a0 100644 --- a/src/checkstyle/checks/whitespace/TabForAligningCheck.hx +++ b/src/checkstyle/checks/whitespace/TabForAligningCheck.hx @@ -15,7 +15,7 @@ class TabForAligningCheck extends Check { for (i in 0 ... checker.lines.length) { var line = checker.lines[i]; if (re.match(line) && !line.contains("//")) { - log("Tab after non-space character, Use space for aligning", i + 1, line.length); + log("Tab after non-space character, use space for aligning", i + 1, line.length); } } } diff --git a/src/checkstyle/reporter/JSONReporter.hx b/src/checkstyle/reporter/JSONReporter.hx index 76c82199..39aed360 100644 --- a/src/checkstyle/reporter/JSONReporter.hx +++ b/src/checkstyle/reporter/JSONReporter.hx @@ -1,8 +1,8 @@ package checkstyle.reporter; import checkstyle.reporter.BaseReporter; -import haxe.Json; import checkstyle.LintMessage.SeverityLevel; +import haxe.Json; class JSONReporter extends BaseReporter { diff --git a/src/checkstyle/reporter/ProgressReporter.hx b/src/checkstyle/reporter/ProgressReporter.hx index a6292d4c..aeddfcec 100644 --- a/src/checkstyle/reporter/ProgressReporter.hx +++ b/src/checkstyle/reporter/ProgressReporter.hx @@ -22,9 +22,7 @@ class ProgressReporter implements IReporter { lineLength = line.length; Sys.print(line); - if (f.index == numFiles - 1) { - Sys.print("\n"); - } + if (f.index == numFiles - 1) Sys.print("\n"); } function clear() { diff --git a/src/checkstyle/reporter/TextReporter.hx b/src/checkstyle/reporter/TextReporter.hx index 8f287890..2ff6b439 100644 --- a/src/checkstyle/reporter/TextReporter.hx +++ b/src/checkstyle/reporter/TextReporter.hx @@ -8,7 +8,6 @@ class TextReporter extends BaseReporter { override public function addMessage(m:LintMessage) { var sb:StringBuf = getMessage(m); - var output:Output = Sys.stderr(); switch (m.severity) { diff --git a/src/checkstyle/token/TokenStream.hx b/src/checkstyle/token/TokenStream.hx index 2731c64d..785bc56f 100644 --- a/src/checkstyle/token/TokenStream.hx +++ b/src/checkstyle/token/TokenStream.hx @@ -1,7 +1,6 @@ package checkstyle.token; import haxe.macro.Expr; - import haxeparser.Data.Token; import haxeparser.Data.TokenDef; @@ -83,6 +82,7 @@ class TokenStream { default: return false; } } + return false; } public function token():TokenDef { @@ -108,7 +108,6 @@ class TokenStream { * '>>>=' -> Binop(OpAssignOp(OpUShr)) * */ - public function consumeOpGt():TokenTree { var tok:TokenTree = consumeTokenDef(Binop(OpGt)); switch (token()) { @@ -165,9 +164,7 @@ class TokenStream { * This function provides a workaround, which scans the tokens around * Binop(OpSub) to see if the token stream should contain a negative const * value and returns a proper Const(CInt(-x)) or Const(CFloat(-x)) token - * */ - public function consumeOpSub():TokenTree { var tok:Token = consumeTokenDef(Binop(OpSub)); switch (token()) { diff --git a/src/checkstyle/token/TokenTree.hx b/src/checkstyle/token/TokenTree.hx index 3f7bbe51..1ea68957 100644 --- a/src/checkstyle/token/TokenTree.hx +++ b/src/checkstyle/token/TokenTree.hx @@ -1,7 +1,6 @@ package checkstyle.token; import haxe.macro.Expr; - import haxeparser.Data.Token; import haxeparser.Data.TokenDef; @@ -13,10 +12,6 @@ class TokenTree extends Token { public var previousSibling:TokenTree; public var childs:Array; - public function new(tok:TokenDef, pos:Position) { - super(tok, pos); - } - public function is(tokenDef:TokenDef):Bool { if (tok == null) return false; return Type.enumEq(tokenDef, tok); @@ -59,18 +54,12 @@ class TokenTree extends Token { public function filter(searchFor:Array, mode:TokenFilterMode, maxLevel:Int = MAX_LEVEL):Array { return filterCallback(function(token:TokenTree, depth:Int):FilterResult { - if (depth > maxLevel) { - return SKIP_SUBTREE; - } + if (depth > maxLevel) return SKIP_SUBTREE; if (token.matchesAny(searchFor)) { - if (mode == ALL) { - return FOUND_GO_DEEPER; - } + if (mode == ALL) return FOUND_GO_DEEPER; return FOUND_SKIP_SUBTREE; } - else { - return GO_DEEPER; - } + else return GO_DEEPER; }); } @@ -101,9 +90,7 @@ class TokenTree extends Token { function matchesAny(searchFor:Array):Bool { if (searchFor == null || tok == null) return false; for (search in searchFor) { - if (Type.enumEq(tok, search)) { - return true; - } + if (Type.enumEq(tok, search)) return true; } return false; } @@ -116,9 +103,7 @@ class TokenTree extends Token { var buf:StringBuf = new StringBuf(); if (tok != null) buf.add('$prefix${tok}\t\t\t\t${getPos()}'); if (childs == null) return buf.toString(); - for (child in childs) { - buf.add('\n$prefix${child.printTokenTree(prefix + " ")}'); - } + for (child in childs) buf.add('\n$prefix${child.printTokenTree(prefix + " ")}'); return buf.toString(); } } diff --git a/src/checkstyle/token/TokenTreeBuilder.hx b/src/checkstyle/token/TokenTreeBuilder.hx index 29983a63..b5a83668 100644 --- a/src/checkstyle/token/TokenTreeBuilder.hx +++ b/src/checkstyle/token/TokenTreeBuilder.hx @@ -81,7 +81,6 @@ class TokenTreeBuilder { * |- Const(CIdent) * */ - function walkAt():TokenTree { var atTok:TokenTree = stream.consumeTokenDef(At); var parent:TokenTree = atTok; @@ -893,7 +892,6 @@ class TokenTreeBuilder { * |- BrClose * */ - function walkCase(parent:TokenTree) { if (!stream.is(Kwd(KwdCase)) && !stream.is(Kwd(KwdDefault))) { throw 'bad token ${stream.token()} != case/default'; @@ -978,7 +976,6 @@ class TokenTreeBuilder { * |- BrClose * */ - function walkTry(parent:TokenTree) { var tryTok:TokenTree = stream.consumeTokenDef(Kwd(KwdTry)); parent.addChild(tryTok); @@ -996,7 +993,6 @@ class TokenTreeBuilder { * |- BrClose * */ - function walkCatch(parent:TokenTree) { var catchTok:TokenTree = stream.consumeTokenDef(Kwd(KwdCatch)); parent.addChild(catchTok); @@ -1016,7 +1012,6 @@ class TokenTreeBuilder { * |- BrClose * */ - function walkWhile(parent:TokenTree) { var whileTok:TokenTree = stream.consumeTokenDef(Kwd(KwdWhile)); parent.addChild(whileTok); @@ -1039,7 +1034,6 @@ class TokenTreeBuilder { * |- Semicolon * */ - function walkDoWhile(parent:TokenTree) { var doTok:TokenTree = stream.consumeTokenDef(Kwd(KwdDo)); parent.addChild(doTok); @@ -1077,7 +1071,6 @@ class TokenTreeBuilder { * |- BrClose * */ - function walkFor(parent:TokenTree) { var forTok:TokenTree = stream.consumeTokenDef(Kwd(KwdFor)); parent.addChild(forTok); @@ -1102,7 +1095,6 @@ class TokenTreeBuilder { * |- PClose * */ - function walkForPOpen(parent:TokenTree) { var pOpen:TokenTree = stream.consumeTokenDef(POpen); var iterator:TokenTree = stream.consumeConstIdent(); @@ -1142,7 +1134,6 @@ class TokenTreeBuilder { * Sharp(_) * */ - function walkSharp(parent:TokenTree) { switch (stream.token()) { case Sharp("if"), Sharp("elseif"): diff --git a/test/checks/coding/ArrayAccessCheckTest.hx b/test/checks/coding/ArrayAccessCheckTest.hx new file mode 100644 index 00000000..b7a907cd --- /dev/null +++ b/test/checks/coding/ArrayAccessCheckTest.hx @@ -0,0 +1,85 @@ +package checks.coding; + +import checkstyle.checks.coding.ArrayAccessCheck; + +class ArrayAccessCheckTest extends CheckTestCase { + + public function testSpaceBefore() { + assertMsg(new ArrayAccessCheck(), TEST1, "Space between array and ["); + } + + public function testSpaceInside() { + var check = new ArrayAccessCheck(); + assertMsg(check, TEST2, "Space between [ and index"); + assertMsg(check, TEST3, "Space between index and ]"); + assertMsg(check, TEST4, "Space between index and ]"); + } + + public function testAllowSpaceInside() { + var check = new ArrayAccessCheck(); + check.spaceBefore = true; + check.spaceInside = true; + assertNoMsg(check, TEST1); + assertNoMsg(check, TEST2); + assertNoMsg(check, TEST3); + assertNoMsg(check, TEST4); + } + + public function testCorrectUsage() { + var check = new ArrayAccessCheck(); + assertNoMsg(check, TEST5); + } +} + +@:enum +abstract ArrayAccessCheckTests(String) to String { + var TEST1 = " + class Test { + + var a:Array = []; + + function a() { + a [0] = 1; + } + }"; + + var TEST2 = " + class Test { + + var a:Array = []; + + function a() { + a[ 0] = 1; + } + }"; + + var TEST3 = " + class Test { + + var a:Array = []; + + function a() { + a[0 ] = 1; + } + }"; + + var TEST4 = " + class Test { + + var a:Array = []; + + function a() { + a[ 0 ] = 1; + } + }"; + + var TEST5 = " + class Test { + + var a:Array = []; + + function a() { + a[0] = 1; + } + }"; +} \ No newline at end of file diff --git a/test/checks/whitespace/TabForAligningCheckTest.hx b/test/checks/whitespace/TabForAligningCheckTest.hx index d4b31082..2d7064ee 100644 --- a/test/checks/whitespace/TabForAligningCheckTest.hx +++ b/test/checks/whitespace/TabForAligningCheckTest.hx @@ -5,7 +5,7 @@ import checkstyle.checks.whitespace.TabForAligningCheck; class TabForAligningCheckTest extends CheckTestCase { public function testTab() { - assertMsg(new TabForAligningCheck(), TEST1, 'Tab after non-space character, Use space for aligning'); + assertMsg(new TabForAligningCheck(), TEST1, 'Tab after non-space character, use space for aligning'); } public function testMultiline() {