From 8f5a12d7af771714bc58fd6361c7a355daf9108e Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 6 Nov 2018 20:40:44 +0000 Subject: [PATCH] Reduce some overly-strict requirements and fix some typos (#141) * Disable JavadocMethod * Disable Header * Fix example for ReturnBooleanFromTernary check * Disable SimeplAccessorNameNotation * Fix typo in UselessSingleCatch * Fix typo in UselessSuperCtorCall --- README.md | 38 +++++++++---------- builder/src/main/resources/application.yml | 9 +++-- .../rules/ReturnBooleanFromTernary.md | 8 ++-- .../resources/rules/UselessSingleCatch.md | 2 +- .../resources/rules/UselessSuperCtorCall.md | 4 +- .../net/kemitix/checkstyle-1-layout.xml | 4 -- .../net/kemitix/checkstyle-2-naming.xml | 5 --- .../net/kemitix/checkstyle-3-javadoc.xml | 10 ----- .../net/kemitix/checkstyle-4-tweaks.xml | 10 ----- .../net/kemitix/checkstyle-5-complexity.xml | 10 ----- 10 files changed, 32 insertions(+), 68 deletions(-) diff --git a/README.md b/README.md index 670d4c1..3aa141b 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ The simplest way to use the ruleset is with the maven-tile: true - net.kemitix.checkstyle:tile:RELEASE + net.kemitix.checkstyle:tile:DEV-SNAPSHOT @@ -155,7 +155,7 @@ Rule|Level|Source|Enabled|Suppressible [ForbidThrowAnonymousExceptions](#forbidthrowanonymousexceptions)|tweaks|sevntu|| [ForbidWildcardAsReturnType](#forbidwildcardasreturntype)|complexity|sevntu|Yes| [GenericWhitespace](#genericwhitespace)|layout|checkstyle|Yes| -[Header](#header)|layout|checkstyle|Yes| +[Header](#header)|layout|checkstyle|| [HiddenField](#hiddenfield)|tweaks|checkstyle|Yes| [HideUtilityClassConstructor](#hideutilityclassconstructor)|tweaks|checkstyle|Yes| [IllegalCatch](#illegalcatch)|tweaks|checkstyle|Yes| @@ -173,7 +173,7 @@ Rule|Level|Source|Enabled|Suppressible [InterfaceIsType](#interfaceistype)|complexity|checkstyle|Yes| [InterfaceMemberImpliedModifier](#interfacememberimpliedmodifier)|tweaks|checkstyle|Yes| [InterfaceTypeParameterName](#interfacetypeparametername)|naming|checkstyle|Yes| -[JavadocMethod](#javadocmethod)|javadoc|checkstyle|Yes| +[JavadocMethod](#javadocmethod)|javadoc|checkstyle|| [JavadocPackage](#javadocpackage)|javadoc|checkstyle|Yes| [JavadocParagraph](#javadocparagraph)|javadoc|checkstyle|Yes| [JavadocStyle](#javadocstyle)|javadoc|checkstyle|Yes| @@ -253,7 +253,7 @@ Rule|Level|Source|Enabled|Suppressible [ReturnNullInsteadOfBoolean](#returnnullinsteadofboolean)|tweaks|sevntu|Yes| [RightCurly](#rightcurly)|layout|checkstyle|Yes| [SeparatorWrap](#separatorwrap)|layout|checkstyle|Yes| -[SimpleAccessorNameNotation](#simpleaccessornamenotation)|naming|sevntu|Yes| +[SimpleAccessorNameNotation](#simpleaccessornamenotation)|naming|sevntu|| [SimplifyBooleanExpression](#simplifybooleanexpression)|complexity|checkstyle|Yes| [SimplifyBooleanReturn](#simplifybooleanreturn)|complexity|checkstyle|Yes| [SingleBreakOrContinue](#singlebreakorcontinue)|tweaks|sevntu|Yes| @@ -957,9 +957,6 @@ Pair p1 = new Pair<>(1, "apple"); List list = ImmutableList.Builder::new; sort(list, Comparable::compareTo); ```` -#### [Header](http://checkstyle.sourceforge.net/config_header.html#Header) - -Checks that all `*.java` source files begin with the contents of the `LICENSE.txt` file. #### [HiddenField](http://checkstyle.sourceforge.net/config_coding.html#HiddenField) Checks that a local variable or parameter in a method doesn't have the same name as a field. Doesn't apply in constructors or setters. @@ -1159,9 +1156,6 @@ Invalid: ```` interface Portable {} ```` -#### [JavadocMethod](http://checkstyle.sourceforge.net/config_javadoc.html#JavadocMethod) - -Checks that all public, protected and package methods have a Javadoc block, that all `@throws` tags are used. Basic setters and getters do not require javadoc. #### [JavadocPackage](http://checkstyle.sourceforge.net/config_javadoc.html#JavadocPackage) Checks that each package has a `package-info.java` file. @@ -2498,14 +2492,14 @@ Ternary statements shouldn't have `Boolean` values as results. Valid: ```` -Boolean set = isSet() ? True : False; -Boolean notReady = isReady() ? False : True; +Boolean set = isSet(); +Boolean notReady = !isReady(); ```` Invalid: ```` -Boolean set = isSet(); -Boolean notReady = !isReady(); +Boolean set = isSet() ? True : False; +Boolean notReady = isReady() ? False : True; ```` #### [ReturnNullInsteadOfBoolean](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/ReturnNullInsteadOfBooleanCheck.html) @@ -2523,9 +2517,6 @@ Boolean isEnabled() { return null; } ```` -#### [SimpleAccessorNameNotation](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/SimpleAccessorNameNotationCheck.html) - -Checks that setters and getters follow the normal setField(), getField() and isField() pattern, where 'Field' is the name of the field being accessed. #### [SingleBreakOrContinue](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/SingleBreakOrContinueCheck.html) Checks that there is at most one `continue` or `break` statement within a looping block (e.g. `for`, `while`, ...) @@ -2560,7 +2551,7 @@ public enum EnumThree { ```` #### [UselessSingleCatch](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/UselessSingleCatchCheck.html) -Checks for catch blocks that are useless. i.e. that catch al exceptions and then just rethrow them. +Checks for catch blocks that are useless. i.e. that catch all exceptions and then just rethrow them. Invalid: ```` @@ -2572,7 +2563,7 @@ try { ```` #### [UselessSuperCtorCall](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/UselessSuperCtorCallCheck.html) -Checks for useless calls the the `super()` method in constructors. +Checks for useless calls to the `super()` method in constructors. Invalid: ```` @@ -2628,6 +2619,9 @@ Recommends using a static import to access constants from another class over inh #### [FinalLocalVariable](http://checkstyle.sourceforge.net/config_coding.html#FinalLocalVariable) Doesn't recognise Lombok's `val` as being `final`. +#### [Header](http://checkstyle.sourceforge.net/config_header.html#Header) + +Shouldn't need to list in every file, simply listing in project root should be enough. #### [IllegalInstantiation](http://checkstyle.sourceforge.net/config_coding.html#IllegalInstantiation) Not really suitable for a template ruleset as it requires an explicit list of classes to apply to. @@ -2643,6 +2637,9 @@ Generic rule; doesn't embody a 'quality' check. #### [Indentation](http://checkstyle.sourceforge.net/config_misc.html#Indentation) Couldn't get my IDE's (IntelliJ) code style to match. +#### [JavadocMethod](http://checkstyle.sourceforge.net/config_javadoc.html#JavadocMethod) + +Only exceptional cases should need to be documented. #### [JavadocTagContinuationIndentation](http://checkstyle.sourceforge.net/config_javadoc.html#JavadocTagContinuationIndentation) Couldn't get my IDE's (IntelliJ) code style to match. @@ -2735,6 +2732,9 @@ Generic rule; doesn't embody a 'quality' check. #### [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. +#### [SimpleAccessorNameNotation](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/SimpleAccessorNameNotationCheck.html) + +allow use of non-bean property-like naming #### [StaticMethodCandidate](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/design/StaticMethodCandidateCheck.html) Can't handle private methods called by reflection, which may cause issues with Spring and other DI frameworks. diff --git a/builder/src/main/resources/application.yml b/builder/src/main/resources/application.yml index 36898b1..0de238c 100644 --- a/builder/src/main/resources/application.yml +++ b/builder/src/main/resources/application.yml @@ -331,12 +331,13 @@ rules: name: Header parent: CHECKER level: LAYOUT - enabled: true + enabled: false source: CHECKSTYLE uri: http://checkstyle.sourceforge.net/config_header.html#Header properties: fileExtensions: java headerFile: LICENSE.txt + reason: Shouldn't need to list in every file, simply listing in project root should be enough. - name: HiddenField parent: TREEWALKER @@ -430,13 +431,14 @@ rules: name: JavadocMethod parent: TREEWALKER level: JAVADOC - enabled: true + enabled: false source: CHECKSTYLE uri: http://checkstyle.sourceforge.net/config_javadoc.html#JavadocMethod properties: allowMissingPropertyJavadoc: true validateThrows: true scope: package + reason: Only exceptional cases should need to be documented. - name: JavadocPackage parent: CHECKER @@ -1163,9 +1165,10 @@ rules: name: SimpleAccessorNameNotation parent: TREEWALKER level: NAMING - enabled: true + enabled: false source: SEVNTU uri: http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/SimpleAccessorNameNotationCheck.html + reason: allow use of non-bean property-like naming - name: SingleBreakOrContinue parent: TREEWALKER diff --git a/builder/src/main/resources/rules/ReturnBooleanFromTernary.md b/builder/src/main/resources/rules/ReturnBooleanFromTernary.md index 2f7f7fd..b976a9d 100644 --- a/builder/src/main/resources/rules/ReturnBooleanFromTernary.md +++ b/builder/src/main/resources/rules/ReturnBooleanFromTernary.md @@ -3,12 +3,12 @@ Ternary statements shouldn't have `Boolean` values as results. Valid: ```` -Boolean set = isSet() ? True : False; -Boolean notReady = isReady() ? False : True; +Boolean set = isSet(); +Boolean notReady = !isReady(); ```` Invalid: ```` -Boolean set = isSet(); -Boolean notReady = !isReady(); +Boolean set = isSet() ? True : False; +Boolean notReady = isReady() ? False : True; ```` diff --git a/builder/src/main/resources/rules/UselessSingleCatch.md b/builder/src/main/resources/rules/UselessSingleCatch.md index 671e33e..76020c6 100644 --- a/builder/src/main/resources/rules/UselessSingleCatch.md +++ b/builder/src/main/resources/rules/UselessSingleCatch.md @@ -1,5 +1,5 @@ -Checks for catch blocks that are useless. i.e. that catch al exceptions and then just rethrow them. +Checks for catch blocks that are useless. i.e. that catch all exceptions and then just rethrow them. Invalid: ```` diff --git a/builder/src/main/resources/rules/UselessSuperCtorCall.md b/builder/src/main/resources/rules/UselessSuperCtorCall.md index 7435aa9..085ee65 100644 --- a/builder/src/main/resources/rules/UselessSuperCtorCall.md +++ b/builder/src/main/resources/rules/UselessSuperCtorCall.md @@ -1,5 +1,5 @@ -Checks for useless calls the the `super()` method in constructors. +Checks for useless calls to the `super()` method in constructors. Invalid: ```` @@ -13,4 +13,4 @@ class Derived extends Base { super(); } } -```` \ No newline at end of file +```` diff --git a/ruleset/src/main/resources/net/kemitix/checkstyle-1-layout.xml b/ruleset/src/main/resources/net/kemitix/checkstyle-1-layout.xml index 8dffd66..354e0a0 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-1-layout.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-1-layout.xml @@ -9,10 +9,6 @@ - - - - diff --git a/ruleset/src/main/resources/net/kemitix/checkstyle-2-naming.xml b/ruleset/src/main/resources/net/kemitix/checkstyle-2-naming.xml index 9630c17..04c8f8d 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-2-naming.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-2-naming.xml @@ -9,10 +9,6 @@ - - - - @@ -88,7 +84,6 @@ - diff --git a/ruleset/src/main/resources/net/kemitix/checkstyle-3-javadoc.xml b/ruleset/src/main/resources/net/kemitix/checkstyle-3-javadoc.xml index e5dc0ca..d683062 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-3-javadoc.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-3-javadoc.xml @@ -9,10 +9,6 @@ - - - - @@ -42,11 +38,6 @@ - - - - - @@ -112,7 +103,6 @@ - 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 112a51f..038f23a 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml @@ -9,10 +9,6 @@ - - - - @@ -70,11 +66,6 @@ - - - - - @@ -168,7 +159,6 @@ - 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 e64ef01..cb8d6ba 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml @@ -12,10 +12,6 @@ - - - - @@ -90,11 +86,6 @@ - - - - - @@ -220,7 +211,6 @@ -