fix tree position of comments (#429)

* fix tree position of comments

CHANGES.md

@@ -14,6 +14,7 @@
 - Improved support for eval target and Haxe 4 [#423](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/423)
 - Refactored configuration parser [#420](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/420)
 - Refactored check exclusion handling [#424](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/424)
+- Refactored comment placement in token tree [#429](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/429)
 - Cleanup `checkstyle.json` [#427](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/427)
 
 ## version 2.3.1 (2018-05-16)

src/checkstyle/checks/whitespace/ExtendedEmptyLinesCheck.hx

@@ -194,12 +194,12 @@ class ExtendedEmptyLinesCheck extends Check {
 		checkType(emptyLines, typeToken, getPolicy(BEGINABSTRACT), getPolicy(ENDABSTRACT), function(child:TokenTree, next:TokenTree):PolicyAndWhat {
 			var isFuncChild:Bool = child.is(Kwd(KwdFunction));
 			var isVarChild:Bool = child.is(Kwd(KwdVar));
-			var isFuncNext:Bool = next.is(Kwd(KwdFunction));
-			var isVarNext:Bool = next.is(Kwd(KwdVar));
-
-			if (isFuncChild && isFuncNext) return makePolicyAndWhat(getPolicy(BETWEENABSTRACTMETHODS), "between abstract functions");
-			if (isVarChild && isFuncNext) return makePolicyAndWhat(getPolicy(AFTERABSTRACTVARS), "after abstract vars");
-			if (isFuncChild && isVarNext) return makePolicyAndWhat(getPolicy(AFTERABSTRACTVARS), "after abstract vars");
+			if (!isVarChild && !isFuncChild) return null;
+			var type:EmptyLinesFieldType = detectNextFieldType(next);
+			if (type == OTHER) return null;
+			if (isFuncChild && (type == FUNCTION)) return makePolicyAndWhat(getPolicy(BETWEENABSTRACTMETHODS), "between abstract functions");
+			if (isVarChild && (type == FUNCTION)) return makePolicyAndWhat(getPolicy(AFTERABSTRACTVARS), "after abstract vars");
+			if (isFuncChild && (type == VAR)) return makePolicyAndWhat(getPolicy(AFTERABSTRACTVARS), "after abstract vars");
 			return makePolicyAndWhat(getPolicy(BETWEENABSTRACTVARS), "between abstract vars");
 		});
 	}
@@ -220,12 +220,12 @@ class ExtendedEmptyLinesCheck extends Check {
 		checkType(emptyLines, typeToken, getPolicy(BEGINCLASS), getPolicy(ENDCLASS), function(child:TokenTree, next:TokenTree):PolicyAndWhat {
 			var isFuncChild:Bool = child.is(Kwd(KwdFunction));
 			var isVarChild:Bool = child.is(Kwd(KwdVar));
-			var isFuncNext:Bool = next.is(Kwd(KwdFunction));
-			var isVarNext:Bool = next.is(Kwd(KwdVar));
-
-			if (isFuncChild && isFuncNext) return makePolicyAndWhat(getPolicy(BETWEENCLASSMETHODS), "between class methods");
-			if (isVarChild && isFuncNext) return makePolicyAndWhat(getPolicy(AFTERCLASSVARS), "after class vars");
-			if (isFuncChild && isVarNext) return makePolicyAndWhat(getPolicy(AFTERCLASSVARS), "after class vars");
+			if (!isVarChild && !isFuncChild) return null;
+			var type:EmptyLinesFieldType = detectNextFieldType(next);
+			if (type == OTHER) return null;
+			if (isFuncChild && (type == FUNCTION)) return makePolicyAndWhat(getPolicy(BETWEENCLASSMETHODS), "between class methods");
+			if (isVarChild && (type == FUNCTION)) return makePolicyAndWhat(getPolicy(AFTERCLASSVARS), "after class vars");
+			if (isFuncChild && (type == VAR)) return makePolicyAndWhat(getPolicy(AFTERCLASSVARS), "after class vars");
 
 			var isStaticChild:Bool = (child.filter([Kwd(KwdStatic)], FIRST).length > 0);
 			var isStaticNext:Bool = (next.filter([Kwd(KwdStatic)], FIRST).length > 0);
@@ -236,6 +236,24 @@ class ExtendedEmptyLinesCheck extends Check {
 		});
 	}
 
+	function detectNextFieldType(field:TokenTree):EmptyLinesFieldType {
+		if (field.is(Kwd(KwdFunction))) return FUNCTION;
+		if (field.is(Kwd(KwdVar))) return VAR;
+		if (!field.isComment()) return OTHER;
+
+		var after:TokenTree = field.nextSibling;
+		while (after != null) {
+			if (after.is(Kwd(KwdFunction))) return FUNCTION;
+			if (after.is(Kwd(KwdVar))) return VAR;
+			if (after.isComment()) {
+				after = after.nextSibling;
+				continue;
+			}
+			return OTHER;
+		}
+		return OTHER;
+	}
+
 	function checkType(emptyLines:ListOfEmptyLines,
 						typeToken:TokenTree,
 						beginPolicy:EmptyLinesPolicy,
@@ -478,6 +496,12 @@ abstract EmptyLinesPolicy(String) {
 	var ATLEAST = "atleast";
 }
 
+enum EmptyLinesFieldType {
+	VAR;
+	FUNCTION;
+	OTHER;
+}
+
 @:enum
 abstract EmptyLinesPlace(String) {
 	var BEFOREPACKAGE = "beforePackage";

src/checkstyle/token/TokenStream.hx

@@ -26,7 +26,7 @@ class TokenStream {
 		var token:Token = tokens[current];
 		current++;
 		#if debugTokenTree
-		Sys.println (token);
+		Sys.println(token);
 		#end
 		return new TokenTree(token.tok, token.pos, current - 1);
 	}

src/checkstyle/token/TokenTree.hx

@@ -20,6 +20,14 @@ class TokenTree extends Token {
 		return Type.enumEq(tokenDef, tok);
 	}
 
+	public function isComment():Bool {
+		if (tok == null) return false;
+		return switch (tok) {
+			case Comment(_), CommentLine(_): true;
+			default: false;
+		}
+	}
+
 	public function addChild(child:TokenTree) {
 		if (children == null) children = [];
 		if (children.length > 0) {

src/checkstyle/token/walk/WalkClass.hx

@@ -49,16 +49,13 @@ class WalkClass {
 				case Kwd(KwdPublic), Kwd(KwdPrivate), Kwd(KwdStatic), Kwd(KwdInline), Kwd(KwdMacro), Kwd(KwdOverride), Kwd(KwdDynamic):
 					tempStore.push(stream.consumeToken());
 				case Comment(_), CommentLine(_):
-					tempStore.push(stream.consumeToken());
+					parent.addChild(stream.consumeToken());
 				default:
 					throw "invalid token tree structure";
 			}
 		}
-		for (tok in tempStore) {
-			switch (tok.tok) {
-				case Comment(_), CommentLine(_): parent.addChild(tok);
-				default: throw "invalid token tree structure";
-			}
+		if (tempStore.length > 0) {
+			throw "invalid token tree structure";
 		}
 	}
 

src/checkstyle/token/walk/WalkVar.hx

@@ -24,7 +24,6 @@ class WalkVar {
 			if (stream.is(Binop(OpAssign))) {
 				WalkStatement.walkStatement(stream, name);
 			}
-			WalkComment.walkComment(stream, name);
 			if (stream.is(Comma)) {
 				var comma:TokenTree = stream.consumeTokenDef(Comma);
 				name.addChild(comma);

test/checks/whitespace/ExtendedEmptyLinesCheckTest.hx

@@ -280,18 +280,26 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 		static var s2:String = 'xx';
 		var a:Int;
 		var b:String = 'x';
+		// var c:Int = 5;
+		var c:String = 'y';
 		function a() {
 			callA();
 			callB();
 			if (test()) {
 				callC();
 			}
 		}
-		// test comment
 		function b() {
 			callA();
 			callB();
 		}
+		// test comment
+		// test comment
+		function c() {
+			callA();
+			callB();
+		}
+		// test comment
 	}
 	interface ITest {
 		function a();
@@ -307,12 +315,18 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 	@:enum
 	abstract Test(String) {
 		var val1 = 'test';
-		var val2 = 'test';
+		var val2 = 'test2';
+		// var val3 = 'test3';
+		var val4 = 'test4';
 		function test() {
 		}
 		function test2() {
 		}
+		// comment
+		function test3() {
+		}
 	}
+	abstract OneOfTwo<T1, T2>(Dynamic) from T1 from T2 to T1 to T2 {}
 	typedef Any = Dynamic;
 	typedef Struct = {
 		var fieldA:String;
@@ -345,6 +359,10 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 
 		var b:String = 'x';
 
+		// var c:Int = 5;
+
+		var c:String = 'y';
+
 		function a() {
 
 			callA();
@@ -359,16 +377,28 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 
 		}
 
+		function b() {
+
+			callA();
+
+			callB();
+
+		}
+
 		// test comment
 
-		function b() {
+		// test comment
+
+		function c() {
 
 			callA();
 
 			callB();
 
 		}
 
+		// test comment
+
 	}
 
 	interface ITest
@@ -404,6 +434,10 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 
 		var val2 = 'test';
 
+		// var val3 = 'test3';
+
+		var val4 = 'test4';
+
 		function test() {
 
 		}
@@ -412,8 +446,16 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 
 		}
 
+		// comment
+
+		function test3() {
+
+		}
+
 	}
 
+	abstract OneOfTwo<T1, T2>(Dynamic) from T1 from T2 to T1 to T2 {}
+
 	typedef Any = Dynamic;
 
 	typedef Struct =
@@ -467,6 +509,12 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 		var b:String = 'x';
 
 
+		// var c:Int = 5;
+
+
+		var c:String = 'y';
+
+
 		function a() {
 
 
@@ -488,10 +536,25 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 		}
 
 
+		function b() {
+
+
+			callA();
+
+
+			callB();
+
+
+		}
+
+
 		// test comment
 
 
-		function b() {
+		// test comment
+
+
+		function c() {
 
 
 			callA();
@@ -503,6 +566,9 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 		}
 
 
+		// test comment
+
+
 	}
 
 
@@ -554,6 +620,12 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 		var val2 = 'test';
 
 
+		// var val3 = 'test3';
+
+
+		var val4 = 'test4';
+
+
 		function test() {
 
 
@@ -566,9 +638,21 @@ abstract ExtendedEmptyLinesCheckTests(String) to String {
 		}
 
 
+		// comment
+
+
+		function test3() {
+
+
+		}
+
+
 	}
 
 
+	abstract OneOfTwo<T1, T2>(Dynamic) from T1 from T2 to T1 to T2 {}
+
+
 	typedef Any = Dynamic;