diff --git a/.github/workflows/checkstyle-linux.yml b/.github/workflows/checkstyle-linux.yml index df8097e8..ca12ea36 100644 --- a/.github/workflows/checkstyle-linux.yml +++ b/.github/workflows/checkstyle-linux.yml @@ -18,6 +18,7 @@ jobs: haxe-version: ['3.4.7', '4.0.0', 'nightly'] env: CC_TEST_REPORTER_ID: 1dff6f89d7179dff5db635c6b4fe64acdd5694c9ed44d7da5f12f0f7d3d163b7 + CODECOV_TOKEN: ${{secrets.CODECOV_TOKEN}} steps: - uses: actions/checkout@v1 - name: Use Node.js 10 diff --git a/.travis.yml b/.travis.yml index 92028949..40bfe931 100644 --- a/.travis.yml +++ b/.travis.yml @@ -41,4 +41,4 @@ after_script: - if [[ "$HAXE_VERSION" == "haxe4" ]]; then (cd src; ../cc-test-reporter upload-coverage); fi after_success: - - if [[ "$HAXE_VERSION" == "haxe4" ]]; bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports" + - bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports" diff --git a/CHANGELOG.md b/CHANGELOG.md index cdc18c99..9d371228 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,19 +2,23 @@ ## dev branch / next version (2.x.x) -- **Breaking Change** changed `MethodLength.countEmpty` into `ignoreEmptyLines` +- **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 `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)) +- 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)) - 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)) +- 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)) - 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 `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)) ## version 2.5.0 (2019-10-10) diff --git a/resources/checkstyle-excludes-schema.json b/resources/checkstyle-excludes-schema.json index 1fbf7c1a..6f212a10 100644 --- a/resources/checkstyle-excludes-schema.json +++ b/resources/checkstyle-excludes-schema.json @@ -24,6 +24,10 @@ "$ref": "#/definitions/ExcludeFilterList", "propertyOrder": 19 }, + "Final": { + "$ref": "#/definitions/ExcludeFilterList", + "propertyOrder": 28 + }, "MultipleStringLiterals": { "$ref": "#/definitions/ExcludeFilterList", "propertyOrder": 45 @@ -89,7 +93,7 @@ }, "HexadecimalLiteral": { "$ref": "#/definitions/ExcludeFilterList", - "propertyOrder": 28 + "propertyOrder": 29 }, "WhitespaceAfter": { "$ref": "#/definitions/ExcludeFilterList", @@ -123,10 +127,6 @@ "$ref": "#/definitions/ExcludeFilterList", "propertyOrder": 52 }, - "InlineFinal": { - "$ref": "#/definitions/ExcludeFilterList", - "propertyOrder": 32 - }, "AvoidStarImport": { "$ref": "#/definitions/ExcludeFilterList", "propertyOrder": 9 @@ -145,7 +145,7 @@ }, "HiddenField": { "$ref": "#/definitions/ExcludeFilterList", - "propertyOrder": 29 + "propertyOrder": 30 }, "UnnecessaryConstructor": { "$ref": "#/definitions/ExcludeFilterList", @@ -229,7 +229,7 @@ }, "Indentation": { "$ref": "#/definitions/ExcludeFilterList", - "propertyOrder": 30 + "propertyOrder": 31 }, "version": { "maximum": 1, @@ -262,7 +262,7 @@ }, "IndentationCharacter": { "$ref": "#/definitions/ExcludeFilterList", - "propertyOrder": 31 + "propertyOrder": 32 }, "MagicNumber": { "$ref": "#/definitions/ExcludeFilterList", diff --git a/resources/checkstyle-schema.json b/resources/checkstyle-schema.json index 9b2abb63..6782c0b4 100644 --- a/resources/checkstyle-schema.json +++ b/resources/checkstyle-schema.json @@ -1679,38 +1679,6 @@ }, "type": "object" }, - "InlineFinalCheck": { - "description": "Checks for places that use inline var instead of inline final (Haxe 4+).", - "additionalProperties": false, - "properties": { - "type": { - "description": "Checks for places that use inline var instead of inline final (Haxe 4+).", - "type": "string", - "enum": [ - "InlineFinal" - ] - }, - "props": { - "description": "Checks for places that use inline var instead of inline final (Haxe 4+).", - "additionalProperties": false, - "properties": { - "severity": { - "description": "sets gravity of reported violations:\n\t- IGNORE = do not report violations, violations do not appear anywhere in output\n\t- INFO = all violations have info / lowest priority\n\t- WARNING = all violations have warning / medium priority\n\t- ERROR = all violations have error / highest priority", - "type": "string", - "enum": [ - "INFO", - "WARNING", - "ERROR", - "IGNORE" - ], - "propertyOrder": 0 - } - }, - "type": "object" - } - }, - "type": "object" - }, "Threshold": { "description": "threshold for code complexity", "required": [ @@ -3059,6 +3027,10 @@ "$ref": "#/definitions/ExcludeFilterList", "propertyOrder": 19 }, + "Final": { + "$ref": "#/definitions/ExcludeFilterList", + "propertyOrder": 28 + }, "MultipleStringLiterals": { "$ref": "#/definitions/ExcludeFilterList", "propertyOrder": 45 @@ -3124,7 +3096,7 @@ }, "HexadecimalLiteral": { "$ref": "#/definitions/ExcludeFilterList", - "propertyOrder": 28 + "propertyOrder": 29 }, "WhitespaceAfter": { "$ref": "#/definitions/ExcludeFilterList", @@ -3158,10 +3130,6 @@ "$ref": "#/definitions/ExcludeFilterList", "propertyOrder": 52 }, - "InlineFinal": { - "$ref": "#/definitions/ExcludeFilterList", - "propertyOrder": 32 - }, "AvoidStarImport": { "$ref": "#/definitions/ExcludeFilterList", "propertyOrder": 9 @@ -3180,7 +3148,7 @@ }, "HiddenField": { "$ref": "#/definitions/ExcludeFilterList", - "propertyOrder": 29 + "propertyOrder": 30 }, "UnnecessaryConstructor": { "$ref": "#/definitions/ExcludeFilterList", @@ -3264,7 +3232,7 @@ }, "Indentation": { "$ref": "#/definitions/ExcludeFilterList", - "propertyOrder": 30 + "propertyOrder": 31 }, "version": { "maximum": 1, @@ -3297,7 +3265,7 @@ }, "IndentationCharacter": { "$ref": "#/definitions/ExcludeFilterList", - "propertyOrder": 31 + "propertyOrder": 32 }, "MagicNumber": { "$ref": "#/definitions/ExcludeFilterList", @@ -3716,6 +3684,38 @@ }, "type": "object" }, + "FinalCheck": { + "description": "Checks for places that use var instead of final (Haxe 4+).", + "additionalProperties": false, + "properties": { + "type": { + "description": "Checks for places that use var instead of final (Haxe 4+).", + "type": "string", + "enum": [ + "Final" + ] + }, + "props": { + "description": "Checks for places that use var instead of final (Haxe 4+).", + "additionalProperties": false, + "properties": { + "severity": { + "description": "sets gravity of reported violations:\n\t- IGNORE = do not report violations, violations do not appear anywhere in output\n\t- INFO = all violations have info / lowest priority\n\t- WARNING = all violations have warning / medium priority\n\t- ERROR = all violations have error / highest priority", + "type": "string", + "enum": [ + "INFO", + "WARNING", + "ERROR", + "IGNORE" + ], + "propertyOrder": 0 + } + }, + "type": "object" + } + }, + "type": "object" + }, "ConditionalCompilationCheck": { "description": "Checks placement and indentation of conditional compilation flags.", "additionalProperties": false, @@ -4299,6 +4299,9 @@ { "$ref": "#/definitions/FileLengthCheck" }, + { + "$ref": "#/definitions/FinalCheck" + }, { "$ref": "#/definitions/HexadecimalLiteralCheck" }, @@ -4311,9 +4314,6 @@ { "$ref": "#/definitions/IndentationCharacterCheck" }, - { - "$ref": "#/definitions/InlineFinalCheck" - }, { "$ref": "#/definitions/InnerAssignmentCheck" }, diff --git a/resources/default-config.json b/resources/default-config.json index 6e04ac9c..b7c9ee0b 100644 --- a/resources/default-config.json +++ b/resources/default-config.json @@ -196,6 +196,10 @@ "ignoreEmptyLines": true } }, + { + "type": "Final", + "props": {} + }, { "type": "HexadecimalLiteral", "props": { @@ -227,10 +231,6 @@ "ignorePattern": "^$" } }, - { - "type": "InlineFinal", - "props": {} - }, { "type": "InnerAssignment", "props": { diff --git a/schema/CheckstyleSchemaGenerator.hx b/schema/CheckstyleSchemaGenerator.hx index df96a9b6..1a9b2f79 100644 --- a/schema/CheckstyleSchemaGenerator.hx +++ b/schema/CheckstyleSchemaGenerator.hx @@ -102,7 +102,7 @@ class CheckstyleSchemaGenerator { continue; } #if (haxe_ver < 4) - if (name == "InlineFinalCheck") { + if (name == "FinalCheck") { continue; } #end diff --git a/src/checkstyle/checks/comments/DocCommentStyleCheck.hx b/src/checkstyle/checks/comments/DocCommentStyleCheck.hx index e9cad1a6..0decd553 100644 --- a/src/checkstyle/checks/comments/DocCommentStyleCheck.hx +++ b/src/checkstyle/checks/comments/DocCommentStyleCheck.hx @@ -38,6 +38,7 @@ class DocCommentStyleCheck extends Check { Kwd(KwdInterface), Kwd(KwdTypedef), Kwd(KwdVar), + #if haxe4 Kwd(KwdFinal) #else Const(CIdent("final")) #end, Kwd(KwdFunction) ], ALL); diff --git a/src/checkstyle/checks/comments/FieldDocCommentCheck.hx b/src/checkstyle/checks/comments/FieldDocCommentCheck.hx index 90752ea1..087208ab 100644 --- a/src/checkstyle/checks/comments/FieldDocCommentCheck.hx +++ b/src/checkstyle/checks/comments/FieldDocCommentCheck.hx @@ -80,7 +80,14 @@ class FieldDocCommentCheck extends Check { var typeTokens = root.filter(typeTokenDefs, ALL); var fieldTokenDefs:Array = []; - if ((fieldType == VARS) || (fieldType == BOTH)) fieldTokenDefs.push(Kwd(KwdVar)); + if ((fieldType == VARS) || (fieldType == BOTH)) { + fieldTokenDefs.push(Kwd(KwdVar)); + #if haxe4 + fieldTokenDefs.push(Kwd(KwdFinal)); + #else + fieldTokenDefs.push(Const(CIdent("final"))); + #end + } if ((fieldType == FUNCTIONS) || (fieldType == BOTH)) fieldTokenDefs.push(Kwd(KwdFunction)); for (typeToken in typeTokens) { @@ -112,8 +119,9 @@ class FieldDocCommentCheck extends Check { var name:String = getTypeName(token); if (excludeNames.indexOf(name) >= 0) return; var prevToken:TokenTree = token.previousSibling; + if (prevToken == null || !prevToken.isComment()) { - logPos('Field "$name" should have documentation', token.getPos()); + logPos('Field "$name" should have documentation', getReportPos(token)); return; } switch (prevToken.tok) { @@ -123,6 +131,29 @@ 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 = 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 = token.filter([Kwd(KwdOverride)], FIRST); diff --git a/src/checkstyle/checks/modifier/InlineFinalCheck.hx b/src/checkstyle/checks/modifier/FinalCheck.hx similarity index 52% rename from src/checkstyle/checks/modifier/InlineFinalCheck.hx rename to src/checkstyle/checks/modifier/FinalCheck.hx index 0c006881..d51381e9 100644 --- a/src/checkstyle/checks/modifier/InlineFinalCheck.hx +++ b/src/checkstyle/checks/modifier/FinalCheck.hx @@ -2,11 +2,11 @@ package checkstyle.checks.modifier; #if (haxe4) /** - Checks for places that use inline var instead of inline final (Haxe 4+) + Checks for places that use var instead of final (Haxe 4+). **/ -@name("InlineFinal") -@desc("Checks for places that use inline var instead of inline final (Haxe 4+).") -class InlineFinalCheck extends Check { +@name("Final", "InlineFinal") +@desc("Checks for places that use var instead of final (Haxe 4+).") +class FinalCheck extends Check { public function new() { super(AST); categories = [Category.STYLE, Category.CLARITY]; @@ -26,9 +26,21 @@ class InlineFinalCheck extends Check { return; } if (f.access.contains(AFinal)) return; - if (!f.access.contains(AInline)) return; - logPos('Consider using "inline final" for field "${f.name}"', f.pos); + checkPublicStatic(f); + checkInlineVar(f); + } + + function checkPublicStatic(f:Field) { + if (!f.access.contains(APublic)) return; + if (!f.access.contains(AStatic)) return; + + logPos('Public static field "${f.name}" should be "final"', f.pos, SHOULD_BE_PUBLIC_FINAL); + } + + function checkInlineVar(f:Field) { + if (!f.access.contains(AInline)) return; + logPos('Consider using "inline final" for field "${f.name}"', f.pos, USE_INLINE_FINAL); } override public function detectableInstances():DetectableInstances { @@ -41,4 +53,10 @@ class InlineFinalCheck extends Check { }]; } } + +@:enum +abstract FinalCode(String) to String { + var USE_INLINE_FINAL = "UseInlineFinal"; + var SHOULD_BE_PUBLIC_FINAL = "ShouldBePublicFinal"; +} #end \ No newline at end of file diff --git a/src/checkstyle/checks/size/MethodLengthCheck.hx b/src/checkstyle/checks/size/MethodLengthCheck.hx index c2369925..4c04450b 100755 --- a/src/checkstyle/checks/size/MethodLengthCheck.hx +++ b/src/checkstyle/checks/size/MethodLengthCheck.hx @@ -1,5 +1,6 @@ package checkstyle.checks.size; +import checkstyle.checks.comments.FieldDocCommentCheck; import checkstyle.checks.whitespace.ListOfEmptyLines; /** @@ -56,7 +57,7 @@ class MethodLengthCheck extends Check { case _: } } - if (len > max) warnFunctionLength(len, name, pos); + if (len > max) warnFunctionLength(len, name, FieldDocCommentCheck.getReportPos(token)); } function getLineCount(lmin:Int, lmax:Int, emptyLines:ListOfEmptyLines):Int { diff --git a/test/checkstyle/checks/modifier/FinalCheckTest.hx b/test/checkstyle/checks/modifier/FinalCheckTest.hx new file mode 100644 index 00000000..2cc7f6ac --- /dev/null +++ b/test/checkstyle/checks/modifier/FinalCheckTest.hx @@ -0,0 +1,45 @@ +package checkstyle.checks.modifier; + +#if haxe4 +class FinalCheckTest extends CheckTestCase { + static inline var ERROR_INLINE_VAR:String = 'Consider using "inline final" for field "test"'; + static inline var ERROR_PUBLIC_STATIC:String = 'Public static field "test" should be "final"'; + + @Test + public function testInlineFinal() { + var check = new FinalCheck(); + assertNoMsg(check, TEST_INLINE_FINAL); + } + + @Test + public function testNoncompliant() { + var check = new FinalCheck(); + assertMsg(check, TEST_INLINE_VAR, ERROR_INLINE_VAR); + assertMsg(check, TEST_PUBLIC_STATIC_VAR, ERROR_PUBLIC_STATIC); + } +} +#end + +@:enum +abstract FinalCheckTests(String) to String { + var TEST_INLINE_FINAL = " + abstractAndClass Test { + public inline final test:String = '0'; + public static final test2:String = '0'; + public static var test3(default, null):String = '0'; + final function test2() { + } + }"; + var TEST_INLINE_VAR = " + abstractAndClass Test { + inline public var test:String = '0'; + inline function test2() { + } + }"; + var TEST_PUBLIC_STATIC_VAR = " + abstractAndClass Test { + public static var test:String = '0'; + public static function test2() { + } + }"; +} \ No newline at end of file diff --git a/test/checkstyle/checks/modifier/InlineFinalCheckTest.hx b/test/checkstyle/checks/modifier/InlineFinalCheckTest.hx deleted file mode 100644 index 125542c4..00000000 --- a/test/checkstyle/checks/modifier/InlineFinalCheckTest.hx +++ /dev/null @@ -1,35 +0,0 @@ -package checkstyle.checks.modifier; - -#if haxe4 -class InlineFinalCheckTest extends CheckTestCase { - static inline var ERROR:String = 'Consider using "inline final" for field "test"'; - - @Test - public function testCorrectOrder() { - var check = new InlineFinalCheck(); - assertNoMsg(check, TEST_INLINE_FINAL); - } - - @Test - public function testWrongOrder() { - var check = new InlineFinalCheck(); - assertMsg(check, TEST_INLINE_VAR, ERROR); - } -} -#end - -@:enum -abstract InlineFinalCheckTests(String) to String { - var TEST_INLINE_FINAL = " - abstractAndClass Test { - public inline final test:String='0'; - final function test2() { - } - }"; - var TEST_INLINE_VAR = " - abstractAndClass Test { - inline public var test:String='0'; - inline function test2() { - } - }"; -} \ No newline at end of file diff --git a/test/checkstyle/detect/DetectCodingStyleTest.hx b/test/checkstyle/detect/DetectCodingStyleTest.hx index 5a0318b4..9dd61945 100644 --- a/test/checkstyle/detect/DetectCodingStyleTest.hx +++ b/test/checkstyle/detect/DetectCodingStyleTest.hx @@ -33,7 +33,7 @@ import checkstyle.checks.literal.StringLiteralCheck; import checkstyle.checks.meta.RedundantAccessMetaCheck; import checkstyle.checks.meta.RedundantAllowMetaCheck; import checkstyle.checks.metrics.CyclomaticComplexityCheck; -import checkstyle.checks.modifier.InlineFinalCheck; +import checkstyle.checks.modifier.FinalCheck; import checkstyle.checks.modifier.RedundantModifierCheck; import checkstyle.checks.naming.ConstantNameCheck; import checkstyle.checks.size.FileLengthCheck; @@ -352,9 +352,9 @@ class DetectCodingStyleTest { #if haxe4 @Test public function testDetectInlineFinal() { - var detectedChecks:Array = DetectCodingStyle.detectCodingStyle([new InlineFinalCheck()], [buildCheckFile(SAMPLE_CODING_STYLE)]); + var detectedChecks:Array = DetectCodingStyle.detectCodingStyle([new FinalCheck()], [buildCheckFile(SAMPLE_CODING_STYLE)]); Assert.areEqual(1, detectedChecks.length); - Assert.areEqual("InlineFinal", detectedChecks[0].type); + Assert.areEqual("Final", detectedChecks[0].type); } #end