Merge pull request #144 from adireddy/refactor-exclude

ArrayAccessCheck & Refactor exclude

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

resources/checkstyle.json → checkstyle.json

@@ -7,6 +7,12 @@
 			},
 			"type": "Anonymous"
 		},
+		{
+			"props": {
+				"severity": "INFO"
+			},
+			"type": "ArrayAccess"
+		},
 		{
 			"props": {
 				"severity": "INFO"

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"

resources/default-config.json

@@ -9,6 +9,14 @@
 			},
 			"type": "Anonymous"
 		},
+		{
+			"props": {
+				"spaceBefore": false,
+				"spaceInside": false,
+				"severity": "IGNORE"
+			},
+			"type": "ArrayAccess"
+		},
 		{
 			"props": {
 				"severity": "IGNORE"

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<String>):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())
 		};
 	}
 }

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]"
 				};
 			}
 		}

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<String> = 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<String> = 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 {

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) {

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<TokenTree>) {

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;
+		});
+	}
+}

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);
 			}
 		}
 	}

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 {
 

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() {

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) {

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()) {

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<TokenTree>;
 
-	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<TokenDef>, mode:TokenFilterMode, maxLevel:Int = MAX_LEVEL):Array<TokenTree> {
 		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<TokenDef>):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();
 	}
 }