From a0ca4958fc0256608fd5fdc9af389db02241b800 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Mon, 29 May 2017 23:55:42 +0100 Subject: [PATCH] builder: use fully qualified classnames in XML files In Checkstyle 7.8 a new mechanisim is used to lookup rules by name. This new method causes an exception to be thrown is there are more than one classes found that match the rule name. The solution is to use fully qualified class names in the module tag in the XML ruleset files. --- builder/pom.xml | 15 + .../CheckstyleClassNotFoundException.java | 39 +++ .../CheckstyleSourceLoadException.java | 42 +++ .../ruleset/builder/CheckstyleWriter.java | 4 +- .../checkstyle/ruleset/builder/Rule.java | 27 +- .../ruleset/builder/RuleSource.java | 40 ++- .../CheckstyleClassNotFoundExceptionTest.java | 24 ++ .../CheckstyleSourceLoadExceptionTest.java | 28 ++ .../ruleset/builder/CheckstyleWriterTest.java | 16 +- .../net/kemitix/checkstyle-1-layout.xml | 73 ++-- .../net/kemitix/checkstyle-2-naming.xml | 129 +++---- .../net/kemitix/checkstyle-3-javadoc.xml | 157 ++++----- .../net/kemitix/checkstyle-4-tweaks.xml | 249 +++++++------- .../net/kemitix/checkstyle-5-complexity.xml | 315 +++++++++--------- 14 files changed, 683 insertions(+), 475 deletions(-) create mode 100644 builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleClassNotFoundException.java create mode 100644 builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleSourceLoadException.java create mode 100644 builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleClassNotFoundExceptionTest.java create mode 100644 builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleSourceLoadExceptionTest.java diff --git a/builder/pom.xml b/builder/pom.xml index 1d0ffbf..73294ba 100644 --- a/builder/pom.xml +++ b/builder/pom.xml @@ -42,6 +42,8 @@ 2.3.5 1.0.0 4.3.0 + 7.8 + 1.24.0 @@ -92,6 +94,19 @@ ${map-builder.version} test + + + com.puppycrawl.tools + checkstyle + ${checkstyle.version} + compile + + + com.github.sevntu-checkstyle + sevntu-checks + ${sevntu.version} + + diff --git a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleClassNotFoundException.java b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleClassNotFoundException.java new file mode 100644 index 0000000..ffa297c --- /dev/null +++ b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleClassNotFoundException.java @@ -0,0 +1,39 @@ +/** + * 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.ruleset.builder; + +/** + * Raised when a Class to implement a Rule is not found. + * + * @author Paul Campbell (pcampbell@kemitix.net). + */ +public class CheckstyleClassNotFoundException extends RuntimeException { + + /** + * Constructor. + * + * @param name the name of the unimplemented rule + */ + public CheckstyleClassNotFoundException(final String name) { + super("No class found to implement " + name); + } +} diff --git a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleSourceLoadException.java b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleSourceLoadException.java new file mode 100644 index 0000000..ef9b959 --- /dev/null +++ b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleSourceLoadException.java @@ -0,0 +1,42 @@ +/** + * 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.ruleset.builder; + +import java.io.IOException; + +/** + * Raised when there is an error scanning for check classes. + * + * @author Paul Campbell (pcampbell@kemitix.net). + */ +public class CheckstyleSourceLoadException extends RuntimeException { + + /** + * Constructor. + * + * @param basePackage the base package classes were being loaded from + * @param cause the cause + */ + public CheckstyleSourceLoadException(final String basePackage, final IOException cause) { + super("Error loading checkstyle rules from package: " + basePackage, cause); + } +} diff --git a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleWriter.java b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleWriter.java index 0619a7d..5e3558a 100644 --- a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleWriter.java +++ b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleWriter.java @@ -101,9 +101,9 @@ class CheckstyleWriter implements CommandLineRunner { private String formatRuleAsModule(final Rule rule) { if (rule.getProperties() .isEmpty()) { - return String.format("", rule.getName()); + return String.format("", rule.getCanonicalClassName()); } - return String.format("%n %s%n", rule.getName(), + return String.format("%n %s%n", rule.getCanonicalClassName(), formatProperties(rule.getProperties()) ); } 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 6e6798e..70f9ae7 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 @@ -40,6 +40,11 @@ public class Rule { private static final Locale LOCALE = Locale.ENGLISH; + /** + * Configuration properties. + */ + private final Map properties = new HashMap<>(); + /** * The name of the rule's Check class. */ @@ -80,11 +85,6 @@ public class Rule { */ private String reason; - /** - * Configuration properties. - */ - private final Map properties = new HashMap<>(); - /** * Compare two Rules lexicographically for sorting by rule name, ignoring case. * @@ -104,4 +104,21 @@ public class Rule { return getName().toLowerCase(LOCALE); } + /** + * Returns the canonical name of the class that implements this rule. + * + * @return the canonical name of the implementing class + */ + public String getCanonicalClassName() { + return source.getCheckClasses() + .filter(this::byRuleName) + .findFirst() + .orElseThrow(() -> new CheckstyleClassNotFoundException(name)); + } + + private boolean byRuleName(final String classname) { + final String classNameWithDelimiter = "." + name; + return classname.endsWith(classNameWithDelimiter) || classname.endsWith(classNameWithDelimiter + "Check"); + } + } diff --git a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/RuleSource.java b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/RuleSource.java index d675310..e10cb95 100644 --- a/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/RuleSource.java +++ b/builder/src/main/java/net/kemitix/checkstyle/ruleset/builder/RuleSource.java @@ -21,6 +21,14 @@ package net.kemitix.checkstyle.ruleset.builder; +import com.google.common.reflect.ClassPath; +import lombok.Getter; + +import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + /** * The origin of the rule. * @@ -28,6 +36,34 @@ package net.kemitix.checkstyle.ruleset.builder; */ public enum RuleSource { - CHECKSTYLE, - SEVNTU, + CHECKSTYLE("com.puppycrawl.tools.checkstyle"), + SEVNTU("com.github.sevntu.checkstyle.checks"); + + @Getter + private final String basePackage; + + private final List checkClasses; + + /** + * Constructor. + * + * @param basePackage the base package that contains all checks from this source + */ + RuleSource(final String basePackage) { + this.basePackage = basePackage; + try { + checkClasses = ClassPath.from(getClass().getClassLoader()) + .getTopLevelClassesRecursive(basePackage) + .stream() + .map(ClassPath.ClassInfo::getName) + .collect(Collectors.toList()); + } catch (IOException e) { + throw new CheckstyleSourceLoadException(basePackage, e); + } + } + + public Stream getCheckClasses() { + return checkClasses.stream() + .sorted(); + } } diff --git a/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleClassNotFoundExceptionTest.java b/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleClassNotFoundExceptionTest.java new file mode 100644 index 0000000..eadf0a5 --- /dev/null +++ b/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleClassNotFoundExceptionTest.java @@ -0,0 +1,24 @@ +package net.kemitix.checkstyle.ruleset.builder; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link CheckstyleClassNotFoundException}. + * + * @author Paul Campbell (pcampbell@kemitix.net). + */ +public class CheckstyleClassNotFoundExceptionTest { + + @Test + public void canRaiseException() { + //given + final String classname = "class name"; + //when + final CheckstyleClassNotFoundException exception = new CheckstyleClassNotFoundException(classname); + //then + assertThat(exception.getMessage()).startsWith("No class found to implement ") + .endsWith(classname); + } +} diff --git a/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleSourceLoadExceptionTest.java b/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleSourceLoadExceptionTest.java new file mode 100644 index 0000000..cb3d1b1 --- /dev/null +++ b/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleSourceLoadExceptionTest.java @@ -0,0 +1,28 @@ +package net.kemitix.checkstyle.ruleset.builder; + +import org.junit.Test; + +import java.io.IOException; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +/** + * Tests for {@link CheckstyleSourceLoadException}. + * + * @author Paul Campbell (pcampbell@kemitix.net). + */ +public class CheckstyleSourceLoadExceptionTest { + + @Test + public void canRaiseException() { + //given + final String basePackage = "base package"; + final IOException cause = new IOException(); + //when + final CheckstyleSourceLoadException exception = new CheckstyleSourceLoadException(basePackage, cause); + //then + assertThat(exception).hasCause(cause) + .hasMessageStartingWith("Error loading checkstyle rules from package: ") + .hasMessageEndingWith(basePackage); + } +} diff --git a/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleWriterTest.java b/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleWriterTest.java index da7d873..4c7547b 100644 --- a/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleWriterTest.java +++ b/builder/src/test/java/net/kemitix/checkstyle/ruleset/builder/CheckstyleWriterTest.java @@ -17,7 +17,6 @@ import java.nio.file.StandardOpenOption; import java.util.AbstractMap; import java.util.List; import java.util.Map; -import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -42,6 +41,8 @@ public class CheckstyleWriterTest { private String ruleName; + private String ruleClassname; + private Map outputFiles; private Path outputDirectory; @@ -57,8 +58,8 @@ public class CheckstyleWriterTest { @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - ruleName = UUID.randomUUID() - .toString(); + ruleName = "RegexpOnFilename"; + ruleClassname = "com.puppycrawl.tools.checkstyle.checks.regexp.RegexpOnFilenameCheck"; outputProperties = new OutputProperties(); outputFiles = new MapBuilder().put(getOutputFile(RuleLevel.LAYOUT)) .put(getOutputFile(RuleLevel.NAMING)) @@ -91,7 +92,7 @@ public class CheckstyleWriterTest { checkstyleWriter.run(); //then val lines = loadOutputFile(RuleLevel.LAYOUT); - assertThat(lines).containsExactly("C:", String.format("TW:", ruleName)); + assertThat(lines).containsExactly("C:", String.format("TW:", ruleClassname)); } // write rule that is below current level @@ -105,7 +106,7 @@ public class CheckstyleWriterTest { checkstyleWriter.run(); //then val lines = loadOutputFile(RuleLevel.NAMING); - assertThat(lines).containsExactly("C:", String.format("TW:", ruleName)); + assertThat(lines).containsExactly("C:", String.format("TW:", ruleClassname)); } // write rule with checker parent @@ -119,7 +120,7 @@ public class CheckstyleWriterTest { checkstyleWriter.run(); //then val lines = loadOutputFile(RuleLevel.LAYOUT); - assertThat(lines).containsExactly(String.format("C:", ruleName), "TW:"); + assertThat(lines).containsExactly(String.format("C:", ruleClassname), "TW:"); } // write rule with properties @@ -135,7 +136,7 @@ public class CheckstyleWriterTest { checkstyleWriter.run(); //then val lines = loadOutputFile(RuleLevel.LAYOUT); - assertThat(lines).containsExactly("C:", String.format("TW:", ruleName), + assertThat(lines).containsExactly("C:", String.format("TW:", ruleClassname), " ", "" ); } @@ -207,6 +208,7 @@ public class CheckstyleWriterTest { private Rule enabledRule(final RuleLevel level, final RuleParent parent) { val rule = new Rule(); rule.setName(ruleName); + rule.setSource(RuleSource.CHECKSTYLE); rule.setEnabled(true); rule.setLevel(level); rule.setParent(parent); 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 c7c3f72..7dd6341 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-1-layout.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-1-layout.xml @@ -4,62 +4,63 @@ "http://www.puppycrawl.com/dtds/configuration_1_3.dtd"> - + - - + + - + - - - - - - - - - - - - - + + + + + + + + + + + + + - - - + + + - - - - - - - + + + + + + + - - - - - - - - - + + + + + + + + + + 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 040fe94..8388113 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-2-naming.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-2-naming.xml @@ -4,94 +4,95 @@ "http://www.puppycrawl.com/dtds/configuration_1_3.dtd"> - + - - + + - + - + - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - + + + + + + + + + + + + + - - - - - + + + + + - - - - + + + + - - - + + + - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + 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 811e021..4b469d9 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-3-javadoc.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-3-javadoc.xml @@ -4,120 +4,121 @@ "http://www.puppycrawl.com/dtds/configuration_1_3.dtd"> - + - - + + - - + + - - - + + + - - - - - - + + + + + + - - - - - - - - - - - - - - + + + + + + + + + + + + + + - - - + + + - - + + - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + - - - - - - + + + + + + - - - - + + + + - - - + + + - - + + - - - - + + + + - - - - - - - - - - - + + + + + + + + + + + + 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 2e27c90..a6ad5cc 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml @@ -4,179 +4,180 @@ "http://www.puppycrawl.com/dtds/configuration_1_3.dtd"> - + - - + + - - + + - - - + + + - - - - - - + + + + + + - + - - - - - - - - - + + + + + + + + + - - - - - - - - - - + + + + + + + + + + - - - - - - + + + + + + - - - - + + + + - - - + + + - - + + - - - - - - - - - - + + + + + + + + + + - - - - - - - - - - + + + + + + + + + + - - - - - - - - - + + + + + + + + + - - - - + + + + - - + + - - - - + + + + - - + + - - - - + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 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 bc2b68b..f486e8c 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml @@ -4,232 +4,233 @@ "http://www.puppycrawl.com/dtds/configuration_1_3.dtd"> - + - - - + + + - - + + - - - + + + - - - - - - - + + + + + + + - + - - - - + + + + - + - - - - - - - - + + + + + + + + - - - - - + + + + + - - - - - - - - - - - - - + + + + + + + + + + + + + - - - - - - + + + + + + - - - - - + + + + + - - - + + + - + - - + + - - - - - + + + + + - + - - - - - - + + + + + + - - - - - - - - + + + + + + + + - - - - - + + + + + - - + + - - - - - - - - + + + + + + + + - - + + - - - + + + - - - + + + - - - - - - + + + + + + - - - + + + - - - - + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +