Merge pull request #80 from kemitix/match-pmd-ruleset

Update ruleset to match kemitix-pmd-ruleset criteria

.mvn/wrapper/maven-wrapper.properties

@@ -1 +1 @@
-distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.5.0/apache-maven-3.5.0-bin.zip
+distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.5.3/apache-maven-3.5.3-bin.zip

Jenkinsfile.groovy

@@ -48,7 +48,8 @@ pipeline {
         stage('Build Java Next') {
             steps {
                 withMaven(maven: 'maven', jdk: 'JDK Next') {
-                    sh "${mvn} clean install"
+                    sh "${mvn} clean install -Djava.version=9 -Dmaven-enforcer-plugin.version=3.0.0-M1"
+                    //TODO: check that git status is still clean - i.e. builder didn't update any rulesets
                 }
             }
         }
@@ -56,6 +57,7 @@ pipeline {
             steps {
                 withMaven(maven: 'maven', jdk: 'JDK LTS') {
                     sh "${mvn} clean install"
+                    //TODO: check that git status is still clean - i.e. builder didn't update any rulesets
                 }
             }
         }

builder/pom.xml

@@ -31,6 +31,7 @@
         <map-builder.version>1.0.0</map-builder.version>
         <mockito.version>2.13.0</mockito.version>
         <assertj.version>3.9.0</assertj.version>
+        <conditional.version>0.3.0</conditional.version>
         <fast-classpath-scanner.version>2.18.1</fast-classpath-scanner.version>
 
         <maven-checkstyle-plugin.version>3.0.0</maven-checkstyle-plugin.version>
@@ -59,6 +60,11 @@
             <version>${lombok.version}</version>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+            <groupId>net.kemitix</groupId>
+            <artifactId>conditional</artifactId>
+            <version>${conditional.version}</version>
+        </dependency>
         <dependency>
             <groupId>io.github.lukehutch</groupId>
             <artifactId>fast-classpath-scanner</artifactId>

builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleWriter.java

@@ -21,21 +21,22 @@
 
 package net.kemitix.checkstyle.ruleset.builder;
 
-import com.speedment.common.mapstream.MapStream;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
-import lombok.val;
 import org.springframework.boot.CommandLineRunner;
 import org.springframework.stereotype.Component;
 
+import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
-import java.util.Arrays;
+import java.nio.file.Path;
-import java.util.Map;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static java.util.Arrays.asList;
+
 /**
  * Writes the Checkstyle XML files.
  *
@@ -46,8 +47,6 @@ import java.util.stream.Stream;
 @RequiredArgsConstructor
 class CheckstyleWriter implements CommandLineRunner {
 
-    private static final String NEWLINE = System.getProperty("line.separator");
-
     private final OutputProperties outputProperties;
 
     private final TemplateProperties templateProperties;
@@ -56,62 +55,79 @@ class CheckstyleWriter implements CommandLineRunner {
 
     private final RuleClassLocator ruleClassLocator;
 
+    private static Predicate<RuleLevel> excludeUnspecifiedRuleLevel() {
+        return level -> !level.equals(RuleLevel.UNSPECIFIED);
+    }
+
+    private static void writeRuleset(
+            final Path filePath,
+            final String content
+                                    ) throws IOException {
+        Files.write(filePath, asList(content.split(System.lineSeparator())), StandardCharsets.UTF_8);
+    }
+
+    private static String ruleset(
+            final String checkerRules,
+            final String treeWalkerRules,
+            final String template
+                                 ) {
+        return String.format(template, checkerRules, treeWalkerRules);
+    }
+
+    private static boolean fileExists(final File file) {
+        return file.exists();
+    }
+
+    private static void writeCheckstyleFileWithException(
+            final Path outputPath,
+            final String checkerRules,
+            final String treeWalkerRules,
+            final Path template
+                                                        ) throws IOException {
+        if (fileExists(template.toFile())) {
+            log.info("Writing ruleset: {}", outputPath);
+            final String xmlTemplate = new String(Files.readAllBytes(template), StandardCharsets.UTF_8);
+            writeRuleset(outputPath, ruleset(checkerRules, treeWalkerRules, xmlTemplate));
+        } else {
+            throw new TemplateNotFoundException(template);
+        }
+    }
+
     @Override
     public void run(final String... args) {
         Stream.of(RuleLevel.values())
-              .filter(level -> !level.equals(RuleLevel.UNSPECIFIED))
+                .filter(excludeUnspecifiedRuleLevel())
-              .forEach(this::writeCheckstyleFile);
+                .forEach(this::writeCheckstyleFile);
     }
 
     private void writeCheckstyleFile(final RuleLevel ruleLevel) {
-        val xmlFile = outputProperties.getDirectory()
+        final Path outputPath = outputPath(ruleLevel);
-                                      .resolve(outputProperties.getRulesetFiles()
+        final String checkerRules = enabledRules(ruleLevel, RuleParent.CHECKER);
-                                                               .get(ruleLevel));
+        final String treeWalkerRules = enabledRules(ruleLevel, RuleParent.TREEWALKER);
-        val checkerRules = rulesProperties.getRules()
+        final Path template = templateProperties.getCheckstyleXml();
-                                          .stream()
-                                          .filter(Rule::isEnabled)
-                                          .filter(rule -> RuleParent.CHECKER.equals(rule.getParent()))
-                                          .filter(rule -> ruleLevel.compareTo(rule.getLevel()) >= 0)
-                                          .map(this::formatRuleAsModule)
-                                          .collect(Collectors.joining(NEWLINE));
-        val treeWalkerRules = rulesProperties.getRules()
-                                             .stream()
-                                             .filter(Rule::isEnabled)
-                                             .filter(rule -> RuleParent.TREEWALKER.equals(rule.getParent()))
-                                             .filter(rule -> ruleLevel.compareTo(rule.getLevel()) >= 0)
-                                             .map(this::formatRuleAsModule)
-                                             .collect(Collectors.joining(NEWLINE));
         try {
-            val checkstyleXmlTemplate = templateProperties.getCheckstyleXml();
+            writeCheckstyleFileWithException(outputPath, checkerRules, treeWalkerRules, template);
-            if (checkstyleXmlTemplate.toFile()
-                                     .exists()) {
-                val bytes = Files.readAllBytes(checkstyleXmlTemplate);
-                val template = new String(bytes, StandardCharsets.UTF_8);
-                val output = Arrays.asList(String.format(template, checkerRules, treeWalkerRules)
-                                                 .split(NEWLINE));
-                log.info("Writing xmlFile: {}", xmlFile);
-                Files.write(xmlFile, output, StandardCharsets.UTF_8);
-            } else {
-                throw new TemplateNotFoundException(checkstyleXmlTemplate);
-            }
         } catch (IOException e) {
             throw new CheckstyleWriterException(e);
         }
     }
 
-    private String formatRuleAsModule(final Rule rule) {
+    private Path outputPath(final RuleLevel ruleLevel) {
-        if (rule.getProperties()
+        return outputProperties.getDirectory()
-                .isEmpty()) {
+                .resolve(outputProperties.getRulesetFiles()
-            return String.format("<module name=\"%s\"/>", ruleClassLocator.apply(rule));
+                                 .get(ruleLevel));
-        }
-        return String.format("<module name=\"%s\">%n    %s%n</module>", ruleClassLocator.apply(rule),
-                             formatProperties(rule.getProperties())
-                            );
     }
 
-    private String formatProperties(final Map<String, String> properties) {
+    private String enabledRules(
-        return MapStream.of(properties)
+            final RuleLevel ruleLevel,
-                        .map((k, v) -> String.format("<property name=\"%s\" value=\"%s\"/>", k, v))
+            final RuleParent ruleParent
-                        .collect(Collectors.joining(NEWLINE));
+                               ) {
+        return rulesProperties.getRules()
+                .stream()
+                .filter(Rule::isEnabled)
+                .filter(Rule.hasParent(ruleParent))
+                .filter(Rule.isIncludedInLevel(ruleLevel))
+                .map(rule -> Rule.asModule(ruleClassLocator.apply(rule), rule))
+                .collect(Collectors.joining(System.lineSeparator()));
     }
 }

builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/Rule.java

@@ -21,13 +21,17 @@
 
 package net.kemitix.checkstyle.ruleset.builder;
 
+import com.speedment.common.mapstream.MapStream;
 import lombok.Getter;
 import lombok.Setter;
+import net.kemitix.conditional.Value;
 
 import java.net.URI;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 /**
  * A single Checkstyle Check.
@@ -40,6 +44,10 @@ public class Rule {
 
     private static final Locale LOCALE = Locale.ENGLISH;
 
+    private static final String MODULE_NO_PROPERTIES = "<module name=\"%s\"/>";
+
+    private static final String MODULE_WITH_PROPERTIES = "<module name=\"%s\">%n    %s%n</module>";
+
     /**
      * Configuration properties.
      */
@@ -100,6 +108,69 @@ public class Rule {
                    .compareTo(right.getLowerCaseRuleName());
     }
 
+    /**
+     * Predicate to check that the Rule has the given RuleParent.
+     *
+     * @param ruleParent the RuleParent to match
+     *
+     * @return a Predicate to check a Rule
+     */
+    static Predicate<Rule> hasParent(final RuleParent ruleParent) {
+        return rule -> ruleParent.equals(rule.getParent());
+    }
+
+
+    /**
+     * Predicate to check that the Rule is included in the given RuleLevel.
+     *
+     * @param ruleLevel the RuleLevel to match
+     *
+     * @return a Predicate to check a Rule
+     */
+    static Predicate<Rule> isIncludedInLevel(final RuleLevel ruleLevel) {
+        return rule -> ruleLevel.compareTo(rule.getLevel()) >= 0;
+    }
+
+    private static String formatProperties(final Map<String, String> properties) {
+        return MapStream.of(properties)
+                .map((k, v) -> String.format("<property name=\"%s\" value=\"%s\"/>", k, v))
+                .collect(Collectors.joining(System.lineSeparator()));
+    }
+
+    private static boolean hasProperties(final Rule rule) {
+        return !rule.getProperties().isEmpty();
+    }
+
+    /**
+     * Formats the Rule as an XML module fragment.
+     *
+     * @param rule the Rule to format
+     * @param ruleClassname the classname for the Rule
+     *
+     * @return an XML {@code <module/>} fragment
+     */
+    static String asModule(
+            final String ruleClassname,
+            final Rule rule
+                          ) {
+        return Value.<String>where(hasProperties(rule))
+                .then(() -> moduleWithParameters(rule, ruleClassname))
+                .otherwise(() -> moduleNoParameters(ruleClassname));
+    }
+
+    private static String moduleNoParameters(
+            final String ruleClassname
+                                            ) {
+        return String.format(MODULE_NO_PROPERTIES, ruleClassname);
+    }
+
+    private static String moduleWithParameters(
+            final Rule rule,
+            final String ruleClassname
+                                              ) {
+        return String.format(MODULE_WITH_PROPERTIES, ruleClassname, formatProperties(rule.getProperties()));
+    }
+
     private String getLowerCaseRuleName() {
         return getName().toLowerCase(LOCALE);
     }

builder/src/main/resources/application.yml

@@ -296,6 +296,8 @@ rules:
         enabled: true
         source: CHECKSTYLE
         uri: http://checkstyle.sourceforge.net/config_sizes.html#FileLength
+        properties:
+          max: 500
     -
         name: FileTabCharacter
         parent: CHECKER
@@ -463,9 +465,9 @@ rules:
         source: CHECKSTYLE
         uri: http://checkstyle.sourceforge.net/config_metrics.html#JavaNCSS
         properties:
-            classMaximum: 1200
+            classMaximum: 250
-            fileMaximum: 1600
+            fileMaximum: 250
-            methodMaximum: 40
+            methodMaximum: 12
     -
         name: LeftCurly
         parent: TREEWALKER
@@ -518,7 +520,7 @@ rules:
         source: CHECKSTYLE
         uri: http://checkstyle.sourceforge.net/config_sizes.html#MethodCount
         properties:
-            maxTotal: 30
+            maxTotal: 10
     -
         name: MethodLength
         parent: TREEWALKER
@@ -527,7 +529,7 @@ rules:
         source: CHECKSTYLE
         uri: http://checkstyle.sourceforge.net/config_sizes.html#MethodLength
         properties:
-            max: 40
+            max: 30
     -
         name: MethodName
         parent: TREEWALKER

ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml

@@ -8,7 +8,9 @@
     <property name="fileNamePattern" value="(.sync-conflict-| conflicted copy )"/>
 <property name="match" value="true"/>
 </module>
-<module name="com.puppycrawl.tools.checkstyle.checks.sizes.FileLengthCheck"/>
+<module name="com.puppycrawl.tools.checkstyle.checks.sizes.FileLengthCheck">
+    <property name="max" value="500"/>
+</module>
 <module name="com.puppycrawl.tools.checkstyle.checks.whitespace.FileTabCharacterCheck"/>
 <module name="com.puppycrawl.tools.checkstyle.checks.header.HeaderCheck">
     <property name="fileExtensions" value="java"/>
@@ -97,9 +99,9 @@
 <module name="com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocStyleCheck"/>
 <module name="com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTypeCheck"/>
 <module name="com.puppycrawl.tools.checkstyle.checks.metrics.JavaNCSSCheck">
-    <property name="classMaximum" value="1200"/>
+    <property name="classMaximum" value="250"/>
-<property name="fileMaximum" value="1600"/>
+<property name="fileMaximum" value="250"/>
-<property name="methodMaximum" value="40"/>
+<property name="methodMaximum" value="12"/>
 </module>
 <module name="com.puppycrawl.tools.checkstyle.checks.blocks.LeftCurlyCheck"/>
 <module name="com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck">
@@ -110,10 +112,10 @@
 <module name="com.puppycrawl.tools.checkstyle.checks.coding.MagicNumberCheck"/>
 <module name="com.puppycrawl.tools.checkstyle.checks.naming.MemberNameCheck"/>
 <module name="com.puppycrawl.tools.checkstyle.checks.sizes.MethodCountCheck">
-    <property name="maxTotal" value="30"/>
+    <property name="maxTotal" value="10"/>
 </module>
 <module name="com.puppycrawl.tools.checkstyle.checks.sizes.MethodLengthCheck">
-    <property name="max" value="40"/>
+    <property name="max" value="30"/>
 </module>
 <module name="com.puppycrawl.tools.checkstyle.checks.naming.MethodNameCheck"/>
 <module name="com.puppycrawl.tools.checkstyle.checks.whitespace.MethodParamPadCheck"/>