diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java index e4496c227f..333edf58b6 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -34,6 +34,7 @@ import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; import org.xml.sax.SAXException; /** @@ -41,6 +42,7 @@ import org.xml.sax.SAXException; * content. By default Rules will be loaded using the ClassLoader for this * class, using the {@link RulePriority#LOW} priority, with Rule deprecation * warnings off. + * By default, the ruleset compatibility filter is active, too. See {@link RuleSetFactoryCompatibility}. */ public class RuleSetFactory { @@ -49,6 +51,7 @@ public class RuleSetFactory { private ClassLoader classLoader = RuleSetFactory.class.getClassLoader(); private RulePriority minimumPriority = RulePriority.LOW; private boolean warnDeprecated = false; + private RuleSetFactoryCompatibility compatibilityFilter = new RuleSetFactoryCompatibility(); /** * Set the ClassLoader to use when loading Rules. @@ -79,6 +82,22 @@ public class RuleSetFactory { this.warnDeprecated = warnDeprecated; } + /** + * Disable the ruleset compatibility filter. Disabling this filter will cause + * exception when loading a ruleset, which uses references to old/not existing rules. + */ + public void disableCompatibilityFilter() { + compatibilityFilter = null; + } + + /** + * Gets the compatibility filter in order to adjust it, e.g. add additional filters. + * @return the {@link RuleSetFactoryCompatibility} + */ + public RuleSetFactoryCompatibility getCompatibilityFilter() { + return compatibilityFilter; + } + /** * Returns an Iterator of RuleSet objects loaded from descriptions from the * "rulesets.properties" resource for each Language with Rule support. @@ -220,7 +239,13 @@ public class RuleSetFactory { } try { DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); - Document document = builder.parse(inputStream); + InputSource inputSource; + if (compatibilityFilter != null) { + inputSource = new InputSource(compatibilityFilter.filterRuleSetFile(inputStream)); + } else { + inputSource = new InputSource(inputStream); + } + Document document = builder.parse(inputSource); Element ruleSetElement = document.getDocumentElement(); RuleSet ruleSet = new RuleSet(); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java new file mode 100644 index 0000000000..10b37d7fb6 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java @@ -0,0 +1,190 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ +package net.sourceforge.pmd; + +import java.io.IOException; +import java.io.InputStream; +import java.io.Reader; +import java.io.StringReader; +import java.nio.charset.Charset; +import java.util.LinkedList; +import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.commons.io.IOUtils; + +/** + * Provides a simple filter mechanism to avoid failing to parse an old ruleset, which references rules, that + * have either been removed from PMD already or renamed or moved to another ruleset. + * + * @see issue 1360 + */ +public class RuleSetFactoryCompatibility { + private static final Logger LOG = Logger.getLogger(RuleSetFactoryCompatibility.class.getName()); + + private List filters = new LinkedList(); + + /** + * Creates a new instance of the compatibility filter with the built-in filters for the + * modified PMD rules. + */ + public RuleSetFactoryCompatibility() { + // PMD 5.3.0 + addFilterRuleRenamed("java", "design", "UncommentedEmptyMethod", "UncommentedEmptyMethodBody"); + addFilterRuleRemoved("java", "controversial", "BooleanInversion"); + + // PMD 5.3.1 + addFilterRuleRenamed("java", "design", "UseSingleton", "UseUtilityClass"); + + // PMD 5.4.0 + addFilterRuleMoved("java", "basic", "empty", "EmptyCatchBlock"); + addFilterRuleMoved("java", "basic", "empty", "EmptyIfStatement"); + addFilterRuleMoved("java", "basic", "empty", "EmptyWhileStmt"); + addFilterRuleMoved("java", "basic", "empty", "EmptyTryBlock"); + addFilterRuleMoved("java", "basic", "empty", "EmptyFinallyBlock"); + addFilterRuleMoved("java", "basic", "empty", "EmptySwitchStatements"); + addFilterRuleMoved("java", "basic", "empty", "EmptySynchronizedBlock"); + addFilterRuleMoved("java", "basic", "empty", "EmptyStatementNotInLoop"); + addFilterRuleMoved("java", "basic", "empty", "EmptyInitializer"); + addFilterRuleMoved("java", "basic", "empty", "EmptyStatementBlock"); + addFilterRuleMoved("java", "basic", "empty", "EmptyStaticInitializer"); + addFilterRuleMoved("java", "basic", "unnecessary", "UnnecessaryConversionTemporary"); + addFilterRuleMoved("java", "basic", "unnecessary", "UnnecessaryReturn"); + addFilterRuleMoved("java", "basic", "unnecessary", "UnnecessaryFinalModifier"); + addFilterRuleMoved("java", "basic", "unnecessary", "UselessOverridingMethod"); + addFilterRuleMoved("java", "basic", "unnecessary", "UselessOperationOnImmutable"); + addFilterRuleMoved("java", "basic", "unnecessary", "UnusedNullCheckInEquals"); + addFilterRuleMoved("java", "basic", "unnecessary", "UselessParentheses"); + } + + void addFilterRuleRenamed(String language, String ruleset, String oldName, String newName) { + filters.add(RuleSetFilter.ruleRenamed(language, ruleset, oldName, newName)); + } + void addFilterRuleMoved(String language, String oldRuleset, String newRuleset, String ruleName) { + filters.add(RuleSetFilter.ruleMoved(language, oldRuleset, newRuleset, ruleName)); + } + void addFilterRuleRemoved(String language, String ruleset, String name) { + filters.add(RuleSetFilter.ruleRemoved(language, ruleset, name)); + } + + /** + * Applies all configured filters against the given input stream. + * The resulting reader will contain the original ruleset modified by + * the filters. + * + * @param stream + * @return + * @throws IOException + */ + public Reader filterRuleSetFile(InputStream stream) throws IOException { + byte[] bytes = IOUtils.toByteArray(stream); + String encoding = determineEncoding(bytes); + String ruleset = new String(bytes, encoding); + + ruleset = applyAllFilters(ruleset); + + return new StringReader(ruleset); + } + + private String applyAllFilters(String in) { + String result = in; + for (RuleSetFilter filter : filters) { + result = filter.apply(result); + } + return result; + } + + private static final Pattern ENCODING_PATTERN = Pattern.compile("encoding=\"([^\"]+)\""); + /** + * Determines the encoding of the given bytes, assuming this is a XML document, which specifies + * the encoding in the first 1024 bytes. + * + * @param bytes the input bytes, might be more or less than 1024 bytes + * @return the determined encoding, falls back to the default UTF-8 encoding + */ + String determineEncoding(byte[] bytes) { + String firstBytes = new String(bytes, 0, bytes.length > 1024 ? 1024 : bytes.length, Charset.forName("ISO-8859-1")); + Matcher matcher = ENCODING_PATTERN.matcher(firstBytes); + String encoding = Charset.forName("UTF-8").name(); + if (matcher.find()) { + encoding = matcher.group(1); + } + return encoding; + } + + private static class RuleSetFilter { + private final Pattern refPattern; + private final String replacement; + private Pattern exclusionPattern; + private String exclusionReplacement; + private final String logMessage; + private RuleSetFilter(String refPattern, String replacement, String logMessage) { + this.logMessage = logMessage; + if (replacement != null) { + this.refPattern = Pattern.compile("ref=\"" + Pattern.quote(refPattern) + "\""); + this.replacement = "ref=\"" + replacement + "\""; + } else { + this.refPattern = Pattern.compile(""); + this.replacement = ""; + } + } + + private void setExclusionPattern(String oldName, String newName) { + exclusionPattern = Pattern.compile(""); + if (newName != null) { + exclusionReplacement = ""; + } else { + exclusionReplacement = ""; + } + } + + public static RuleSetFilter ruleRenamed(String language, String ruleset, String oldName, String newName) { + String base = "rulesets/" + language + "/" + ruleset + ".xml/"; + RuleSetFilter filter = new RuleSetFilter(base + oldName, base + newName, + "The rule \"" + oldName + "\" has been renamed to \"" + newName + "\". Please change your ruleset!"); + filter.setExclusionPattern(oldName, newName); + return filter; + } + public static RuleSetFilter ruleMoved(String language, String oldRuleset, String newRuleset, String ruleName) { + String base = "rulesets/" + language + "/"; + return new RuleSetFilter(base + oldRuleset + ".xml/" + ruleName, base + newRuleset + ".xml/" + ruleName, + "The rule \"" + ruleName + "\" has been moved from ruleset \"" + oldRuleset + "\" to \"" + newRuleset + "\". Please change your ruleset!"); + } + public static RuleSetFilter ruleRemoved(String language, String ruleset, String name) { + RuleSetFilter filter = new RuleSetFilter("rulesets/" + language + "/" + ruleset + ".xml/" + name, null, + "The rule \"" + name + "\" in ruleset \"" + ruleset + "\" has been removed from PMD and no longer exists. Please change your ruleset!"); + filter.setExclusionPattern(name, null); + return filter; + } + + String apply(String in) { + String result = in; + Matcher matcher = refPattern.matcher(in); + + if (matcher.find()) { + result = matcher.replaceAll(replacement); + + if (LOG.isLoggable(Level.WARNING)) { + LOG.warning("Applying rule set filter: " + logMessage); + } + } + + if (exclusionPattern == null) return result; + + Matcher exclusions = exclusionPattern.matcher(result); + if (exclusions.find()) { + result = exclusions.replaceAll(exclusionReplacement); + + if (LOG.isLoggable(Level.WARNING)) { + LOG.warning("Applying rule set filter for exclusions: " + logMessage); + } + } + + return result; + } + } +} diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryCompatibilityTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryCompatibilityTest.java new file mode 100644 index 0000000000..e0069c45fb --- /dev/null +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryCompatibilityTest.java @@ -0,0 +1,139 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ +package net.sourceforge.pmd; +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.io.Reader; +import java.nio.charset.Charset; + +import org.apache.commons.io.IOUtils; +import org.junit.Assert; +import org.junit.Test; + +public class RuleSetFactoryCompatibilityTest { + private static final Charset ISO_8859_1 = Charset.forName("ISO-8859-1"); + private static final Charset UTF_8 = Charset.forName("UTF-8"); + + @Test + public void testCorrectOldReference() throws Exception { + final String ruleset = "\n" + + "\n" + + "\n" + + " Test\n" + + "\n" + + " \n" + + "\n"; + + RuleSetFactory factory = new RuleSetFactory(); + factory.getCompatibilityFilter().addFilterRuleMoved("dummy", "notexisting", "basic", "DummyBasicMockRule"); + + RuleSet createdRuleSet = createRulesetFromString(ruleset, factory); + Assert.assertNotNull(createdRuleSet.getRuleByName("DummyBasicMockRule")); + } + + @Test + public void testExclusion() throws Exception { + final String ruleset = "\n" + + "\n" + + "\n" + + " Test\n" + + "\n" + + " \n" + + " \n" + + " \n" + + "\n"; + + RuleSetFactory factory = new RuleSetFactory(); + factory.getCompatibilityFilter().addFilterRuleRenamed("dummy", "basic", "OldNameOfSampleXPathRule", "SampleXPathRule"); + + RuleSet createdRuleSet = createRulesetFromString(ruleset, factory); + Assert.assertNotNull(createdRuleSet.getRuleByName("DummyBasicMockRule")); + Assert.assertNull(createdRuleSet.getRuleByName("SampleXPathRule")); + } + + @Test + public void testFilter() throws Exception { + RuleSetFactoryCompatibility rsfc = new RuleSetFactoryCompatibility(); + rsfc.addFilterRuleMoved("dummy", "notexisting", "basic", "DummyBasicMockRule"); + rsfc.addFilterRuleRemoved("dummy", "basic", "DeletedRule"); + rsfc.addFilterRuleRenamed("dummy", "basic", "OldNameOfBasicMockRule", "NewNameOfBasicMockRule"); + + String in = "\n" + + "\n" + + "\n" + + " Test\n" + + "\n" + + " \n" + + " \n" + + " \n" + + "\n"; + InputStream stream = new ByteArrayInputStream(in.getBytes(ISO_8859_1)); + Reader filtered = rsfc.filterRuleSetFile(stream); + String out = IOUtils.toString(filtered); + + Assert.assertFalse(out.contains("notexisting.xml")); + Assert.assertTrue(out.contains("")); + + Assert.assertFalse(out.contains("DeletedRule")); + + Assert.assertFalse(out.contains("OldNameOfBasicMockRule")); + Assert.assertTrue(out.contains("")); + } + + @Test + public void testExclusionFilter() throws Exception { + RuleSetFactoryCompatibility rsfc = new RuleSetFactoryCompatibility(); + rsfc.addFilterRuleRenamed("dummy", "basic", "AnotherOldNameOfBasicMockRule", "NewNameOfBasicMockRule"); + + String in = "\n" + + "\n" + + "\n" + + " Test\n" + + "\n" + + " \n" + + " \n" + + " \n" + + "\n"; + InputStream stream = new ByteArrayInputStream(in.getBytes(ISO_8859_1)); + Reader filtered = rsfc.filterRuleSetFile(stream); + String out = IOUtils.toString(filtered); + + Assert.assertFalse(out.contains("OldNameOfBasicMockRule")); + Assert.assertTrue(out.contains("")); + } + + @Test + public void testEncoding() { + RuleSetFactoryCompatibility rsfc = new RuleSetFactoryCompatibility(); + String testString; + + testString = ""; + Assert.assertEquals("ISO-8859-1", rsfc.determineEncoding(testString.getBytes(ISO_8859_1))); + + testString = ""; + Assert.assertEquals("UTF-8", rsfc.determineEncoding(testString.getBytes(ISO_8859_1))); + } + + private RuleSet createRulesetFromString(final String ruleset, RuleSetFactory factory) + throws RuleSetNotFoundException { + return factory.createRuleSet(new RuleSetReferenceId(null) { + @Override + public InputStream getInputStream(ClassLoader classLoader) throws RuleSetNotFoundException { + return new ByteArrayInputStream(ruleset.getBytes(UTF_8)); + } + }); + } +} diff --git a/pmd-core/src/test/resources/rulesets/dummy/basic.xml b/pmd-core/src/test/resources/rulesets/dummy/basic.xml index 30273be6d8..0a141832a2 100644 --- a/pmd-core/src/test/resources/rulesets/dummy/basic.xml +++ b/pmd-core/src/test/resources/rulesets/dummy/basic.xml @@ -19,4 +19,18 @@ Just for test + + + Test + 3 + + + + + + + " \ No newline at end of file diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 133cee331c..bfd9ffda0b 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -9,6 +9,7 @@ **Feature Request and Improvements:** * A JSON-renderer for PMD which is compatible with CodeClimate. See [PR#83](https://github.com/pmd/pmd/pull/83). +* [#1360](https://sourceforge.net/p/pmd/bugs/1360/): Provide backwards compatibility for PMD configuration file **New/Modified/Deprecated Rules:**