From 119223c2a517a3e869ad870abb91dee4d9c35dc2 Mon Sep 17 00:00:00 2001 From: AlexHaxe Date: Wed, 21 Sep 2022 22:42:51 +0200 Subject: [PATCH] fixed MagicNumber check to support numeric separators and suffixes (#515) * 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 --- .github/workflows/checkstyle-linux.yml | 6 ++-- CHANGELOG.md | 4 +++ haxelib.json | 4 +-- package-lock.json | 4 +-- package.json | 2 +- src/checkstyle/checks/Check.hx | 4 +-- src/checkstyle/checks/block/LeftCurlyCheck.hx | 4 +-- .../checks/block/RightCurlyCheck.hx | 4 +-- .../checks/coding/HiddenFieldCheck.hx | 6 ++-- .../checks/coding/MagicNumberCheck.hx | 14 ++++---- .../checks/whitespace/ListOfEmptyLines.hx | 2 +- src/checkstyle/import.hx | 1 + .../checks/coding/MagicNumberCheckTest.hx | 34 +++++++++++++++++++ 13 files changed, 65 insertions(+), 24 deletions(-) diff --git a/.github/workflows/checkstyle-linux.yml b/.github/workflows/checkstyle-linux.yml index cf76faac..72ff6c5a 100644 --- 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,7 +61,7 @@ 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; \ @@ -69,5 +69,5 @@ jobs: ../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 index 11cd6c1c..81d21afa 100644 --- 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 index 62b77f0b..122ab6f1 100644 --- 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": {} } \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index ba0de5b3..5c91c6cd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@haxecheckstyle/haxe-checkstyle", - "version": "2.8.2", + "version": "2.8.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@haxecheckstyle/haxe-checkstyle", - "version": "2.8.2", + "version": "2.8.3", "license": "MIT", "bin": { "haxe-checkstyle": "bin/checkstyle.js" diff --git a/package.json b/package.json index dd4dfa0f..8f114e8f 100644 --- 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 index 34cbeb7b..2058cec3 100644 --- a/src/checkstyle/checks/Check.hx +++ b/src/checkstyle/checks/Check.hx @@ -37,7 +37,7 @@ class Check { try { actualRun(); } - catch (e:String) { + catch (e:Exception) { ErrorUtils.handleException(e, checker.file, getModuleName()); } } @@ -45,7 +45,7 @@ class Check { } 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 index fb80f6b0..da7bae32 100644 --- a/src/checkstyle/checks/block/LeftCurlyCheck.hx +++ b/src/checkstyle/checks/block/LeftCurlyCheck.hx @@ -241,7 +241,7 @@ 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 } } @@ -249,7 +249,7 @@ class LeftCurlyCheck extends Check { 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 index 8f2d06b7..043bda21 100644 --- a/src/checkstyle/checks/block/RightCurlyCheck.hx +++ b/src/checkstyle/checks/block/RightCurlyCheck.hx @@ -195,7 +195,7 @@ 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 } } @@ -203,7 +203,7 @@ class RightCurlyCheck extends Check { 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 index 924fd095..9f980313 100644 --- 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, 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 = 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 index 20d1e57e..d5d1d050 100644 --- 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 index 4263ae47..3d6eed65 100644 --- 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 = getRanges(start, end); switch (policy) { diff --git a/src/checkstyle/import.hx b/src/checkstyle/import.hx index 5cbdcd3b..8ad5834c 100644 --- 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 index fd5f6bf5..807aa7e8 100644 --- a/test/checkstyle/checks/coding/MagicNumberCheckTest.hx +++ b/test/checkstyle/checks/coding/MagicNumberCheckTest.hx @@ -51,6 +51,26 @@ class MagicNumberCheckTest extends CheckTestCase { 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; + }"; } \ No newline at end of file