fixed BrOpen handling of NeedBraces check (#446)

* fixed BrOpen handling of NeedBraces check

Author: AlexHaxe

CHANGELOG.md

@@ -11,6 +11,7 @@
 - Fixed indentation calculation for functions bodys without curly braces [#443](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/443)
 - Fixed segmentaion faults in NeedBraces and CatchParameterName checks [#443](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/443)
 - Fixed reported position of EmptyBlock check [#444](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/444)
+- Fixed BrOpen detection in NeedBraces check [#446](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/446)
 - Changed message of NestedForLoop check [#443](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/443)
 - Changed autodetection for nested for/if/try checks to start at zero [#444](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/444)
 - Refactored `Checker.getLinePos` to use binary search, reduces runtime from O(N/2) to O(log N) [#439](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/439)

src/checkstyle/checks/block/NeedBracesCheck.hx

@@ -67,7 +67,7 @@ class NeedBracesCheck extends Check {
 				case Kwd(KwdFunction):
 					checkFunctionChild(tok);
 				case Kwd(KwdDo):
-					checkDoWhileChild(tok);
+					checkLastChild(tok);
 				case Kwd(KwdWhile):
 					checkWhileChild(tok);
 				default:
@@ -94,57 +94,35 @@ class NeedBracesCheck extends Check {
 	function checkFunctionChild(token:TokenTree) {
 		if (token == null || !token.hasChildren()) return;
 
-		var lastChild:TokenTree = token.getLastChild();
-		switch (lastChild.tok) {
-			case Const(CIdent(_)), Kwd(KwdNew):
-				if (!lastChild.hasChildren()) return;
-				lastChild = lastChild.getLastChild();
-			default:
-		}
-		switch (lastChild.tok) {
-			case BrOpen:
-				return;
-			case Semicolon:
-				return;
-			default:
-				checkNoBraces(token, lastChild);
+		var accessHelper:TokenTreeAccessHelper;
+		if (token.children.length == 1) {
+			accessHelper = TokenTreeAccessHelper.access(token).firstChild();
 		}
-	}
-
-	function checkDoWhileChild(token:TokenTree) {
-		if (token == null || !token.hasChildren()) return;
-
-		var lastChild:TokenTree = token.getLastChild();
-		var expr:TokenTree = lastChild.previousSibling;
-		switch (expr.tok) {
-			case BrOpen:
-				return;
-			default:
-				checkNoBraces(token, lastChild);
+		else {
+			accessHelper = TokenTreeAccessHelper.access(token);
 		}
+		// function bodies
+		var lastChild:TokenTree = accessHelper.firstOf(BrOpen).token;
+		if (lastChild != null) return;
+		// interfaces
+		lastChild = accessHelper.firstOf(Semicolon).token;
+		if (lastChild != null) return;
+		lastChild = accessHelper.lastChild().token;
+
+		checkNoBraces(token, lastChild);
 	}
 
 	function checkWhileChild(token:TokenTree) {
 		if (token == null || !token.hasChildren() || Type.enumEq(token.parent.tok, Kwd(KwdDo))) return;
-		var lastChild:TokenTree = token.getLastChild();
-		switch (lastChild.tok) {
-			case BrOpen:
-				return;
-			default:
-				checkNoBraces(token, lastChild);
-		}
+		checkLastChild(token);
 	}
 
 	function checkLastChild(token:TokenTree) {
 		if (token == null || !token.hasChildren()) return;
-
-		var lastChild:TokenTree = token.getLastChild();
-		switch (lastChild.tok) {
-			case BrOpen:
-				return;
-			default:
-				checkNoBraces(token, lastChild);
-		}
+		var lastChild:TokenTree = TokenTreeAccessHelper.access(token).firstOf(BrOpen).token;
+		if (lastChild != null) return;
+		lastChild = TokenTreeAccessHelper.access(token).lastChild().token;
+		checkNoBraces(token, lastChild);
 	}
 
 	function checkNoBraces(parent:TokenTree, child:TokenTree) {

test/checks/block/NeedBracesCheckTest.hx

@@ -15,6 +15,9 @@ class NeedBracesCheckTest extends CheckTestCase<NeedBracesCheckTests> {
 	static inline var MSG_SAME_LINE_FOR:String = 'Body of "for" on same line';
 	static inline var MSG_SAME_LINE_WHILE:String = 'Body of "while" on same line';
 
+	static inline var MSG_SAME_LINE_FUNCTION:String = 'Body of "function" on same line';
+	static inline var MSG_SAME_LINE_DO_WHILE:String = 'Body of "do" on same line';
+
 	@Test
 	public function testCorrectBraces() {
 		var check = new NeedBracesCheck();
@@ -28,6 +31,8 @@ class NeedBracesCheckTest extends CheckTestCase<NeedBracesCheckTests> {
 		assertNoMsg(check, TEST13);
 		assertNoMsg(check, TEST14);
 		assertNoMsg(check, INTERFACE_DEF);
+		assertNoMsg(check, ANON_FUNCTION);
+		assertNoMsg(check, ANON_FUNCTION_NO_BRACES);
 	}
 
 	@Test
@@ -177,6 +182,62 @@ class NeedBracesCheckTest extends CheckTestCase<NeedBracesCheckTests> {
 		check.allowSingleLineStatement = false;
 		assertMsg(check, TEST, MSG_SAME_LINE_WHILE);
 	}
+
+	@Test
+	public function testTokenFunction() {
+		var check = new NeedBracesCheck();
+		check.tokens = [FUNCTION];
+
+		assertNoMsg(check, TEST);
+		assertNoMsg(check, TEST1);
+		assertNoMsg(check, TEST2);
+		assertNoMsg(check, TEST3);
+		assertNoMsg(check, TEST4);
+		assertNoMsg(check, TEST5);
+		assertNoMsg(check, TEST6);
+		assertNoMsg(check, TEST7);
+		assertNoMsg(check, TEST8);
+		assertNoMsg(check, TEST9);
+		assertNoMsg(check, TEST10);
+		assertNoMsg(check, TEST11);
+		assertNoMsg(check, TEST12);
+		assertNoMsg(check, TEST13);
+		assertNoMsg(check, TEST14);
+		assertNoMsg(check, ANON_FUNCTION);
+		assertNoMsg(check, ANON_FUNCTION_NO_BRACES);
+
+		check.allowSingleLineStatement = false;
+		assertNoMsg(check, ANON_FUNCTION);
+		assertMsg(check, ANON_FUNCTION_NO_BRACES, MSG_SAME_LINE_FUNCTION);
+	}
+
+	@Test
+	public function testTokenDoWhile() {
+		var check = new NeedBracesCheck();
+		check.tokens = [DO_WHILE];
+
+		assertNoMsg(check, TEST);
+		assertNoMsg(check, TEST1);
+		assertNoMsg(check, TEST2);
+		assertNoMsg(check, TEST3);
+		assertNoMsg(check, TEST4);
+		assertNoMsg(check, TEST5);
+		assertNoMsg(check, TEST6);
+		assertNoMsg(check, TEST7);
+		assertNoMsg(check, TEST8);
+		assertNoMsg(check, TEST9);
+		assertNoMsg(check, TEST10);
+		assertNoMsg(check, TEST11);
+		assertNoMsg(check, TEST12);
+		assertNoMsg(check, TEST13);
+		assertNoMsg(check, TEST14);
+		assertNoMsg(check, DO_WHILE);
+		assertNoMsg(check, DO_WHILE_NO_BRACES);
+
+		check.allowSingleLineStatement = false;
+		assertNoMsg(check, DO_WHILE);
+		assertMsg(check, DO_WHILE_NO_BRACES, MSG_SAME_LINE_DO_WHILE);
+	}
 }
 
 @:enum
@@ -346,12 +407,12 @@ abstract NeedBracesCheckTests(String) to String {
 	var TEST15 = "
 	class Test {
 		public function test(a:Bool, b:Bool) {
-		   if (a) {
+			if (a) {
 
-		   }
-		   else if (!b) {
+			}
+			else if (!b) {
 
-		   }
+			}
 		}
 	}";
 
@@ -369,4 +430,37 @@ abstract NeedBracesCheckTests(String) to String {
 	interface Test {
 		function test();
 	}";
+
+	var ANON_FUNCTION = "
+	abstractAndClass Test {
+		function test() {
+			doSomething(function() {
+				doIt();
+			}, false);
+		}
+	}";
+
+	var ANON_FUNCTION_NO_BRACES = "
+	abstractAndClass Test {
+		function test() {
+			doSomething(function() doIt(), false);
+		}
+	}";
+
+	var DO_WHILE = "
+	class Test {
+		function test() {
+			do {
+				return i;
+			}
+			while (true);
+		}
+	}";
+
+	var DO_WHILE_NO_BRACES = "
+	class Test {
+		function test() {
+			do return i while (true);
+		}
+	}";
 }