Merge pull request #355 from HaxeCheckstyle/MethodLengthCheck

Added countEmpty option
HaxeCheckstyle · Aug 25, 2017 · ab4e107 · ab4e107
2 parents ab399fa + 295ab80
commit ab4e107
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 20 deletions.
diff --git a/resources/default-config.json b/resources/default-config.json
@@ -231,7 +231,8 @@
 		},
 		{
 			"props": {
-				"max": 50
+				"max": 50,
+				"countEmpty": false
 			},
 			"type": "MethodLength"
 		},

diff --git a/src/checkstyle/checks/size/MethodLengthCheck.hx b/src/checkstyle/checks/size/MethodLengthCheck.hx
@@ -7,10 +7,12 @@ class MethodLengthCheck extends Check {
 	static var DEFAULT_MAX_LENGTH:Int = 50;
 
 	public var max:Int;
+	public var countEmpty:Bool;
 
 	public function new() {
 		super(AST);
 		max = DEFAULT_MAX_LENGTH;
+		countEmpty = false;
 		categories = [Category.COMPLEXITY, Category.CLARITY];
 		points = 8;
 	}
@@ -39,7 +41,8 @@ class MethodLengthCheck extends Check {
 		var lp = checker.getLinePos(f.pos.min);
 		var lmin = lp.line;
 		var lmax = checker.getLinePos(f.pos.max).line;
-		var len = lmax - lmin;
+
+		var len = getLineCount(lmin, lmax);
 		if (len > max) warnFunctionLength(len, f.name, f.pos);
 	}
 
@@ -54,10 +57,20 @@ class MethodLengthCheck extends Check {
 			default: throw "EFunction only";
 		}
 
-		var len = lmax - lmin;
+		var len = getLineCount(lmin, lmax);
 		if (len > max) warnFunctionLength(len, fname, f.pos);
 	}
 
+	function getLineCount(lmin:Int, lmax:Int):Int {
+		var emptyLines = 0;
+		if (countEmpty) {
+			for (i in lmin...lmax) {
+				if (~/^\s*$/.match(checker.lines[i]) || ~/^\s*\/\/.*/.match(checker.lines[i])) emptyLines++;
+			}
+		}
+		return lmax - lmin - emptyLines;
+	}
+
 	function warnFunctionLength(len:Int, name:String, pos:Position) {
 		logPos('Method `${name}` length is ${len} lines (max allowed is ${max})', pos);
 	}

diff --git a/test/checks/size/MethodLengthCheckTest.hx b/test/checks/size/MethodLengthCheckTest.hx
@@ -16,7 +16,18 @@ class MethodLengthCheckTest extends CheckTestCase<MethodLengthCheckTests> {
 		var check = new MethodLengthCheck();
 		check.max = 10;
 
-		assertMsg(check, TEST3, "Method `test` length is 12 lines (max allowed is 10)");
+		assertMsg(check, TEST3, "Method `test` length is 14 lines (max allowed is 10)");
+	}
+
+	public function testIgnoreEmptyLines() {
+		var check = new MethodLengthCheck();
+		check.max = 10;
+		check.countEmpty = true;
+
+		assertNoMsg(check, TEST3);
+
+		check.countEmpty = false;
+		assertMsg(check, TEST3, "Method `test` length is 14 lines (max allowed is 10)");
 	}
 }
 
@@ -25,7 +36,7 @@ abstract MethodLengthCheckTests(String) to String {
 	var TEST1 = "
 	abstractAndClass Test {
 		public function test() {
-			tarce('TEST');\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
+			trace('TEST');\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
 			\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
 			\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
 			\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
@@ -41,43 +52,45 @@ abstract MethodLengthCheckTests(String) to String {
 	var TEST2 =
 	"abstractAndClass Test {
 		public function test() {
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 		}
 	}";
 
 	var TEST3 =
 	"abstractAndClass Test {
 		public function test() {
-			tarce('TEST');
+			trace('TEST');
+
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			// comment
 		}
 
 		@SuppressWarnings('checkstyle:MethodLength')
 		public function test1() {
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 
-			tarce('TEST');
+			trace('TEST');
 		}
 	}";
 }