Fix positions

CHANGELOG.md

@@ -3,7 +3,7 @@
 ## dev branch / next version (2.x.x)
 
 - **Breaking Change** changed `MethodLength.countEmpty` into `ignoreEmptyLines` ([#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
-- 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 `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) + [#488](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/488))
 - 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))
@@ -12,10 +12,12 @@
 - Added `ignoreEmptyLines` in `FileLengthCheck` to ignore empty lines (default = true) ([#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
 - Added support for final in `DocCommentStyle` and `FieldDocComment` checks ([#487](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/487))
 - Added suggestion to use `final` for `public static var` fields in `Final` check ([#487](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/487))
+- Added ringbuffer for similarity hashes allowing vscode extension (and others) to manage cache ([#488](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/488))
 - 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))
 - Changed reported position for `FieldDocComment` and `MethodLength` to only include function signature ([#487](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/487))
 - Fixed range exclusion to allow excluding construtor (`new`) ([#479](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/479))
+- Fixed reported positions for `FieldDocComment`, `MethodLength`, `ParameterNumber`, `RedundantModifier` and `ReturnCount` checks ([#488](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/488))
 - 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))
 - Refactored / renamed `InlineFinal` to `Final` ([#487](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/487))

checkstyle.json

@@ -631,6 +631,7 @@
 			"checkstyle/checks/metrics/CyclomaticComplexityCheck",
 			"checkstyle/checks/block/LeftCurlyCheck",
 			"checkstyle/checks/block/RightCurlyCheck",
+			"checkstyle/checks/coding/CodeSimilarityCheck",
 			"checkstyle/checks/naming/MethodNameCheck",
 			"checkstyle/checks/type/ReturnCheck",
 			"checkstyle/checks/whitespace/OperatorWrapCheck",

src/checkstyle/checks/coding/CodeSimilarityCheck.hx

@@ -21,6 +21,10 @@ class CodeSimilarityCheck extends Check {
 	static var SIMILAR_HASHES:Map<String, HashedCodeBlock> = new Map<String, HashedCodeBlock>();
 	static var IDENTICAL_HASHES:Map<String, HashedCodeBlock> = new Map<String, HashedCodeBlock>();
 	static var LOCK:Mutex = new Mutex();
+	#if use_similarity_ringbuffer
+	static var FILE_RINGBUFFER:Array<String> = [];
+	static var FILE_HASHES:Map<String, Array<CodeHashes>> = new Map<String, Array<CodeHashes>>();
+	#end
 
 	/**
 		severity level for identical code blocks
@@ -92,6 +96,9 @@ class CodeSimilarityCheck extends Check {
 			startColumn: offsetToColumn(lineStart),
 			endColumn: offsetToColumn(lineEnd)
 		}
+		#if use_similarity_ringbuffer
+		recordFileHashes(hashes, codeBlock);
+		#end
 
 		if (hashes.tokenCount > thresholdIdentical) {
 			var existing:Null<HashedCodeBlock> = checkOrAddHash(hashes.identicalHash, codeBlock, IDENTICAL_HASHES);
@@ -117,11 +124,58 @@ class CodeSimilarityCheck extends Check {
 	function checkOrAddHash(hash:String, codeBlock:HashedCodeBlock, hashTable:Map<String, HashedCodeBlock>):Null<HashedCodeBlock> {
 		LOCK.acquire();
 		var existing:Null<HashedCodeBlock> = hashTable.get(hash);
-		if (existing == null) hashTable.set(hash, codeBlock);
+		if (existing == null) {
+			hashTable.set(hash, codeBlock);
+		}
 		LOCK.release();
 		return existing;
 	}
 
+	#if use_similarity_ringbuffer
+	function recordFileHashes(hashes:CodeHashes, codeBlock:HashedCodeBlock) {
+		LOCK.acquire();
+		if (!FILE_RINGBUFFER.contains(codeBlock.fileName)) FILE_RINGBUFFER.push(codeBlock.fileName);
+		var fileHashes:Null<Array<CodeHashes>> = FILE_HASHES.get(codeBlock.fileName);
+		if (fileHashes == null) {
+			fileHashes = [];
+			FILE_HASHES.set(codeBlock.fileName, fileHashes);
+		}
+		fileHashes.push(hashes);
+		LOCK.release();
+	}
+
+	static function cleanupRingBuffer(maxFileCount:Int) {
+		LOCK.acquire();
+		while (FILE_RINGBUFFER.length > maxFileCount) {
+			var fileName:String = FILE_RINGBUFFER.shift();
+			var fileHashes:Null<Array<CodeHashes>> = FILE_HASHES.get(fileName);
+			if (fileHashes == null) {
+				continue;
+			}
+			for (hash in fileHashes) {
+				SIMILAR_HASHES.remove(hash.similarHash);
+				IDENTICAL_HASHES.remove(hash.identicalHash);
+			}
+			FILE_HASHES.remove(fileName);
+		}
+		LOCK.release();
+	}
+
+	static function cleanupFile(fileName:String) {
+		LOCK.acquire();
+		FILE_RINGBUFFER.remove(fileName);
+		var fileHashes:Null<Array<CodeHashes>> = FILE_HASHES.get(fileName);
+		if (fileHashes != null) {
+			for (hash in fileHashes) {
+				SIMILAR_HASHES.remove(hash.similarHash);
+				IDENTICAL_HASHES.remove(hash.identicalHash);
+			}
+			FILE_HASHES.remove(fileName);
+		}
+		LOCK.release();
+	}
+	#end
+
 	function makeCodeHashes(token:TokenTree):CodeHashes {
 		var similar:StringBuf = new StringBuf();
 		var identical:StringBuf = new StringBuf();
@@ -151,7 +205,8 @@ class CodeSimilarityCheck extends Check {
 		switch (token.tok) {
 			case Const(CFloat(_)):
 				return "const_float";
-			case Const(CString(_)):
+			case Const(CString(s)):
+				if (StringUtils.isStringInterpolation(s, checker.file.content, token.pos)) return "const_string_interpol";
 				return "const_string";
 			case Const(CIdent(_)):
 				return "identifier";

src/checkstyle/checks/coding/ReturnCountCheck.hx

@@ -1,5 +1,7 @@
 package checkstyle.checks.coding;
 
+import checkstyle.utils.PosHelper;
+
 /**
 	Restricts the number of return statements in methods (2 by default). Ignores methods that matches "ignoreFormat" regex property.
 **/
@@ -36,10 +38,10 @@ class ReturnCountCheck extends Check {
 				default:
 			}
 			if (isPosSuppressed(fn.pos)) continue;
-			if (!fn.hasChildren()) throw "function has invalid structure!";
+			if (!fn.hasChildren()) continue;
 			var returns = fn.filterCallback(filterReturns);
 			if (returns.length > max) {
-				logPos('Return count is ${returns.length} (max allowed is ${max})', fn.pos);
+				logPos('Return count is ${returns.length} (max allowed is ${max})', PosHelper.getReportPos(fn));
 			}
 		}
 	}

src/checkstyle/checks/comments/FieldDocCommentCheck.hx

@@ -1,6 +1,7 @@
 package checkstyle.checks.comments;
 
 import checkstyle.checks.comments.TypeDocCommentCheck.TypeDocCommentToken;
+import checkstyle.utils.PosHelper;
 
 /**
 	Checks code documentation on type level
@@ -121,7 +122,7 @@ class FieldDocCommentCheck extends Check {
 		var prevToken:TokenTree = token.previousSibling;
 
 		if (prevToken == null || !prevToken.isComment()) {
-			logPos('Field "$name" should have documentation', getReportPos(token));
+			logPos('Field "$name" should have documentation', PosHelper.getReportPos(token));
 			return;
 		}
 		switch (prevToken.tok) {
@@ -131,29 +132,6 @@ class FieldDocCommentCheck extends Check {
 		}
 	}
 
-	/**
-		report function signature not body
-		@param token function or var token
-		@return Position token position without body
-	**/
-	public static function getReportPos(token:TokenTree):Position {
-		var pos:Position = token.getPos();
-		var body:Null<TokenTree> = token.access().firstChild().firstOf(POpen).token;
-		if (body == null) return pos;
-		body = body.nextSibling;
-		if (body == null) return pos;
-		switch (body.tok) {
-			case BrOpen:
-			case DblDot:
-				body = body.nextSibling;
-			default:
-				return pos;
-		}
-		if (body == null) return pos;
-		pos.max = body.pos.min;
-		return pos;
-	}
-
 	function checkIgnoreOverride(token:TokenTree):Bool {
 		if (!ignoreOverride) return false;
 		var ignoreTokens:Array<TokenTree> = token.filter([Kwd(KwdOverride)], FIRST);

src/checkstyle/checks/modifier/RedundantModifierCheck.hx

@@ -1,5 +1,7 @@
 package checkstyle.checks.modifier;
 
+import checkstyle.utils.PosHelper;
+
 /**
 	Checks for redundant modifiers.
 	Omitting the visibility modifier usually defaults the visibility to "private" in normal classes and "public" in interfaces and externs.
@@ -51,13 +53,13 @@ class RedundantModifierCheck extends Check {
 		var redundantCode:String = isDefaultPrivate ? REDUNDANT_PRIVATE : REDUNDANT_PUBLIC;
 		if (!f.access.contains(APublic) && !f.access.contains(APrivate)) {
 			if ((!isDefaultPrivate && forcePublic) || (isDefaultPrivate && forcePrivate)) {
-				logPos('Missing "$implicitAccess" keyword for "${f.name}"', f.pos, missingCode);
+				logPos('Missing "$implicitAccess" keyword for "${f.name}"', PosHelper.makeFieldSignaturePosition(f), missingCode);
 			}
 		}
 
 		if ((!forcePrivate && isDefaultPrivate && f.access.contains(APrivate))
 			|| (!forcePublic && !isDefaultPrivate && f.access.contains(APublic))) {
-			logPos('"$implicitAccess" keyword is redundant for "${f.name}"', f.pos, redundantCode);
+			logPos('"$implicitAccess" keyword is redundant for "${f.name}"', PosHelper.makeFieldSignaturePosition(f), redundantCode);
 		}
 	}
 

src/checkstyle/checks/size/MethodLengthCheck.hx

@@ -1,7 +1,7 @@
 package checkstyle.checks.size;
 
-import checkstyle.checks.comments.FieldDocCommentCheck;
 import checkstyle.checks.whitespace.ListOfEmptyLines;
+import checkstyle.utils.PosHelper;
 
 /**
 	Checks for long methods. If a method becomes very long it is hard to understand.
@@ -57,7 +57,7 @@ class MethodLengthCheck extends Check {
 				case _:
 			}
 		}
-		if (len > max) warnFunctionLength(len, name, FieldDocCommentCheck.getReportPos(token));
+		if (len > max) warnFunctionLength(len, name, PosHelper.getReportPos(token));
 	}
 
 	function getLineCount(lmin:Int, lmax:Int, emptyLines:ListOfEmptyLines):Int {

src/checkstyle/checks/size/ParameterNumberCheck.hx

@@ -1,5 +1,7 @@
 package checkstyle.checks.size;
 
+import checkstyle.utils.PosHelper;
+
 /**
 	Checks the number of parameters of a method.
 **/
@@ -35,7 +37,7 @@ class ParameterNumberCheck extends Check {
 		switch (f.kind) {
 			case FFun(fun):
 				if ((fun.args != null) && (fun.args.length > max)) {
-					warnMaxParameter(f.name, f.pos);
+					warnMaxParameter(f.name, PosHelper.makeFieldSignaturePosition(f));
 				}
 			default:
 		}

src/checkstyle/utils/PosHelper.hx

@@ -0,0 +1,44 @@
+package checkstyle.utils;
+
+class PosHelper {
+	/**
+		make a position for AST based checks that only holds function signature
+		@param field determine position for field
+		@return Position position that holds function signature
+	**/
+	public static function makeFieldSignaturePosition(field:Field):Position {
+		var pos:Position = {file: field.pos.file, min: field.pos.min, max: field.pos.max};
+		switch (field.kind) {
+			case FFun(fun):
+				if (fun.expr != null) {
+					pos.max = fun.expr.pos.min;
+				}
+			case FVar(_):
+			case FProp(_):
+		}
+		return pos;
+	}
+
+	/**
+		report function signature not body
+		@param token function or var token
+		@return Position token position without body
+	**/
+	public static function getReportPos(token:TokenTree):Position {
+		var pos:Position = token.getPos();
+		var body:Null<TokenTree> = token.access().firstChild().firstOf(POpen).token;
+		if (body == null) return pos;
+		body = body.nextSibling;
+		if (body == null) return pos;
+		switch (body.tok) {
+			case BrOpen:
+			case DblDot:
+				body = body.nextSibling;
+			default:
+				return pos;
+		}
+		if (body == null) return pos;
+		pos.max = body.pos.min;
+		return pos;
+	}
+}