added ignoreEmptyLines to FileLength and MethodLength checks (#486)

* added ignoreEmptyLines to FileLength and MethodLength checks
* *breaking* removed countEmpty from MethodLength (use ignoreEmptyLines)
* using token count for threshold in CodeSimilarity

Author: AlexHaxe

.travis.yml

@@ -41,4 +41,4 @@ after_script:
   - if [[ "$HAXE_VERSION" == "haxe4" ]]; then (cd src; ../cc-test-reporter upload-coverage); fi
 
 after_success:
-  - bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"
+  - if [[ "$HAXE_VERSION" == "haxe4" ]]; bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"

CHANGELOG.md

@@ -2,12 +2,16 @@
 
 ## dev branch / next version (2.x.x)
 
-- New check `CodeSimilarity` to check for similar or identical code blocks ([#479](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/479) + [#480](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/480) + [#484](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/484))
+- **Breaking Change** changed `MethodLength.countEmpty` into `ignoreEmptyLines`
+- New check `CodeSimilarity` to check for similar or identical code blocks ([#479](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/479) + [#480](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/480) + [#484](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/484) + [#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
 - New check `EnforceVarTypeHint` to enforce type hints for all variables and finals, fixes [#464](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/464) ([#481](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/481) + [#482](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/482))
 - New check `AvoidIdentifier` marks identifiers to avoid ([#483](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/483))
 - New check `ArrowFunction` to check for curlies, nested functions and returns in arrow functions ([#484](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/484))
 - New check `NestedControlFlow` to check for nested control flow expressions (e.g. `if`, `for`, `while`, `do/while`, `switch` and `try`) ([#485](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/485))
 - Added coverage upload to codeclimate ([#478](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/478))
+- Added `ignoreEmptyLines` in FileLengthCheck to ignore empty lines (default = true) ([#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
+- Changed default value for `max` in `FileLengthCheck` to 1000 ([#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
+- Changed `MethodLength` check to use tokentree ([#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
 - Fixed allow excluding construtor (`new`) via range exclusion ([#479](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/479))
 - Refactored build system to use lix ([#478](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/478))
 - Refactored / renamed AvoidInlineConditionals to AvoidTernaryOperator ([#479](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/479))

checkstyle.json

@@ -134,9 +134,6 @@
 			}
 		},
 		{
-			"props": {
-				"max": 2000
-			},
 			"type": "FileLength"
 		},
 		{

resources/checkstyle-schema.json

@@ -910,6 +910,11 @@
                                 "ERROR",
                                 "IGNORE"
                             ],
+                            "propertyOrder": 2
+                        },
+                        "ignoreEmptyLines": {
+                            "description": "ignores or includes empty lines when counting total file length",
+                            "type": "boolean",
                             "propertyOrder": 1
                         }
                     },
@@ -3513,12 +3518,12 @@
                     "additionalProperties": false,
                     "properties": {
                         "thresholdSimilar": {
-                            "description": "threshold for similar code blocks",
+                            "description": "maximum number of tokens allowed before detecting similar code blocks",
                             "type": "integer",
                             "propertyOrder": 2
                         },
                         "thresholdIdentical": {
-                            "description": "threshold for identical code blocks",
+                            "description": "maximum number of tokens allowed before detecting identical code blocks",
                             "type": "integer",
                             "propertyOrder": 1
                         },
@@ -3927,11 +3932,6 @@
                     "description": "Checks for long methods. If a method becomes very long it is hard to understand. Therefore long methods should usually be refactored into several individual methods that focus on a specific task.",
                     "additionalProperties": false,
                     "properties": {
-                        "countEmpty": {
-                            "description": "maximum includes empty lines / should ignore empty lines",
-                            "type": "boolean",
-                            "propertyOrder": 1
-                        },
                         "max": {
                             "description": "maximum number of lines per method (default: 50)",
                             "type": "integer",
@@ -3947,6 +3947,11 @@
                                 "IGNORE"
                             ],
                             "propertyOrder": 2
+                        },
+                        "ignoreEmptyLines": {
+                            "description": "ignores or includes empty lines when counting method length",
+                            "type": "boolean",
+                            "propertyOrder": 1
                         }
                     },
                     "type": "object"

resources/default-config.json

@@ -57,8 +57,8 @@
 			"type": "CodeSimilarity",
 			"props": {
 				"severityIdentical": "WARNING",
-				"thresholdIdentical": 8,
-				"thresholdSimilar": 12
+				"thresholdIdentical": 60,
+				"thresholdSimilar": 120
 			}
 		},
 		{
@@ -192,7 +192,8 @@
 		{
 			"type": "FileLength",
 			"props": {
-				"max": 2000
+				"max": 1000,
+				"ignoreEmptyLines": true
 			}
 		},
 		{
@@ -323,7 +324,7 @@
 			"type": "MethodLength",
 			"props": {
 				"max": 50,
-				"countEmpty": false
+				"ignoreEmptyLines": true
 			}
 		},
 		{

src/checkstyle/checks/coding/CodeSimilarityCheck.hx

@@ -28,20 +28,20 @@ class CodeSimilarityCheck extends Check {
 	public var severityIdentical:SeverityLevel;
 
 	/**
-		threshold for identical code blocks
+		maximum number of tokens allowed before detecting identical code blocks
 	**/
 	public var thresholdIdentical:Int;
 
 	/**
-		threshold for similar code blocks
+		maximum number of tokens allowed before detecting similar code blocks
 	**/
 	public var thresholdSimilar:Int;
 
 	public function new() {
 		super(TOKEN);
 		severityIdentical = WARNING;
-		thresholdIdentical = 8;
-		thresholdSimilar = 12;
+		thresholdIdentical = 60;
+		thresholdSimilar = 120;
 		categories = [STYLE, DUPLICATION];
 	}
 
@@ -82,10 +82,9 @@ class CodeSimilarityCheck extends Check {
 
 		var lineStart:LinePos = checker.getLinePos(pos.min);
 		var lineEnd:LinePos = checker.getLinePos(pos.max);
-		var lines:Int = lineEnd.line - lineStart.line;
-		if (lines <= Math.min(thresholdIdentical, thresholdSimilar)) return false;
 
 		var hashes:CodeHashes = makeCodeHashes(token);
+		if (hashes.tokenCount <= Math.min(thresholdIdentical, thresholdSimilar)) return false;
 		var codeBlock:HashedCodeBlock = {
 			fileName: token.pos.file,
 			lineStart: lineStart,
@@ -94,15 +93,15 @@ class CodeSimilarityCheck extends Check {
 			endColumn: offsetToColumn(lineEnd)
 		}
 
-		if (lines > thresholdIdentical) {
+		if (hashes.tokenCount > thresholdIdentical) {
 			var existing:Null<HashedCodeBlock> = checkOrAddHash(hashes.identicalHash, codeBlock, IDENTICAL_HASHES);
 			if (existing != null) {
-				logRange("Found identical code block - " + formatFirstFound(existing), pos.min, pos.max, SIMILAR_BLOCK, ERROR);
+				logRange("Found identical code block - " + formatFirstFound(existing), pos.min, pos.max, SIMILAR_BLOCK, severityIdentical);
 				return true;
 			}
 		}
 
-		if (lines > thresholdSimilar) {
+		if (hashes.tokenCount > thresholdSimilar) {
 			var existing:Null<HashedCodeBlock> = checkOrAddHash(hashes.similarHash, codeBlock, SIMILAR_HASHES);
 			if (existing == null) return false;
 			logRange("Found similar code block - " + formatFirstFound(existing), pos.min, pos.max, SIMILAR_BLOCK);
@@ -126,19 +125,26 @@ class CodeSimilarityCheck extends Check {
 	function makeCodeHashes(token:TokenTree):CodeHashes {
 		var similar:StringBuf = new StringBuf();
 		var identical:StringBuf = new StringBuf();
-		makeCodeHashesRecursive(token, similar, identical);
+		var tokenCount:Int = makeCodeHashesRecursive(token, similar, identical);
 		return {
 			identicalHash: identical.toString(),
-			similarHash: similar.toString()
+			similarHash: similar.toString(),
+			tokenCount: tokenCount
 		};
 	}
 
-	function makeCodeHashesRecursive(token:TokenTree, similar:StringBuf, identical:StringBuf) {
+	function makeCodeHashesRecursive(token:TokenTree, similar:StringBuf, identical:StringBuf):Int {
 		similar.add(similarTokenText(token));
-		identical.add(identicalTokenText(token));
+		var count:Int = 0;
+		var identicalText:Null<String> = identicalTokenText(token);
+		if (identicalText != null) {
+			count++;
+			identical.add(identicalText);
+		}
 		if (token.children != null) {
-			for (child in token.children) makeCodeHashesRecursive(child, similar, identical);
+			for (child in token.children) count += makeCodeHashesRecursive(child, similar, identical);
 		}
+		return count;
 	}
 
 	function similarTokenText(token:TokenTree):String {
@@ -184,7 +190,7 @@ class CodeSimilarityCheck extends Check {
 		}
 	}
 
-	function identicalTokenText(token:TokenTree):String {
+	function identicalTokenText(token:TokenTree):Null<String> {
 		switch (token.tok) {
 			case Const(CFloat(f)):
 				return '$f';
@@ -203,9 +209,9 @@ class CodeSimilarityCheck extends Check {
 			case Binop(op):
 				return '$op';
 			case Comment(_):
-				return "";
+				return null;
 			case CommentLine(_):
-				return "";
+				return null;
 			case IntInterval(i):
 				return '...$i';
 			default:
@@ -241,4 +247,5 @@ typedef HashedCodeBlock = {
 typedef CodeHashes = {
 	var identicalHash:String;
 	var similarHash:String;
+	var tokenCount:Int;
 }

src/checkstyle/checks/size/FileLengthCheck.hx

@@ -1,36 +1,51 @@
 package checkstyle.checks.size;
 
+import checkstyle.checks.whitespace.ListOfEmptyLines;
+
 /**
 	Checks for long source files. If a source file becomes very long it is hard to understand.
 	Therefore long classes should usually be refactored into several individual classes that focus on a specific task.
 **/
 @name("FileLength")
 @desc("Checks for long source files. If a source file becomes very long it is hard to understand. Therefore long classes should usually be refactored into several individual classes that focus on a specific task.")
 class FileLengthCheck extends Check {
-	static var DEFAULT_MAX_LENGTH:Int = 2000;
+	static var DEFAULT_MAX_LENGTH:Int = 1000;
 
 	/**
 		maximum number of lines permitted per file (default: 2000)
 	**/
 	public var max:Int;
 
+	/**
+		ignores or includes empty lines when counting total file length
+	**/
+	public var ignoreEmptyLines:Bool;
+
 	public function new() {
 		super(LINE);
 		max = DEFAULT_MAX_LENGTH;
+		ignoreEmptyLines = true;
 		categories = [Category.COMPLEXITY, Category.CLARITY];
 		points = 21;
 	}
 
 	override function actualRun() {
 		if (checker.ast == null) return;
+
 		for (td in checker.ast.decls) {
 			switch (td.decl) {
 				case EClass(d):
 					for (field in d.data) if (isCheckSuppressed(field)) return;
 				default:
 			}
 		}
-		if (checker.lines.length > max) {
+
+		var count = checker.lines.length;
+		if (ignoreEmptyLines) {
+			var emptyLines:ListOfEmptyLines = ListOfEmptyLines.detectEmptyLines(checker);
+			count -= emptyLines.lines.length;
+		}
+		if (count > max) {
 			log('File length is ${checker.lines.length} lines (max allowed is ${max})', checker.lines.length, 0, checker.lines.length, 0);
 		}
 	}
@@ -40,7 +55,7 @@ class FileLengthCheck extends Check {
 			fixed: [],
 			properties: [{
 				propertyName: "max",
-				values: [for (i in 0...10) 400 + i * 200]
+				values: [for (i in 0...10) 400 + i * 100]
 			}]
 		}];
 	}