From c14c9ea8cac43f762110f1282542017af859140a Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sat, 27 May 2017 14:04:19 +0100 Subject: [PATCH] KCR22: builder: sort rules alphabetically --- README.md | 44 +++++++++---------- .../builder/DefaultReadmeIndexBuilder.java | 8 +--- .../ruleset/builder/ReadmeWriter.java | 1 + .../checkstyle/ruleset/builder/Rule.java | 23 ++++++++++ .../ruleset/builder/ReadmeWriterTest.java | 33 +++++++++++--- .../plugin/CheckConfigurationTest.java | 1 - 6 files changed, 75 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index a6fd4a2..e0c3067 100644 --- a/README.md +++ b/README.md @@ -285,22 +285,6 @@ The following is a list of each of the checks and the expectations each has on y Rules are listed in alphabetical order. -#### [RegexpOnFilename](http://checkstyle.sourceforge.net/config_regexp.html#RegexpOnFilename) - -Checks for the existence of forbidden java file names. - -File names are forbidden if they match the pattern `(.sync-conflict-| conflicted copy )`. - -N.B. only `*.java` files are checked. - -This check is intended to detect Syncthing and Dropbox conflict files. - -e.g. -```` -DataClass (Bob's conflicted copy 2017-03-11).java -DataClass.sync-conflict-20170311-1648.java -```` - #### [AbbreviationAsWordInName](http://checkstyle.sourceforge.net/config_naming.html#AbbreviationAsWordInName) Enforces proper `CamelCase` and avoids sequences of consecutive uppercase characters in identifiers. Does not apply to @Overridden methods. @@ -1759,6 +1743,22 @@ Checks for redundant modifiers. Checks for: * Inner interface declarations that are declared as static. * Class constructors. * Nested enum definitions that are declared as static. +#### [RegexpOnFilename](http://checkstyle.sourceforge.net/config_regexp.html#RegexpOnFilename) + +Checks for the existence of forbidden java file names. + +File names are forbidden if they match the pattern `(.sync-conflict-| conflicted copy )`. + +N.B. only `*.java` files are checked. + +This check is intended to detect Syncthing and Dropbox conflict files. + +e.g. +```` +DataClass (Bob's conflicted copy 2017-03-11).java +DataClass.sync-conflict-20170311-1648.java +```` + #### [RequireThis](http://checkstyle.sourceforge.net/config_coding.html#RequireThis) Checks that references to instance fields where a parameter name overlaps are qualified by `this.`. @@ -1950,12 +1950,12 @@ Prevents the use of `@SuppressWarnings` for the following checks: * [PackageDeclaration](#packagedeclaration) * [TypeName](#typename) * [VisibilityModifier](#visibilitymodifier) -#### [SuppressWarningsHolder](http://checkstyle.sourceforge.net/config_annotation.html#SuppressWarningsHolder) - -Used by Checkstyle to hold the checks to be suppressed from `@SuppressWarnings(...)` annotations. #### [SuppressWarningsFilter](http://checkstyle.sourceforge.net/config_annotation.html#SuppressWarningsFilter) Allows the use of the `@SuppressWarnings` annotation. +#### [SuppressWarningsHolder](http://checkstyle.sourceforge.net/config_annotation.html#SuppressWarningsHolder) + +Used by Checkstyle to hold the checks to be suppressed from `@SuppressWarnings(...)` annotations. #### [ThrowsCount](http://checkstyle.sourceforge.net/config_design.html#ThrowsCount) Restricts non-private methods to only `throws` 4 distinct Exception types. Exceptions should be hierarchical to allow catching suitable root Exceptions. @@ -2667,9 +2667,6 @@ Generic rule; doesn't embody a 'quality' check. As the sevntu check are considered experimental not all those that are not enabled are listed here. Only where they are disabled due to a conflict with my 'style' or there is another irreconcilable difference that prevents them from being enabled, will they be documented to prevent repeated investigations. -#### [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` #### [AvoidConditionInversion](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/design/AvoidConditionInversionCheck.html) Should already be covered by [SimplifyBooleanExpression](simplifybooleanexpression). @@ -2706,6 +2703,9 @@ 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/java/net/kemitix/checkstyle/ruleset/builder/DefaultReadmeIndexBuilder.java b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/DefaultReadmeIndexBuilder.java index 588f22c..fb7cad3 100644 --- a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/DefaultReadmeIndexBuilder.java +++ b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/DefaultReadmeIndexBuilder.java @@ -24,10 +24,8 @@ package net.kemitix.checkstyle.ruleset.builder; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Component; -import java.util.Comparator; import java.util.Locale; import java.util.Optional; -import java.util.function.Function; import java.util.stream.Collectors; /** @@ -49,15 +47,11 @@ public class DefaultReadmeIndexBuilder implements ReadmeIndexBuilder { public final String build() { return rulesProperties.getRules() .stream() - .sorted(Comparator.comparing(lowerCaseRuleName())) + .sorted(Rule::sortByName) .map(this::formatRuleRow) .collect(Collectors.joining(NEWLINE)); } - private Function lowerCaseRuleName() { - return this::getLowerCaseRuleName; - } - private String getLowerCaseRuleName(final Rule rule) { return rule.getName() .toLowerCase(LOCALE); diff --git a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/ReadmeWriter.java b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/ReadmeWriter.java index 09efffd..12d1c41 100644 --- a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/ReadmeWriter.java +++ b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/ReadmeWriter.java @@ -88,6 +88,7 @@ class ReadmeWriter implements CommandLineRunner { return rulesProperties.getRules() .stream() .filter(predicate) + .sorted(Rule::sortByName) .flatMap(ruleReadmeLoader::load) .collect(Collectors.joining(NEWLINE)); } diff --git a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/Rule.java b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/Rule.java index d408913..8e8055b 100644 --- a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/Rule.java +++ b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/Rule.java @@ -26,6 +26,7 @@ import lombok.Setter; import java.net.URI; import java.util.HashMap; +import java.util.Locale; import java.util.Map; /** @@ -37,6 +38,8 @@ import java.util.Map; @Getter public class Rule { + private static final Locale LOCALE = Locale.ENGLISH; + /** * The name of the rule's Check class. */ @@ -81,4 +84,24 @@ public class Rule { * Configuration properties. */ private final Map properties = new HashMap<>(); + + /** + * Compare two Rules lexicographically for sorting by rule name, ignoring case. + * + * @param left the first rule + * @param right the second rule + * + * @return the value 0 if the rules are equal; a value less than 0 if the left string is lexicographically less than + * the right string; and a value greater than 0 if the left string is lexicographically greater than the right + * string. + */ + static int sortByName(final Rule left, final Rule right) { + return left.getLowerCaseRuleName() + .compareTo(right.getLowerCaseRuleName()); + } + + private String getLowerCaseRuleName() { + return getName().toLowerCase(LOCALE); + } + } diff --git a/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/ReadmeWriterTest.java b/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/ReadmeWriterTest.java index f64ab97..bc45598 100644 --- a/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/ReadmeWriterTest.java +++ b/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/ReadmeWriterTest.java @@ -73,10 +73,10 @@ public class ReadmeWriterTest { "sd:sevntu-disabled" ); val rules = rulesProperties.getRules(); - final Rule checkstyleEnabled = rule(RuleSource.CHECKSTYLE, true); - final Rule checkstyleDisabled = rule(RuleSource.CHECKSTYLE, false); - final Rule sevntuEnabled = rule(RuleSource.SEVNTU, true); - final Rule sevntuDisabled = rule(RuleSource.SEVNTU, false); + final Rule checkstyleEnabled = rule(RuleSource.CHECKSTYLE, true, "checkstyle enabled"); + final Rule checkstyleDisabled = rule(RuleSource.CHECKSTYLE, false, "checkstyle disabled"); + final Rule sevntuEnabled = rule(RuleSource.SEVNTU, true, "sevntu enabled"); + final Rule sevntuDisabled = rule(RuleSource.SEVNTU, false, "sevntu disabled"); rules.add(checkstyleEnabled); rules.add(checkstyleDisabled); rules.add(sevntuEnabled); @@ -93,10 +93,33 @@ public class ReadmeWriterTest { assertThat(lines).containsExactlyElementsOf(expected); } - private Rule rule(final RuleSource source, final boolean enabled) { + private Rule rule(final RuleSource source, final boolean enabled, final String name) { val rule = new Rule(); + rule.setName(name); rule.setSource(source); rule.setEnabled(enabled); return rule; } + + @Test + public void rulesAreAlphabetical() throws Exception { + //given + val expected = Arrays.asList("ce:alpha", "beta", "delta"); + val rules = rulesProperties.getRules(); + final Rule alpha = rule(RuleSource.CHECKSTYLE, true, "alpha"); + final Rule beta = rule(RuleSource.CHECKSTYLE, true, "beta"); + final Rule delta = rule(RuleSource.CHECKSTYLE, true, "delta"); + rules.add(alpha); + rules.add(delta); + rules.add(beta); + given(indexBuilder.build()).willReturn("index"); + given(ruleReadmeLoader.load(alpha)).willReturn(Stream.of(alpha.getName())); + given(ruleReadmeLoader.load(beta)).willReturn(Stream.of(beta.getName())); + given(ruleReadmeLoader.load(delta)).willReturn(Stream.of(delta.getName())); + //when + readmeWriter.run(); + //then + final Stream lines = Files.lines(readme, StandardCharsets.UTF_8); + assertThat(lines).containsSequence(expected); + } } diff --git a/plugin/src/test/java/net/kemitix/checkstyle/ruleset/plugin/CheckConfigurationTest.java b/plugin/src/test/java/net/kemitix/checkstyle/ruleset/plugin/CheckConfigurationTest.java index 3fb2448..15c63ac 100644 --- a/plugin/src/test/java/net/kemitix/checkstyle/ruleset/plugin/CheckConfigurationTest.java +++ b/plugin/src/test/java/net/kemitix/checkstyle/ruleset/plugin/CheckConfigurationTest.java @@ -20,7 +20,6 @@ public class CheckConfigurationTest { .sourceDirectory(sourceDirectory) .toString(); //then - System.out.println("result = " + result); assertThat(result).contains("rulesetVersion=ruleset version") .contains("sourceDirectory=source directory"); }