From 403d4b6504aa9d455c7114a6417b4f1b6fc48fa2 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 28 May 2017 18:18:05 +0100 Subject: [PATCH 1/3] pom.xml: upgrade checkstyle to 7.7 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 8e373c6..ea9865f 100644 --- a/pom.xml +++ b/pom.xml @@ -21,8 +21,8 @@ 2.8.2 2.17 - 7.6.1 1.23.1 + 7.7 1.10.19 3.8.0 From 6072613b44f7ac982ad5e4b8918f0774dfd9d230 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 28 May 2017 18:29:46 +0100 Subject: [PATCH 2/3] pom.xml: upgrade sevntu to 1.24.0 * Fixes bug in ForbidWildcardAsReturnType which is now enabled again * Additional pluginRepositories config is no longer required --- README.md | 67 ++++++------------- builder/src/main/resources/README-template.md | 10 +-- builder/src/main/resources/application.yml | 3 +- .../src/main/resources/rules/EnumValueName.md | 39 ++--------- plugin/pom.xml | 7 -- .../plugin/DefaultCheckstyleExecutor.java | 2 +- pom.xml | 2 +- .../net/kemitix/checkstyle-5-complexity.xml | 1 + 8 files changed, 28 insertions(+), 103 deletions(-) diff --git a/README.md b/README.md index e0c3067..9cdfee9 100644 --- a/README.md +++ b/README.md @@ -66,19 +66,11 @@ The level is now specified as a configuration parameter. See the example below. - - - - sevntu-maven - sevntu-maven - http://sevntu-checkstyle.github.io/sevntu.checkstyle/maven2 - - ```` ## All Checks -Rule|Level|Source|Enabled|Suppressable +Rule|Level|Source|Enabled|Suppressible ----|-----|------|-------|------------ [AbbreviationAsWordInName](#abbreviationaswordinname)|naming|checkstyle|Yes| [AbstractClassName](#abstractclassname)|naming|checkstyle|Yes| @@ -143,7 +135,7 @@ Rule|Level|Source|Enabled|Suppressable [ForbidInstantiation](#forbidinstantiation)|unspecified|sevntu|| [ForbidReturnInFinallyBlock](#forbidreturninfinallyblock)|complexity|sevntu|Yes| [ForbidThrowAnonymousExceptions](#forbidthrowanonymousexceptions)|tweaks|sevntu|| -[ForbidWildcardAsReturnType](#forbidwildcardasreturntype)|complexity|sevntu|| +[ForbidWildcardAsReturnType](#forbidwildcardasreturntype)|complexity|sevntu|Yes| [GenericWhitespace](#genericwhitespace)|layout|checkstyle|Yes| [Header](#header)|layout|checkstyle|Yes| [HiddenField](#hiddenfield)|tweaks|checkstyle|Yes| @@ -2264,32 +2256,13 @@ Checks that when an exception is caught, that if it is logged then it is not als Accepts `java.util.logging.Logger` and `org.slf4j.Logger`. #### [EnumValueName](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/naming/EnumValueNameCheck.html) -Enums are considered to be of two distinct types: 'Class' or 'Value' enumerations. The distinction being that Class Enumerations have methods (other than `toString()`) defined. - -The values defined in the `enum` must match the appropriate pattern: - -* Class: `^[A-Z][a-zA-Z0-9]*$` -* Value: `^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$` - -The difference being that Class enumerations can't contain underscores but can include lowercase letters (after the first initial capital). Value enumerations can include underscores, but not as the first or second character. +Checks that Enum Values match the pattern: `^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$` Valid: ```` -enum ValidConstants { +enum Valid { - ALPHA, BETA; -} - -enum ValidClassLike { - - Alpha("a"), - Beta("b"); - - private String name; - - ValidClassLike(String name) { - this.name = name; - } + ALPHA, BETA, GAMMA_RAY; } ```` @@ -2297,19 +2270,7 @@ Invalid: ```` enum InvalidConstants { - alpha, Beta, GAMMA_RAY; -} - -enum InvalidClassLike { - - alpha("a"), - beta("b"); - - private String name; - - InvalidClassLike(String name) { - this.name = name; - } + alpha, Beta; } ```` #### [ForbidCCommentsInMethods](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/ForbidCCommentsInMethodsCheck.html) @@ -2343,6 +2304,19 @@ try { return true; // invalid } ```` +#### [ForbidWildcardAsReturnType](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/design/ForbidWildcardAsReturnTypeCheck.html) + +Prevents declaring a method from returning a wildcard type as its return value. + +Valid: +```` + List getList() {} +```` + +Invalid: +```` + List getList() {} +```` #### [LogicConditionNeedOptimization](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/LogicConditionNeedOptimizationCheck.html) Prevent the placement of variables or fields after methods in an expression. @@ -2703,9 +2677,6 @@ Generic rule; doesn't embody a 'quality' check. #### [ForbidThrowAnonymousExceptions](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/ForbidThrowAnonymousExceptionsCheck.html) [IllegalThrows](#illegalthrows) performs a similar check. -#### [ForbidWildcardAsReturnType](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/design/ForbidWildcardAsReturnTypeCheck.html) - -Causes `NullPointerException` when used with `@Value.Immutables` from `org.immutables:value` #### [RequiredParameterForAnnotation](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/annotation/RequiredParameterForAnnotationCheck.html) Generic rule; doesn't embody a 'quality' check. diff --git a/builder/src/main/resources/README-template.md b/builder/src/main/resources/README-template.md index 93250af..a6f9614 100644 --- a/builder/src/main/resources/README-template.md +++ b/builder/src/main/resources/README-template.md @@ -66,19 +66,11 @@ The level is now specified as a configuration parameter. See the example below. - - - - sevntu-maven - sevntu-maven - http://sevntu-checkstyle.github.io/sevntu.checkstyle/maven2 - - ```` ## All Checks -Rule|Level|Source|Enabled|Suppressable +Rule|Level|Source|Enabled|Suppressible ----|-----|------|-------|------------ %s diff --git a/builder/src/main/resources/application.yml b/builder/src/main/resources/application.yml index 3380f3a..7519e16 100644 --- a/builder/src/main/resources/application.yml +++ b/builder/src/main/resources/application.yml @@ -1056,10 +1056,9 @@ rules: name: ForbidWildcardAsReturnType parent: TREEWALKER level: COMPLEXITY - enabled: false + enabled: true source: SEVNTU uri: http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/design/ForbidWildcardAsReturnTypeCheck.html - reason: Causes `NullPointerException` when used with `@Value.Immutables` from `org.immutables:value` - name: LogicConditionNeedOptimization parent: TREEWALKER diff --git a/builder/src/main/resources/rules/EnumValueName.md b/builder/src/main/resources/rules/EnumValueName.md index c389ed7..07e93b2 100644 --- a/builder/src/main/resources/rules/EnumValueName.md +++ b/builder/src/main/resources/rules/EnumValueName.md @@ -1,30 +1,11 @@ -Enums are considered to be of two distinct types: 'Class' or 'Value' enumerations. The distinction being that Class Enumerations have methods (other than `toString()`) defined. - -The values defined in the `enum` must match the appropriate pattern: - -* Class: `^[A-Z][a-zA-Z0-9]*$` -* Value: `^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$` - -The difference being that Class enumerations can't contain underscores but can include lowercase letters (after the first initial capital). Value enumerations can include underscores, but not as the first or second character. +Checks that Enum Values match the pattern: `^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$` Valid: ```` -enum ValidConstants { +enum Valid { - ALPHA, BETA; -} - -enum ValidClassLike { - - Alpha("a"), - Beta("b"); - - private String name; - - ValidClassLike(String name) { - this.name = name; - } + ALPHA, BETA, GAMMA_RAY; } ```` @@ -32,18 +13,6 @@ Invalid: ```` enum InvalidConstants { - alpha, Beta, GAMMA_RAY; -} - -enum InvalidClassLike { - - alpha("a"), - beta("b"); - - private String name; - - InvalidClassLike(String name) { - this.name = name; - } + alpha, Beta; } ```` diff --git a/plugin/pom.xml b/plugin/pom.xml index 270b3fd..db90e76 100644 --- a/plugin/pom.xml +++ b/plugin/pom.xml @@ -151,11 +151,4 @@ - - - sevntu-maven - sevntu-maven - http://sevntu-checkstyle.github.io/sevntu.checkstyle/maven2 - - diff --git a/plugin/src/main/java/net/kemitix/checkstyle/ruleset/plugin/DefaultCheckstyleExecutor.java b/plugin/src/main/java/net/kemitix/checkstyle/ruleset/plugin/DefaultCheckstyleExecutor.java index 24d8e7d..68f03c6 100644 --- a/plugin/src/main/java/net/kemitix/checkstyle/ruleset/plugin/DefaultCheckstyleExecutor.java +++ b/plugin/src/main/java/net/kemitix/checkstyle/ruleset/plugin/DefaultCheckstyleExecutor.java @@ -60,7 +60,7 @@ public class DefaultCheckstyleExecutor implements CheckstyleExecutor { private static final String CHECKSTYLE_ARTIFACTID = "checkstyle"; - private static final String SEVNTU_GROUPID = "com.github.sevntu.checkstyle"; + private static final String SEVNTU_GROUPID = "com.github.sevntu-checkstyle"; private static final String SEVNTU_ARTIFACTID = "sevntu-checkstyle-maven-plugin"; diff --git a/pom.xml b/pom.xml index ea9865f..f5c9d8f 100644 --- a/pom.xml +++ b/pom.xml @@ -21,8 +21,8 @@ 2.8.2 2.17 - 1.23.1 7.7 + 1.24.0 1.10.19 3.8.0 diff --git a/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml b/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml index 8c9c3fa..b258137 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml @@ -210,6 +210,7 @@ + From 4a0ee1de46ae523588f55e6becfeae131fe0b8d4 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 28 May 2017 19:56:43 +0100 Subject: [PATCH 3/3] ruleset: MoveVariableInsideIf: enabled for 4-tweaks and above --- README.md | 24 +++++++ builder/src/main/resources/application.yml | 7 ++ .../rules/MoveVariableInsideIfCheck.md | 22 +++++++ .../regressions/MoveVariableInsideIf.java | 65 +++++++++++++++++++ .../net/kemitix/checkstyle-4-tweaks.xml | 1 + .../net/kemitix/checkstyle-5-complexity.xml | 1 + 6 files changed, 120 insertions(+) create mode 100644 builder/src/main/resources/rules/MoveVariableInsideIfCheck.md create mode 100644 regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java diff --git a/README.md b/README.md index 9cdfee9..ac39463 100644 --- a/README.md +++ b/README.md @@ -181,6 +181,7 @@ Rule|Level|Source|Enabled|Suppressible [MissingSwitchDefault](#missingswitchdefault)|tweaks|checkstyle|Yes| [ModifiedControlVariable](#modifiedcontrolvariable)|tweaks|checkstyle|Yes| [ModifierOrder](#modifierorder)|naming|checkstyle|Yes| +[MoveVariableInsideIfCheck](#movevariableinsideifcheck)|tweaks|sevntu|Yes| [MultipleStringLiterals](#multiplestringliterals)|naming|checkstyle|Yes| [MultipleVariableDeclarations](#multiplevariabledeclarations)|naming|checkstyle|Yes| [MutableException](#mutableexception)|tweaks|checkstyle|Yes| @@ -2333,6 +2334,29 @@ if (getProperty() && property) {} #### [MapIterationInForEachLoop](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/MapIterationInForEachLoopCheck.html) Checks for unoptimised iterations over `Map`s. Check use of `map.values()`, `map.keySet()` and `map.entrySet()` against the use of the iterator produced to verify if another could be better. +#### [MoveVariableInsideIfCheck](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/MoveVariableInsideIfCheck.html) + +Checks if a variable is declared outside an `if` block that is only used within that block. + +Valid: +```` +if (condition) { + String variable = input.substring(1); + return method(variable); +} +return ""; +```` + +Invalid: +```` +String variable = input.substring(1); +if (condition) { + return method(variable); +} +return ""; +```` + +Example: [MoveVariableInsideIf.java](https://github.com/kemitix/kemitix-checkstyle-ruleset/blob/master/regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java) #### [NameConventionForJunit4TestClasses](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/NameConventionForJunit4TestClassesCheck.html) Checks the names of JUnit test classes. Classes checked are those that have at least one method annotated with `Test` or `org.junit.Test`. diff --git a/builder/src/main/resources/application.yml b/builder/src/main/resources/application.yml index 7519e16..3b60a5d 100644 --- a/builder/src/main/resources/application.yml +++ b/builder/src/main/resources/application.yml @@ -1498,3 +1498,10 @@ rules: source: SEVNTU uri: http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/WhitespaceBeforeArrayInitializerCheck.html reason: "TODO: enable" + - + name: MoveVariableInsideIfCheck + parent: TREEWALKER + level: TWEAKS + enabled: true + source: SEVNTU + uri: http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/MoveVariableInsideIfCheck.html diff --git a/builder/src/main/resources/rules/MoveVariableInsideIfCheck.md b/builder/src/main/resources/rules/MoveVariableInsideIfCheck.md new file mode 100644 index 0000000..e5304b3 --- /dev/null +++ b/builder/src/main/resources/rules/MoveVariableInsideIfCheck.md @@ -0,0 +1,22 @@ + +Checks if a variable is declared outside an `if` block that is only used within that block. + +Valid: +```` +if (condition) { + String variable = input.substring(1); + return method(variable); +} +return ""; +```` + +Invalid: +```` +String variable = input.substring(1); +if (condition) { + return method(variable); +} +return ""; +```` + +Example: [MoveVariableInsideIf.java](https://github.com/kemitix/kemitix-checkstyle-ruleset/blob/master/regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java) diff --git a/regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java b/regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java new file mode 100644 index 0000000..bfdb49d --- /dev/null +++ b/regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java @@ -0,0 +1,65 @@ +/** + * The MIT License (MIT) + * + * Copyright (c) 2017 Paul Campbell + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software + * and associated documentation files (the "Software"), to deal in the Software without restriction, + * including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies + * or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE + * AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +package net.kemitix.checkstyle.regressions; + +/** + * Regression demo for {@code MoveVariableInsideIfCheck}. + * + * @author Paul Campbell (pcampbell@kemitix.net) + */ +class MoveVariableInsideIf { + + private String input = "1"; + + private boolean condition; + + private String method(final String variable) { + return "value"; + } + + /** + * Fails if not suppressed. + * + * @return value + */ + @SuppressWarnings("movevariableinsideif") + String invalid() { + String variable = input.substring(1); + if (condition) { + return method(variable); + } + return ""; + } + + /** + * Rewrite {@link #invalid()} as this to pass. + * + * @return value + */ + String valid() { + if (condition) { + String variable = input.substring(1); + return method(variable); + } + return ""; + } +} diff --git a/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml b/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml index 50b1a88..2e27c90 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml @@ -175,6 +175,7 @@ + diff --git a/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml b/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml index b258137..bc2b68b 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml @@ -228,6 +228,7 @@ +