From 70ba49359e61db98b311dbad3203e6a67ae847c0 Mon Sep 17 00:00:00 2001 From: AlexHaxe Date: Sun, 24 Nov 2019 13:31:53 +0100 Subject: [PATCH] fixed position reported for Dynamic violations (#489) * fixed position reported for Dynamic violations * changed implementation of Dynamic check to use tokentree * moved to Haxe 4.0.2 --- .github/workflows/checkstyle-linux.yml | 8 ++--- .haxerc | 2 +- .travis.yml | 2 +- CHANGELOG.md | 1 + README.md | 1 + src/checkstyle/Checker.hx | 6 ++-- src/checkstyle/Main.hx | 10 +++--- src/checkstyle/checks/type/DynamicCheck.hx | 32 +++++++------------ src/checkstyle/import.hx | 12 +++---- test/TestMain.hx | 8 ++--- test/checkstyle/checks/CheckTestCase.hx | 4 +-- .../checks/type/DynamicCheckTest.hx | 14 ++++---- test/checkstyle/config/ExcludeManagerTest.hx | 6 ++-- .../detect/DetectCodingStyleTest.hx | 12 +++---- 14 files changed, 56 insertions(+), 62 deletions(-) diff --git a/.github/workflows/checkstyle-linux.yml b/.github/workflows/checkstyle-linux.yml index ca12ea36..8ef72b0b 100644 --- a/.github/workflows/checkstyle-linux.yml +++ b/.github/workflows/checkstyle-linux.yml @@ -15,7 +15,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - haxe-version: ['3.4.7', '4.0.0', 'nightly'] + haxe-version: ['3.4.7', '4.0.2', 'nightly'] env: CC_TEST_REPORTER_ID: 1dff6f89d7179dff5db635c6b4fe64acdd5694c9ed44d7da5f12f0f7d3d163b7 CODECOV_TOKEN: ${{secrets.CODECOV_TOKEN}} @@ -26,7 +26,7 @@ jobs: with: node-version: 10 - name: Installing codeclimate client - if: matrix.haxe-version == '4.0.0' + if: matrix.haxe-version == '4.0.2' run: | curl -L https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64 > ./cc-test-reporter chmod +x ./cc-test-reporter @@ -68,7 +68,7 @@ jobs: - name: Run Java tests run: npx haxe testJava.hxml - name: Format and upload codeclimate coverage - if: success() && matrix.haxe-version == '4.0.0' + if: success() && matrix.haxe-version == '4.0.2' run: | ( \ cd src; \ @@ -76,5 +76,5 @@ jobs: ../cc-test-reporter upload-coverage; \ ) - name: Upload results to codecov - if: success() && matrix.haxe-version == '4.0.0' + if: success() && matrix.haxe-version == '4.0.2' run: bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports" diff --git a/.haxerc b/.haxerc index d7842d32..0047672a 100644 --- a/.haxerc +++ b/.haxerc @@ -1,4 +1,4 @@ { - "version": "4.0.0", + "version": "4.0.2", "resolveLibs": "scoped" } \ No newline at end of file diff --git a/.travis.yml b/.travis.yml index 40bfe931..7550bfff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,7 +17,7 @@ install: - npm install - if [[ "$HAXE_VERSION" == "haxe347" ]]; then mv haxe_libraries haxe4_libraries; mv haxe3_libraries haxe_libraries; fi - if [[ "$HAXE_VERSION" == "haxe347" ]]; then npx lix download haxe 3.4.7; npx lix use haxe 3.4.7; fi - - if [[ "$HAXE_VERSION" == "haxe4" ]]; then npx lix download haxe 4.0.0; npx lix use haxe 4.0.0; fi + - if [[ "$HAXE_VERSION" == "haxe4" ]]; then npx lix download haxe 4.0.2; npx lix use haxe 4.0.2; fi - if [[ "$HAXE_VERSION" == "nightly" ]]; then npx lix download haxe nightly; npx lix use haxe nightly; fi - npx lix download - npx haxe -version diff --git a/CHANGELOG.md b/CHANGELOG.md index 8707a92d..7e799642 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - 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)) +- Changed `Dynamic` check implementation to tokentree, now only reports token location ([#489](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/489)) - 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)) diff --git a/README.md b/README.md index e9943054..f033c98d 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,7 @@ haxe buildCpp.hxml # for C++ version ``` Compiling with Haxe 3 + ```bash git clone https://github.com/HaxeCheckstyle/haxe-checkstyle.git mv haxe_libraries haxe4_libraries diff --git a/src/checkstyle/Checker.hx b/src/checkstyle/Checker.hx index 2741e28a..6ec1be3d 100644 --- a/src/checkstyle/Checker.hx +++ b/src/checkstyle/Checker.hx @@ -1,12 +1,12 @@ package checkstyle; -import haxeparser.HaxeParser; -import haxeparser.HaxeLexer; -import sys.io.File; import checkstyle.checks.Check; import checkstyle.config.ExcludeManager; import checkstyle.config.ExcludeRange; import checkstyle.reporter.ReporterManager; +import haxeparser.HaxeLexer; +import haxeparser.HaxeParser; +import sys.io.File; import tokentree.TokenTreeBuilder; class Checker { diff --git a/src/checkstyle/Main.hx b/src/checkstyle/Main.hx index b2941a69..541b0a2a 100644 --- a/src/checkstyle/Main.hx +++ b/src/checkstyle/Main.hx @@ -2,25 +2,25 @@ package checkstyle; import checkstyle.ChecksInfo; import checkstyle.checks.Check; -import checkstyle.config.ConfigParser; import checkstyle.config.CheckConfig; +import checkstyle.config.ConfigParser; import checkstyle.config.ExcludeManager; import checkstyle.detect.DetectCodingStyle; +import checkstyle.reporter.CodeClimateReporter; +import checkstyle.reporter.ExitCodeReporter; import checkstyle.reporter.IReporter; import checkstyle.reporter.JSONReporter; import checkstyle.reporter.ProgressReporter; +import checkstyle.reporter.ReporterManager; import checkstyle.reporter.TextReporter; import checkstyle.reporter.XMLReporter; -import checkstyle.reporter.CodeClimateReporter; -import checkstyle.reporter.ExitCodeReporter; -import checkstyle.reporter.ReporterManager; import checkstyle.utils.ConfigUtils; import haxe.CallStack; import haxe.Json; +import haxe.io.Path; import hxargs.Args; import sys.FileSystem; import sys.io.File; -import haxe.io.Path; class Main { static var DEFAULT_CONFIG:String = "checkstyle.json"; diff --git a/src/checkstyle/checks/type/DynamicCheck.hx b/src/checkstyle/checks/type/DynamicCheck.hx index c0169320..02ff2603 100644 --- a/src/checkstyle/checks/type/DynamicCheck.hx +++ b/src/checkstyle/checks/type/DynamicCheck.hx @@ -1,7 +1,5 @@ package checkstyle.checks.type; -import checkstyle.utils.ComplexTypeUtils; - /** Checks for use of Dynamic type anywhere in the code. **/ @@ -9,29 +7,23 @@ import checkstyle.utils.ComplexTypeUtils; @desc("Checks for use of Dynamic type anywhere in the code.") class DynamicCheck extends Check { public function new() { - super(AST); + super(TOKEN); categories = [Category.CLARITY, Category.BUG_RISK, Category.COMPLEXITY]; points = 3; } override function actualRun() { - if (checker.ast == null) return; - ComplexTypeUtils.walkFile(checker.ast, callbackComplexType); - } - - function callbackComplexType(t:ComplexType, name:String, pos:Position) { - if (t == null) return; - switch (t) { - case TPath(p): - if (p.name != "Dynamic") return; - if (isPosSuppressed(pos)) return; - error(name, pos); - default: - } - } - - function error(name:String, pos:Position) { - logPos('"${name}" type is "Dynamic"', pos); + var root:TokenTree = checker.getTokenTree(); + root.filterCallback(function(token:TokenTree, index:Int):FilterResult { + switch token.tok { + case Const(CIdent("Dynamic")): + if (isPosSuppressed(token.pos)) return SKIP_SUBTREE; + logPos('Avoid using "Dynamic" as type', token.pos); + return SKIP_SUBTREE; + default: + return GO_DEEPER; + } + }); } override public function detectableInstances():DetectableInstances { diff --git a/src/checkstyle/import.hx b/src/checkstyle/import.hx index 6e9082ee..8b94a048 100644 --- a/src/checkstyle/import.hx +++ b/src/checkstyle/import.hx @@ -1,19 +1,19 @@ package checkstyle; -import haxe.io.Bytes; -import haxe.macro.Expr; -import haxeparser.Data; import checkstyle.Checker.LinePos; import checkstyle.SeverityLevel; import checkstyle.detect.DetectableInstances; import checkstyle.utils.ErrorUtils; +import haxe.io.Bytes; +import haxe.macro.Expr; +import haxeparser.Data; import tokentree.TokenTree; import tokentree.TokenTreeAccessHelper; import tokentree.utils.TokenTreeCheckUtils; +using StringTools; using checkstyle.utils.ArrayUtils; -using checkstyle.utils.FieldUtils; using checkstyle.utils.ExprUtils; +using checkstyle.utils.FieldUtils; using checkstyle.utils.StringUtils; -using tokentree.TokenTreeAccessHelper; -using StringTools; \ No newline at end of file +using tokentree.TokenTreeAccessHelper; \ No newline at end of file diff --git a/test/TestMain.hx b/test/TestMain.hx index 4e632df3..fb7d9fd3 100644 --- a/test/TestMain.hx +++ b/test/TestMain.hx @@ -1,14 +1,14 @@ -import haxe.EntryPoint; import massive.munit.TestRunner; +import mcover.coverage.MCoverage; import mcover.coverage.munit.client.MCoverPrintClient; +#if (neko || cpp || hl) +import haxe.EntryPoint; +#end #if codecov_json import mcover.coverage.client.CodecovJsonPrintClient; #else import mcover.coverage.client.LcovPrintClient; #end -import mcover.coverage.MCoverage; - -using StringTools; class TestMain { public function new() { diff --git a/test/checkstyle/checks/CheckTestCase.hx b/test/checkstyle/checks/CheckTestCase.hx index aa8a0b4d..262499ce 100644 --- a/test/checkstyle/checks/CheckTestCase.hx +++ b/test/checkstyle/checks/CheckTestCase.hx @@ -1,11 +1,11 @@ package checkstyle.checks; import byte.ByteData; -import checkstyle.CheckMessage; import checkstyle.CheckFile; +import checkstyle.CheckMessage; +import checkstyle.Checker; import checkstyle.reporter.IReporter; import checkstyle.reporter.ReporterManager; -import checkstyle.Checker; class CheckTestCase { static inline var FILE_NAME:String = "Test.hx"; diff --git a/test/checkstyle/checks/type/DynamicCheckTest.hx b/test/checkstyle/checks/type/DynamicCheckTest.hx index 10a09dab..c5571386 100644 --- a/test/checkstyle/checks/type/DynamicCheckTest.hx +++ b/test/checkstyle/checks/type/DynamicCheckTest.hx @@ -1,6 +1,8 @@ package checkstyle.checks.type; class DynamicCheckTest extends CheckTestCase { + public static inline var AVOID_USING_DYNAMIC_AS_TYPE:String = 'Avoid using "Dynamic" as type'; + @Test public function testNoDynamic() { var check = new DynamicCheck(); @@ -10,12 +12,12 @@ class DynamicCheckTest extends CheckTestCase { @Test public function testDetectDynamic() { var check = new DynamicCheck(); - assertMsg(check, TEST1, '"Count" type is "Dynamic"'); - assertMsg(check, TEST2, '"test" type is "Dynamic"'); - assertMsg(check, TEST3, '"Count" type is "Dynamic"'); - assertMsg(check, TEST4, '"param" type is "Dynamic"'); - assertMsg(check, TEST5, '"test" type is "Dynamic"'); - assertMsg(check, TEST6, '"Test" type is "Dynamic"'); + assertMsg(check, TEST1, AVOID_USING_DYNAMIC_AS_TYPE); + assertMsg(check, TEST2, AVOID_USING_DYNAMIC_AS_TYPE); + assertMsg(check, TEST3, AVOID_USING_DYNAMIC_AS_TYPE); + assertMsg(check, TEST4, AVOID_USING_DYNAMIC_AS_TYPE); + assertMsg(check, TEST5, AVOID_USING_DYNAMIC_AS_TYPE); + assertMsg(check, TEST6, AVOID_USING_DYNAMIC_AS_TYPE); assertNoMsg(check, ISSUE_43); } diff --git a/test/checkstyle/config/ExcludeManagerTest.hx b/test/checkstyle/config/ExcludeManagerTest.hx index 6ff961e8..c36a00dc 100644 --- a/test/checkstyle/config/ExcludeManagerTest.hx +++ b/test/checkstyle/config/ExcludeManagerTest.hx @@ -1,8 +1,8 @@ package checkstyle.config; +import checkstyle.checks.CheckTestCase; import checkstyle.checks.type.DynamicCheck; import checkstyle.checks.type.ReturnCheck; -import checkstyle.checks.CheckTestCase; class ExcludeManagerTest extends CheckTestCase { static inline var LOCAL_PATH:String = "./"; @@ -65,7 +65,7 @@ class ExcludeManagerTest extends CheckTestCase { configParser.parseExcludes(cast { version: 1, path: ExcludePath.RELATIVE_TO_PROJECT, - Dynamic: ["src/checkstyle/checks/Check$", "test/checkstyle/checks/Check$"] + "Dynamic": ["src/checkstyle/checks/Check$", "test/checkstyle/checks/Check$"] }); Assert.isTrue(ExcludeManager.isExcludedFromCheck(CHECK_FILE_NAME, DYNAMIC)); @@ -86,7 +86,7 @@ class ExcludeManagerTest extends CheckTestCase { configParser.parseExcludes(cast { version: 1, path: ExcludePath.RELATIVE_TO_PROJECT, - Dynamic: [ + "Dynamic": [ "src/checkstyle/checks/ChecksInfo$:10", "src/checkstyle/checks/ChecksInfo$:14-16", "src/checkstyle/checks/ChecksInfo$:usesDynamic", diff --git a/test/checkstyle/detect/DetectCodingStyleTest.hx b/test/checkstyle/detect/DetectCodingStyleTest.hx index 9dd61945..acb5ca94 100644 --- a/test/checkstyle/detect/DetectCodingStyleTest.hx +++ b/test/checkstyle/detect/DetectCodingStyleTest.hx @@ -1,7 +1,6 @@ package checkstyle.detect; import byte.ByteData; -import checkstyle.config.CheckConfig; import checkstyle.CheckFile; import checkstyle.SeverityLevel; import checkstyle.checks.block.BlockBreakingConditionalCheck; @@ -20,13 +19,13 @@ import checkstyle.checks.coding.NullableParameterCheck; import checkstyle.checks.coding.ReturnCountCheck; import checkstyle.checks.coding.TraceCheck; import checkstyle.checks.coding.UnusedLocalVarCheck; -import checkstyle.checks.design.EmptyPackageCheck; -import checkstyle.checks.design.InterfaceCheck; -import checkstyle.checks.design.UnnecessaryConstructorCheck; import checkstyle.checks.comments.DocCommentStyleCheck; import checkstyle.checks.comments.FieldDocCommentCheck; import checkstyle.checks.comments.TODOCommentCheck; import checkstyle.checks.comments.TypeDocCommentCheck; +import checkstyle.checks.design.EmptyPackageCheck; +import checkstyle.checks.design.InterfaceCheck; +import checkstyle.checks.design.UnnecessaryConstructorCheck; import checkstyle.checks.imports.AvoidStarImportCheck; import checkstyle.checks.imports.UnusedImportCheck; import checkstyle.checks.literal.StringLiteralCheck; @@ -48,18 +47,17 @@ import checkstyle.checks.type.TypeCheck; import checkstyle.checks.whitespace.ArrayAccessCheck; import checkstyle.checks.whitespace.EmptyLinesCheck; import checkstyle.checks.whitespace.ExtendedEmptyLinesCheck; -import checkstyle.checks.whitespace.ExtendedEmptyLinesCheck.EmptyLinesPolicy; import checkstyle.checks.whitespace.IndentationCharacterCheck; import checkstyle.checks.whitespace.IndentationCheck; import checkstyle.checks.whitespace.OperatorWhitespaceCheck; import checkstyle.checks.whitespace.OperatorWrapCheck; -import checkstyle.checks.whitespace.SeparatorWrapCheck; import checkstyle.checks.whitespace.SeparatorWhitespaceCheck; +import checkstyle.checks.whitespace.SeparatorWrapCheck; import checkstyle.checks.whitespace.SpacingCheck; -import checkstyle.checks.whitespace.SpacingCheck.SpacingPolicy; import checkstyle.checks.whitespace.TrailingWhitespaceCheck; import checkstyle.checks.whitespace.WhitespaceCheckBase.WhitespacePolicy; import checkstyle.checks.whitespace.WrapCheckBase.WrapCheckBaseOption; +import checkstyle.config.CheckConfig; class DetectCodingStyleTest { // checkstyle.checks.block