fix parser errors and refactored error handling (#413)

* fix parser errors and refactored error handling

CHANGES.md

@@ -1,5 +1,10 @@
 ## dev branch / next version (2.x.x)
 
+- New command line option `-show-parser-errors` to include parser errors into checkstyle results (default: off) [#413](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/413)
+- Fixed handling of `Arrow` in type names in TokenTree [#413](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/413)
+- Fixed handling of `Dot` after `KwdNew` in TokenTree [#413](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/413)
+- Improved handling of file content and `class` parsing in TokenTree [#413](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/413)
+- Refactored handling of internal errors (parsing and checks) [#413](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/413)
 
 ## version 2.3.0 (2018-05-07)
 
@@ -42,9 +47,9 @@
 
 - Added support for Binop(OpIn) [#352](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/352) ([#359](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/359))
 - Added 1 parser and n checker threads ([#374](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/374))
-  * use `-checkerthreads n` to change number of checker threads (range:1-15 default: 5)
-  * use `-nothreads` to turn off threads and use old behaviour
-  * use `numberOfCheckerThreads` in config file to set number of checker threads (see `resources/default-conmfig.json`)
+  - use `-checkerthreads n` to change number of checker threads (range:1-15 default: 5)
+  - use `-nothreads` to turn off threads and use old behaviour
+  - use `numberOfCheckerThreads` in config file to set number of checker threads (see `resources/default-conmfig.json`)
 - Fixed allow same regex logic for "all" excludes, fixes [#361](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/361) ([#362](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/362))
 - Fixed altering position info in `RightCurlyCheck` ([#367](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/367))
 - Fixed multiple metadatas infront of statement ([#369](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/369))

src/checkstyle/Checker.hx

@@ -1,6 +1,5 @@
 package checkstyle;
 
-import haxe.CallStack;
 import haxeparser.HaxeParser;
 import haxeparser.HaxeLexer;
 import sys.io.File;
@@ -113,13 +112,7 @@ class Checker {
 			}
 		}
 		catch (e:Any) {
-			#if debug
-			Sys.println(e);
-			Sys.println("Stacktrace: " + CallStack.toString(CallStack.exceptionStack()));
-			#end
-			#if unittest
-			throw e;
-			#end
+			ErrorUtils.handleException(e, file, "makeTokens");
 		}
 	}
 
@@ -147,13 +140,7 @@ class Checker {
 			return parser.parse();
 		}
 		catch (e:Any) {
-			#if debug
-			Sys.println(e);
-			Sys.println("Stacktrace: " + CallStack.toString(CallStack.exceptionStack()));
-			#end
-			#if unittest
-			throw e;
-			#end
+			ErrorUtils.handleException(e, file, "makeAST [" + defines.join(",") + "]");
 		}
 		return null;
 	}
@@ -201,7 +188,7 @@ class Checker {
 			getTokenTree();
 		}
 		catch (e:Any) {
-			ReporterManager.INSTANCE.addParseError(file, e);
+			ErrorUtils.handleException(e, file, "createContext");
 			return false;
 		}
 		return true;
@@ -233,7 +220,7 @@ class Checker {
 			return check.run(this);
 		}
 		catch (e:Any) {
-			ReporterManager.INSTANCE.addCheckError(file, e, check.getModuleName());
+			ErrorUtils.handleException(e, file, check.getModuleName());
 			return [];
 		}
 	}

src/checkstyle/CheckerThread.hx

@@ -99,9 +99,9 @@ class CheckerThread {
 			return check.run(checker);
 		}
 		catch (e:Any) {
-			ReporterManager.INSTANCE.addCheckError(checker.file, e, check.getModuleName());
-			return [];
+			ErrorUtils.handleException(e, checker.file, check.getModuleName());
 		}
+		return [];
 	}
 
 	function checkForExclude(moduleName:String, checker:Checker):Bool {

src/checkstyle/Main.hx

@@ -82,11 +82,13 @@ class Main {
 			@doc("List all available checks and exit") ["--list-checks"] => function() listChecks(),
 			@doc("List all available reporters and exit") ["--list-reporters"] => function() listReporters(),
 			@doc("Generate a default config and exit") ["--default-config"] => function(path) generateDefaultConfig(path),
-			@doc("To omit styling in output summary") ["-nostyle"] => function() NO_STYLE = true,
+			@doc("Omit styling in output summary") ["-nostyle"] => function() NO_STYLE = true,
 			@doc("Show checks missing from active config") ["-show-missing-checks"] => function () SHOW_MISSING_CHECKS = true,
 			@doc("Sets the number of checker threads") ["-checkerthreads"] => function (num:Int) overrideCheckerThreads = num,
 			@doc("Do not use checker threads") ["-nothreads"] => function () disableThreads = true,
 			@doc("Try to detect your coding style (experimental)") ["-detect"] => function (path) detectCodingStyle(path),
+			@doc("Adds error messages for files that checkstyle fails to parse") ["-show-parser-errors"] => function ()
+				ReporterManager.SHOW_PARSE_ERRORS = true,
 			@doc("Show report [DEPRECATED]") ["-report"] => function() Sys.println("\n-report is no longer needed."),
 			_ => function(arg:String) failWith("Unknown command: " + arg)
 		]);
@@ -117,15 +119,12 @@ class Main {
 		if (configPath == null && FileSystem.exists(DEFAULT_CONFIG) && !FileSystem.isDirectory(DEFAULT_CONFIG)) {
 			configPath = DEFAULT_CONFIG;
 		}
-
 		if (excludePath == null && FileSystem.exists(DEFAULT_EXCLUDE_CONFIG) && !FileSystem.isDirectory(DEFAULT_EXCLUDE_CONFIG)) {
 			excludePath = DEFAULT_EXCLUDE_CONFIG;
 		}
 
 		loadConfig(configPath);
-
 		if (excludePath != null) loadExcludeConfig(excludePath);
-
 		start();
 	}
 

src/checkstyle/checks/Check.hx

@@ -1,9 +1,5 @@
 package checkstyle.checks;
 
-#if debug
-import haxe.CallStack;
-#end
-
 class Check {
 
 	public var severity:SeverityLevel;
@@ -41,13 +37,7 @@ class Check {
 				actualRun();
 			}
 			catch (e:String) {
-				#if debug
-				Sys.println(e);
-				Sys.println("Stacktrace: " + CallStack.toString(CallStack.exceptionStack()));
-				#end
-				#if unittest
-				throw e;
-				#end
+				ErrorUtils.handleException(e, checker.file, getModuleName());
 			}
 		}
 		return messages;

src/checkstyle/checks/block/EmptyBlockCheck.hx

@@ -1,5 +1,7 @@
 package checkstyle.checks.block;
 
+import checkstyle.token.TokenTreeAccessHelper;
+
 @name("EmptyBlock")
 @desc("Checks for empty blocks. The policy to verify is specified using the property `option`.")
 class EmptyBlockCheck extends Check {
@@ -104,8 +106,9 @@ class EmptyBlockCheck extends Check {
 	}
 
 	function checkForEmpty(brOpen:TokenTree) {
-		if (brOpen.children.length > 1) return;
-		var brClose:TokenTree = brOpen.children[0];
+		if ((brOpen.children == null) || (brOpen.children.length > 1)) return;
+		var brClose:TokenTree = TokenTreeAccessHelper.access(brOpen).firstChild().is(BrClose).token;
+		if (brClose == null) return;
 		if (brOpen.pos.max != brClose.pos.min) logPos('Empty block should be written as "{}"', brOpen.pos);
 	}
 }

src/checkstyle/checks/block/RightCurlyCheck.hx

@@ -38,6 +38,7 @@ class RightCurlyCheck extends Check {
 		for (brClose in allBrClose) {
 			if (isPosSuppressed(brClose.pos)) continue;
 			var brOpen:TokenTree = brClose.parent;
+			if ((brOpen == null) || (brOpen.pos == null)) continue;
 			if (filterParentToken(brOpen.parent)) continue;
 			check(brClose, isSingleLine(brOpen.pos.min, brClose.pos.max));
 		}

src/checkstyle/checks/coding/ReturnCountCheck.hx

@@ -20,6 +20,7 @@ class ReturnCountCheck extends Check {
 		var root:TokenTree = checker.getTokenTree();
 		var functions = root.filter([Kwd(KwdFunction)], ALL);
 		for (fn in functions) {
+			if (fn.children == null) continue;
 			switch (fn.getFirstChild().tok) {
 				case Const(CIdent(name)):
 					if (ignoreFormatRE.match(name)) continue;

src/checkstyle/checks/whitespace/IndentationCheck.hx

@@ -117,12 +117,7 @@ class IndentationCheck extends Check {
 		for (token in tokenList) {
 			switch (token.tok) {
 				case BkOpen:
-					var child:TokenTree = token.getFirstChild();
-					if (child.is(BrOpen)) {
-						// only indent once, if directly next to each other `[{`
-						if (token.pos.min + 1 == child.pos.min) continue;
-					}
-					increaseBlockIndent(token, lineIndentation);
+					calcLineIndentationBkOpen(token, lineIndentation);
 				case BrOpen:
 					increaseBlockIndent(token, lineIndentation);
 				case Kwd(KwdIf), Kwd(KwdElse):
@@ -131,6 +126,7 @@ class IndentationCheck extends Check {
 					calcLineIndentationLoops(token, lineIndentation);
 				case Kwd(KwdCase):
 					var child:TokenTree = token.getLastChild();
+					if (child == null) continue;
 					increaseRangeIndent(child.getPos(), lineIndentation);
 				case Kwd(KwdDefault):
 					var child:TokenTree = token.getLastChild();
@@ -145,17 +141,29 @@ class IndentationCheck extends Check {
 		return lineIndentation;
 	}
 
+	function calcLineIndentationBkOpen(token:TokenTree, lineIndentation:Array<Int>) {
+		var child:TokenTree = token.getFirstChild();
+		if (child == null) return;
+		if (child.is(BrOpen)) {
+			// only indent once, if directly next to each other `[{`
+			if (token.pos.min + 1 == child.pos.min) return;
+		}
+		increaseBlockIndent(token, lineIndentation);
+	}
+
 	function calcLineIndentationIf(token:TokenTree, lineIndentation:Array<Int>) {
 		switch (token.tok) {
 			case Kwd(KwdIf):
 				var child:TokenTree = token.getLastChild();
+				if (child == null) return;
 				if (child.is(Kwd(KwdElse))) {
 					child = token.children[token.children.length - 2];
 				}
 				if (child.is(BrOpen)) return;
 				increaseIndentIfNextLine(token, child, lineIndentation);
 			case Kwd(KwdElse):
 				var child:TokenTree = token.getFirstChild();
+				if (child == null) return;
 				if (child.is(BrOpen)) return;
 				increaseIndentIfNextLine(token, child, lineIndentation);
 			default:
@@ -199,14 +207,17 @@ class IndentationCheck extends Check {
 		switch (token.tok) {
 			case Kwd(KwdFor):
 				var child:TokenTree = token.getLastChild();
+				if (child == null) return;
 				if (child.is(BrOpen)) return;
 				increaseIndentIfNextLine(token, child, lineIndentation);
 			case Kwd(KwdDo):
 				var child:TokenTree = token.getFirstChild();
+				if (child == null) return;
 				if (child.is(BrOpen)) return;
 				increaseIndentIfNextLine(token, child, lineIndentation);
 			case Kwd(KwdWhile):
 				var child:TokenTree = token.getLastChild();
+				if (child == null) return;
 				if (child.is(BrOpen)) return;
 				increaseIndentIfNextLine(token, child, lineIndentation);
 			default:
@@ -239,6 +250,7 @@ class IndentationCheck extends Check {
 		for (token in tokenList) {
 			var pos = token.getPos();
 			var child:TokenTree = token.getFirstChild();
+			if (child == null) continue;
 			if (token.is(Dot)) pos = token.parent.getPos();
 			if (child.is(BkOpen)) continue;
 			ignoreRange(pos, wrapped);
@@ -280,6 +292,7 @@ class IndentationCheck extends Check {
 	}
 
 	function increaseIndentBetween(blockStart:TokenTree, blockEnd:TokenTree, lineIndentation:Array<Int>) {
+		if (blockEnd == null) return;
 		var start:Int = checker.getLinePos(blockStart.pos.min).line + 1;
 		var end:Int = checker.getLinePos(blockEnd.pos.min).line;
 		increaseIndent(lineIndentation, start, end);
@@ -292,6 +305,7 @@ class IndentationCheck extends Check {
 	}
 
 	function increaseIndentIfNextLine(parent:TokenTree, child:TokenTree, lineIndentation:Array<Int>) {
+		if (child == null) return;
 		var parentLine:Int = checker.getLinePos(parent.pos.min).line;
 		var childLine:Int = checker.getLinePos(child.pos.min).line;
 		if (parentLine == childLine) return;

src/checkstyle/import.hx

@@ -10,6 +10,7 @@ import checkstyle.SeverityLevel;
 import checkstyle.token.TokenTree;
 import checkstyle.detect.DetectableInstances;
 
+import checkstyle.utils.ErrorUtils;
 using checkstyle.utils.ArrayUtils;
 using checkstyle.utils.FieldUtils;
 using checkstyle.utils.ExprUtils;

src/checkstyle/reporter/ReporterManager.hx

@@ -6,19 +6,21 @@ import neko.vm.Mutex;
 import cpp.vm.Mutex;
 #end
 
-import haxe.CallStack;
-
 import checkstyle.CheckMessage;
 
 import checkstyle.checks.Category;
 
 class ReporterManager {
 	public static var INSTANCE:ReporterManager = new ReporterManager();
+	public static var SHOW_PARSE_ERRORS:Bool = false;
 
 	var reporters:Array<IReporter>;
 	var lock:Mutex;
 
 	function new () {
+		#if (debug || unittest)
+		SHOW_PARSE_ERRORS = true;
+		#end
 		clear();
 		lock = new Mutex();
 	}
@@ -51,16 +53,8 @@ class ReporterManager {
 		lock.release();
 	}
 
-	public function addParseError(f:CheckFile, e:Any) {
-		lock.acquire();
-		for (reporter in reporters) {
-			reporter.addMessage(getErrorMessage(e, f.name, "Parsing"));
-			reporter.fileFinish(f);
-		}
-		lock.release();
-	}
-
-	public function addCheckError(f:CheckFile, e:Any, name:String) {
+	public function addError(f:CheckFile, e:Any, name:String) {
+		if (!SHOW_PARSE_ERRORS) return;
 		lock.acquire();
 		for (reporter in reporters) reporter.addMessage(getErrorMessage(e, f.name, "Check " + name));
 		lock.release();
@@ -110,8 +104,8 @@ class ReporterManager {
 			moduleName:"Checker",
 			categories:[Category.STYLE],
 			points:1,
-			desc: "",
-			message:step + " failed: " + e + "\nStacktrace: " + CallStack.toString(CallStack.exceptionStack())
+			desc:"",
+			message:step + " failed: " + e + "\nPlease file a github issue at https://github.com/HaxeCheckstyle/haxe-checkstyle/issues"
 		};
 	}
 }

src/checkstyle/token/TokenTreeAccessHelper.hx

@@ -0,0 +1,56 @@
+package checkstyle.token;
+
+class TokenTreeAccessHelper {
+
+	public var token:TokenTree;
+
+	function new(tok:TokenTree) {
+		token = tok;
+	}
+
+	public static function access(tok:TokenTree):TokenTreeAccessHelper {
+		return new TokenTreeAccessHelper(tok);
+	}
+
+	public function firstChild():TokenTreeAccessHelper {
+		if (token == null) return this;
+		return new TokenTreeAccessHelper(token.getFirstChild());
+	}
+
+	public function lastChild():TokenTreeAccessHelper {
+		if (token == null) return this;
+		return new TokenTreeAccessHelper(token.getLastChild());
+	}
+
+	public function firstOf(tokenDef:TokenDef):TokenTreeAccessHelper {
+		if (token == null) return this;
+		if (token.children == null) return new TokenTreeAccessHelper(null);
+		for (tok in token.children) {
+			if (tok.is(tokenDef)) return new TokenTreeAccessHelper(tok);
+		}
+		return new TokenTreeAccessHelper(null);
+	}
+
+	public function lastOf(tokenDef:TokenDef):TokenTreeAccessHelper {
+		if (token == null) return this;
+		if (token.children == null) return new TokenTreeAccessHelper(null);
+		var found:TokenTree = null;
+		for (tok in token.children) {
+			if (tok.is(tokenDef)) found = tok;
+		}
+		return new TokenTreeAccessHelper(found);
+	}
+
+	public function child(index:Int):TokenTreeAccessHelper {
+		if (token == null) return this;
+		if (token.children == null) return new TokenTreeAccessHelper(null);
+		if (token.children.length <= index) return new TokenTreeAccessHelper(null);
+		return new TokenTreeAccessHelper(token.children[index]);
+	}
+
+	public function is(tokenDef:TokenDef):TokenTreeAccessHelper {
+		if (token == null) return this;
+		if (token.is(tokenDef)) return this;
+		return new TokenTreeAccessHelper(null);
+	}
+}