fixed MagicNumber check to support numeric separators and suffixes (#515)

AlexHaxe · web-flow · commit 119223c2a517 · 2022-09-21T22:42:51.000+02:00
* fixed MagicNumber check to support numeric separators and suffixes
* excluded numeric separator and suffixes test from 4.2.5 runs
* switched coverage reporting to nightly runs
diff --git a/.github/workflows/checkstyle-linux.yml b/.github/workflows/checkstyle-linux.yml
@@ -26,7 +26,7 @@ jobs:
       with:
         node-version: 12
     - name: Installing codeclimate client
-      if: matrix.haxe-version == '4.2.5'
+      if: matrix.haxe-version == 'nightly'
       run: |
         curl -L https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64 > ./cc-test-reporter
         chmod +x ./cc-test-reporter
@@ -61,13 +61,13 @@ jobs:
     - name: Run JVM tests
       run: npx haxe testJvm.hxml
     - name: Format and upload codeclimate coverage
-      if: success() && matrix.haxe-version == '4.2.5'
+      if: success() && matrix.haxe-version == 'nightly'
       run: |
         ( \
           cd src; \
           ../cc-test-reporter format-coverage -t lcov ../lcov.info; \
           ../cc-test-reporter upload-coverage; \
         )
     - name: Upload results to codecov
-      if: success() && (matrix.haxe-version == '4.2.5')
+      if: success() && (matrix.haxe-version == 'nightly')
       run: bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## dev branch / next version (2.x.x)
 
+## version 2.8.3 (2022-09-21)
+
+- Fixed MagicNumber check to support numeric separators and suffixes ([#515](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/515))
+
 ## version 2.8.2 (2022-09-14)
 
 - Updated haxeparser and tokentree libs to support latest Haxe nightly syntax ([#514](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/514))
diff --git a/haxelib.json b/haxelib.json
@@ -18,8 +18,8 @@
         "adireddy",
         "Gama11"
 	],
-    "releasenote": "updated haxeparser and tokentree libs to support latest Haxe nightly syntax - see CHANGELOG",
-    "version": "2.8.2",
+    "releasenote": "fixed support for numeric separators and suffixes in MagicNumber check - see CHANGELOG",
+    "version": "2.8.3",
     "url": "https://github.com/HaxeCheckstyle/haxe-checkstyle",
     "dependencies": {}
 }
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
     "name": "@haxecheckstyle/haxe-checkstyle",
-    "version": "2.8.2",
+    "version": "2.8.3",
     "description": "Automated code analysis ideal for projects that want to enforce a coding standard.",
     "repository": {
         "type": "git",
diff --git a/src/checkstyle/checks/Check.hx b/src/checkstyle/checks/Check.hx
@@ -37,15 +37,15 @@ class Check {
 			try {
 				actualRun();
 			}
-			catch (e:String) {
+			catch (e:Exception) {
 				ErrorUtils.handleException(e, checker.file, getModuleName());
 			}
 		}
 		return messages;
 	}
 
 	function actualRun() {
-		throw "Unimplemented";
+		throw new Exception("Unimplemented");
 	}
 
 	public function logPos(msg:String, pos:Position, ?code:String, ?sev:SeverityLevel):Message {
diff --git a/src/checkstyle/checks/block/LeftCurlyCheck.hx b/src/checkstyle/checks/block/LeftCurlyCheck.hx
@@ -241,15 +241,15 @@ class LeftCurlyCheck extends Check {
 			logErrorIf((option == NLOW) && !wrapped, "Left curly should be at EOL (previous expression is not split over multiple lines)", pos);
 			logErrorIf((option != NL) && (option != NLOW), "Left curly unknown option ${option}", pos);
 		}
-		catch (e:String) {
+		catch (e:Exception) {
 			// one of the error messages fired -> do nothing
 		}
 	}
 
 	function logErrorIf(condition:Bool, msg:String, pos:Position) {
 		if (condition) {
 			logPos(msg, pos);
-			throw "exit";
+			throw new Exception("exit");
 		}
 	}
 
diff --git a/src/checkstyle/checks/block/RightCurlyCheck.hx b/src/checkstyle/checks/block/RightCurlyCheck.hx
@@ -195,15 +195,15 @@ class RightCurlyCheck extends Check {
 			logErrorIf(shouldHaveSameOption && (option == SAME), 'Right curly should be on same line as following block (e.g. "} else" or "} catch")',
 				curlyPos);
 		}
-		catch (e:String) {
+		catch (e:Exception) {
 			// one of the error messages fired -> do nothing
 		}
 	}
 
 	function logErrorIf(condition:Bool, msg:String, pos:Position) {
 		if (condition) {
 			logPos(msg, pos);
-			throw "exit";
+			throw new Exception("exit");
 		}
 	}
 
diff --git a/src/checkstyle/checks/coding/HiddenFieldCheck.hx b/src/checkstyle/checks/coding/HiddenFieldCheck.hx
@@ -65,7 +65,7 @@ class HiddenFieldCheck extends Check {
 	}
 
 	function checkMethod(method:TokenTree, memberNames:Array<String>, ignoreFormatRE:EReg) {
-		if (!method.hasChildren()) throw "function has invalid structure!";
+		if (!method.hasChildren()) throw new Exception("function has invalid structure!");
 
 		// handle constructor and setters
 		var methodName:TokenTree = method.children[0];
@@ -115,7 +115,7 @@ class HiddenFieldCheck extends Check {
 		});
 
 		if ((paramDef == null) || (paramDef.length != 1)) {
-			throw "function parameters have invalid structure!";
+			throw new Exception("function parameters have invalid structure!");
 		}
 		var paramList:Array<TokenTree> = paramDef[0].children;
 		for (param in paramList) checkName(param, memberNames, "Parameter definition");
@@ -131,7 +131,7 @@ class HiddenFieldCheck extends Check {
 			}
 		});
 		for (v in vars) {
-			if (!v.hasChildren()) throw "var has invalid structure!";
+			if (!v.hasChildren()) throw new Exception("var has invalid structure!");
 			checkName(v.children[0], memberNames, "Variable definition");
 		}
 	}
diff --git a/src/checkstyle/checks/coding/MagicNumberCheck.hx b/src/checkstyle/checks/coding/MagicNumberCheck.hx
@@ -47,14 +47,16 @@ class MagicNumberCheck extends Check {
 			if (isPosSuppressed(numberToken.pos)) continue;
 			if (filterNumber(numberToken)) continue;
 			switch (numberToken.tok) {
-				case Const(CInt(n)):
-					var number:Int = Std.parseInt(n);
+				case Const(CInt(n, s)):
+					var number:Int = Std.parseInt(n.replace("_", ""));
 					if (ignoreNumbers.contains(number)) continue;
-					logPos('"$n" is a magic number', numberToken.pos);
-				case Const(CFloat(n)):
-					var number:Float = Std.parseFloat(n);
+					if (s == null) s = "";
+					logPos('"$n$s" is a magic number', numberToken.pos);
+				case Const(CFloat(n, s)):
+					var number:Float = Std.parseFloat(n.replace("_", ""));
 					if (ignoreNumbers.contains(number)) continue;
-					logPos('"$n" is a magic number', numberToken.pos);
+					if (s == null) s = "";
+					logPos('"$n$s" is a magic number', numberToken.pos);
 				default:
 			}
 		}
diff --git a/src/checkstyle/checks/whitespace/ListOfEmptyLines.hx b/src/checkstyle/checks/whitespace/ListOfEmptyLines.hx
@@ -110,7 +110,7 @@ class ListOfEmptyLines {
 		@return EmptyLineRange matching range or NONE
 	**/
 	public function checkPolicySingleRange(policy:EmptyLinesPolicy, max:Int, start:Int, end:Int):EmptyLineRange {
-		if (start > end) throw "*** wrong order!! *** " + start + " " + end;
+		if (start > end) throw new Exception("*** wrong order!! *** " + start + " " + end);
 		var range:Array<EmptyLineRange> = getRanges(start, end);
 
 		switch (policy) {
diff --git a/src/checkstyle/import.hx b/src/checkstyle/import.hx
@@ -4,6 +4,7 @@ import checkstyle.Checker.LinePos;
 import checkstyle.SeverityLevel;
 import checkstyle.detect.DetectableInstances;
 import checkstyle.utils.ErrorUtils;
+import haxe.Exception;
 import haxe.io.Bytes;
 import haxe.macro.Expr;
 import haxeparser.Data;
diff --git a/test/checkstyle/checks/coding/MagicNumberCheckTest.hx b/test/checkstyle/checks/coding/MagicNumberCheckTest.hx
@@ -51,6 +51,26 @@ class MagicNumberCheckTest extends CheckTestCase<MagicNumberCheckTests> {
 		assertNoMsg(check, HAXE4_FINAL_VAR);
 		assertMsg(check, HAXE4_FINAL_FUNCTION, '"7" is a magic number');
 	}
+
+	#if (haxe >= version("4.3.0-rc.1"))
+	@Test
+	public function testNumberSeparatorAndSuffixes() {
+		var check = new MagicNumberCheck();
+		assertMessages(check, NUMBER_SEPARATOR_AND_SUFFIX, [
+			'"1_000.1e2f64" is a magic number',
+			'"1_000i64" is a magic number',
+			'"0xabcdei32" is a magic number',
+			'"0xabcdeu32" is a magic number',
+			'"1.1E+3f64" is a magic number',
+			'"1.E+3f64" is a magic number',
+			'"1E+3f64" is a magic number',
+			'"1.1f64" is a magic number',
+			'"5f64" is a magic number',
+			'".1f64" is a magic number',
+			'"5i64" is a magic number',
+		]);
+	}
+	#end
 }
 
 enum abstract MagicNumberCheckTests(String) to String {
@@ -143,4 +163,18 @@ enum abstract MagicNumberCheckTests(String) to String {
 		function test() {
 		}
 	}";
+	var NUMBER_SEPARATOR_AND_SUFFIX = "
+	abstractAndClass Test {
+		var x = 1_000.1e2f64;
+		var y = 1_000i64;
+		var z = 0xabcdei32;
+		var x0 = 0xabcdeu32;
+		var x1 = 1.1E+3f64;
+		var x2 = 1.E+3f64;
+		var x3 = 1E+3f64;
+		var x4 = 1.1f64;
+		var x5 = 5f64;
+		var x6 = .1f64;
+		var x7 = 5i64;
+	}";
 }

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@haxecheckstyle/haxe-checkstyle",`
`3`		`- "version": "2.8.2",`
	`3`	`+ "version": "2.8.3",`
`4`	`4`	`"description": "Automated code analysis ideal for projects that want to enforce a coding standard.",`
`5`	`5`	`"repository": {`
`6`	`6`	`"type": "git",`
Original file line number	Diff line number	Diff line change
`@@ -37,15 +37,15 @@ class Check {`
`37`	`37`	`try {`
`38`	`38`	`actualRun();`
`39`	`39`	`}`
`40`		`- catch (e:String) {`
	`40`	`+ catch (e:Exception) {`
`41`	`41`	`ErrorUtils.handleException(e, checker.file, getModuleName());`
`42`	`42`	`}`
`43`	`43`	`}`
`44`	`44`	`return messages;`
`45`	`45`	`}`
`46`	`46`
`47`	`47`	`function actualRun() {`
`48`		`- throw "Unimplemented";`
	`48`	`+ throw new Exception("Unimplemented");`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	`public function logPos(msg:String, pos:Position, ?code:String, ?sev:SeverityLevel):Message {`
Original file line number	Diff line number	Diff line change
`@@ -241,15 +241,15 @@ class LeftCurlyCheck extends Check {`
`241`	`241`	`logErrorIf((option == NLOW) && !wrapped, "Left curly should be at EOL (previous expression is not split over multiple lines)", pos);`
`242`	`242`	`logErrorIf((option != NL) && (option != NLOW), "Left curly unknown option ${option}", pos);`
`243`	`243`	`}`
`244`		`- catch (e:String) {`
	`244`	`+ catch (e:Exception) {`
`245`	`245`	`// one of the error messages fired -> do nothing`
`246`	`246`	`}`
`247`	`247`	`}`
`248`	`248`
`249`	`249`	`function logErrorIf(condition:Bool, msg:String, pos:Position) {`
`250`	`250`	`if (condition) {`
`251`	`251`	`logPos(msg, pos);`
`252`		`- throw "exit";`
	`252`	`+ throw new Exception("exit");`
`253`	`253`	`}`
`254`	`254`	`}`
`255`	`255`
Original file line number	Diff line number	Diff line change
`@@ -195,15 +195,15 @@ class RightCurlyCheck extends Check {`
`195`	`195`	`logErrorIf(shouldHaveSameOption && (option == SAME), 'Right curly should be on same line as following block (e.g. "} else" or "} catch")',`
`196`	`196`	`curlyPos);`
`197`	`197`	`}`
`198`		`- catch (e:String) {`
	`198`	`+ catch (e:Exception) {`
`199`	`199`	`// one of the error messages fired -> do nothing`
`200`	`200`	`}`
`201`	`201`	`}`
`202`	`202`
`203`	`203`	`function logErrorIf(condition:Bool, msg:String, pos:Position) {`
`204`	`204`	`if (condition) {`
`205`	`205`	`logPos(msg, pos);`
`206`		`- throw "exit";`
	`206`	`+ throw new Exception("exit");`
`207`	`207`	`}`
`208`	`208`	`}`
`209`	`209`
Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ class HiddenFieldCheck extends Check {`
`65`	`65`	`}`
`66`	`66`
`67`	`67`	`function checkMethod(method:TokenTree, memberNames:Array<String>, ignoreFormatRE:EReg) {`
`68`		`- if (!method.hasChildren()) throw "function has invalid structure!";`
	`68`	`+ if (!method.hasChildren()) throw new Exception("function has invalid structure!");`
`69`	`69`
`70`	`70`	`// handle constructor and setters`
`71`	`71`	`var methodName:TokenTree = method.children[0];`
`@@ -115,7 +115,7 @@ class HiddenFieldCheck extends Check {`
`115`	`115`	`});`
`116`	`116`
`117`	`117`	`if ((paramDef == null) \|\| (paramDef.length != 1)) {`
`118`		`- throw "function parameters have invalid structure!";`
	`118`	`+ throw new Exception("function parameters have invalid structure!");`
`119`	`119`	`}`
`120`	`120`	`var paramList:Array<TokenTree> = paramDef[0].children;`
`121`	`121`	`for (param in paramList) checkName(param, memberNames, "Parameter definition");`
`@@ -131,7 +131,7 @@ class HiddenFieldCheck extends Check {`
`131`	`131`	`}`
`132`	`132`	`});`
`133`	`133`	`for (v in vars) {`
`134`		`- if (!v.hasChildren()) throw "var has invalid structure!";`
	`134`	`+ if (!v.hasChildren()) throw new Exception("var has invalid structure!");`
`135`	`135`	`checkName(v.children[0], memberNames, "Variable definition");`
`136`	`136`	`}`
`137`	`137`	`}`