diff --git a/docs/pages/pmd/userdocs/extending/writing_rules_intro.md b/docs/pages/pmd/userdocs/extending/writing_rules_intro.md index e98398684f..2c0b4dc478 100644 --- a/docs/pages/pmd/userdocs/extending/writing_rules_intro.md +++ b/docs/pages/pmd/userdocs/extending/writing_rules_intro.md @@ -120,6 +120,11 @@ Example: ``` +{% include note.html content="In PMD 7, the `language` attribute will be required on all `rule` + elements that declare a new rule. Some base rule classes set the language implicitly in their + constructor, and so this is not required in all cases for the rule to work. But this + behavior will be discontinued in PMD 7, so missing `language` attributes are + reported beginning with PMD 6.27.0 as a forward compatibility warning." %} ## Resource index diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 61598b7970..32c102919b 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -16,11 +16,19 @@ This is a {{ site.pmd.release_type }} release. ### Fixed Issues +* core + * [#724](https://github.com/pmd/pmd/issues/724): \[core] Avoid parsing rulesets multiple times * java-performance * [#2441](https://github.com/pmd/pmd/issues/2441): \[java] RedundantFieldInitializer can not detect a special case for char initialize: `char foo = '\0';` ### API Changes +* XML rule definition in rulesets: In PMD 7, the `language` attribute will be required on all `rule` + elements that declare a new rule. Some base rule classes set the language implicitly in their + constructor, and so this is not required in all cases for the rule to work. But this + behavior will be discontinued in PMD 7, so missing `language` attributes are now + reported as a forward compatibility warning. + ### External Contributions * [#2677](https://github.com/pmd/pmd/pull/2677): \[java] RedundantFieldInitializer can not detect a special case for char initialize: `char foo = '\0';` - [Mykhailo Palahuta](https://github.com/Drofff) diff --git a/pmd-apex/src/main/resources/category/apex/bestpractices.xml b/pmd-apex/src/main/resources/category/apex/bestpractices.xml index f91fa5095a..5c13033efc 100644 --- a/pmd-apex/src/main/resources/category/apex/bestpractices.xml +++ b/pmd-apex/src/main/resources/category/apex/bestpractices.xml @@ -10,6 +10,7 @@ Rules which enforce generally accepted best practices. 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 4603f6f727..ce55b8a35f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -8,9 +8,11 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Properties; import java.util.Set; import java.util.logging.Level; @@ -60,6 +62,8 @@ public class RuleSetFactory { private final boolean warnDeprecated; private final RuleSetFactoryCompatibility compatibilityFilter; + private final Map parsedRulesets = new HashMap<>(); + /** * @deprecated Use {@link RulesetsFactoryUtils#defaultFactory()} */ @@ -338,10 +342,16 @@ public class RuleSetFactory { private Rule createRule(RuleSetReferenceId ruleSetReferenceId, boolean withDeprecatedRuleReferences) throws RuleSetNotFoundException { if (ruleSetReferenceId.isAllRules()) { - throw new IllegalArgumentException( - "Cannot parse a single Rule from an all Rule RuleSet reference: <" + ruleSetReferenceId + ">."); + throw new IllegalArgumentException("Cannot parse a single Rule from an all Rule RuleSet reference: <" + ruleSetReferenceId + ">."); + } + RuleSet ruleSet; + // java8: computeIfAbsent + if (parsedRulesets.containsKey(ruleSetReferenceId)) { + ruleSet = parsedRulesets.get(ruleSetReferenceId); + } else { + ruleSet = createRuleSet(ruleSetReferenceId, withDeprecatedRuleReferences); + parsedRulesets.put(ruleSetReferenceId, ruleSet); } - RuleSet ruleSet = createRuleSet(ruleSetReferenceId, withDeprecatedRuleReferences); return ruleSet.getRuleByName(ruleSetReferenceId.getRuleName()); } @@ -610,12 +620,18 @@ public class RuleSetFactory { // Stop if we're looking for a particular Rule, and this element is not // it. if (StringUtils.isNotBlank(ruleSetReferenceId.getRuleName()) - && !isRuleName(ruleElement, ruleSetReferenceId.getRuleName())) { + && !isRuleName(ruleElement, ruleSetReferenceId.getRuleName())) { return; } Rule rule = new RuleFactory(resourceLoader).buildRule(ruleElement); rule.setRuleSetName(ruleSetBuilder.getName()); + if (StringUtils.isBlank(ruleElement.getAttribute("language"))) { + LOG.warning("Rule " + ruleSetReferenceId.getRuleSetFileName() + "/" + rule.getName() + " does not mention attribute" + + " language='" + rule.getLanguage().getTerseName() + "'," + + " please mention it explicitly to be compatible with PMD 7"); + } + ruleSetBuilder.addRule(rule); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleFactory.java index 217bfe2084..5ff8a4e41c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleFactory.java @@ -152,9 +152,9 @@ public class RuleFactory { String name = ruleElement.getAttribute(NAME); RuleBuilder builder = new RuleBuilder(name, - resourceLoader, - ruleElement.getAttribute(CLASS), - ruleElement.getAttribute("language")); + resourceLoader, + ruleElement.getAttribute(CLASS), + ruleElement.getAttribute("language")); if (ruleElement.hasAttribute(MINIMUM_LANGUAGE_VERSION)) { builder.minimumLanguageVersion(ruleElement.getAttribute(MINIMUM_LANGUAGE_VERSION)); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java index 95c9c5ba35..f5b26ec49f 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java @@ -604,7 +604,7 @@ public class RuleSetFactoryTest { } @Test(expected = IllegalArgumentException.class) - public void testIncorrectMinimumLanugageVersion() throws RuleSetNotFoundException { + public void testIncorrectMinimumLanguageVersion() throws RuleSetNotFoundException { loadFirstRule(INCORRECT_MINIMUM_LANGUAGE_VERSION); } @@ -633,7 +633,7 @@ public class RuleSetFactoryTest { } @Test(expected = IllegalArgumentException.class) - public void testIncorrectMaximumLanugageVersion() throws RuleSetNotFoundException { + public void testIncorrectMaximumLanguageVersion() throws RuleSetNotFoundException { loadFirstRule(INCORRECT_MAXIMUM_LANGUAGE_VERSION); } @@ -985,6 +985,7 @@ public class RuleSetFactoryTest { + " testdesc\n" + "\n" @@ -997,6 +998,7 @@ public class RuleSetFactoryTest { + " testdesc\n" + "\n" @@ -1034,6 +1036,7 @@ public class RuleSetFactoryTest { + "\n" + "testdesc\n" + "\n" @@ -1045,11 +1048,13 @@ public class RuleSetFactoryTest { + "\n" + "testdesc\n" + "\n" + "\n" + "\n" + "\n" + "\n" @@ -1059,6 +1064,7 @@ public class RuleSetFactoryTest { + "\n" + "testdesc\n" + "\n" + "\n" @@ -1080,6 +1086,7 @@ public class RuleSetFactoryTest { + "\n" + "testdesc\n" + "\n" + "3\n" @@ -1098,6 +1105,7 @@ public class RuleSetFactoryTest { + "\n" + "testdesc\n" + "\n" @@ -1110,7 +1118,8 @@ public class RuleSetFactoryTest { + "\n" + + "class=\"net.sourceforge.pmd.lang.rule.MockRule\" " + + "language=\"dummy\">\n" + ""; private static final String INCORRECT_LANGUAGE = "\n" @@ -1186,8 +1195,8 @@ public class RuleSetFactoryTest { + "\n" + "testdesc\n" + "\n" + ""; @@ -1216,6 +1225,7 @@ public class RuleSetFactoryTest { + "\n" + "testdesc\n" + " @@ -483,6 +489,7 @@ public void bar(String string) { diff --git a/pmd-javascript/src/main/resources/category/ecmascript/bestpractices.xml b/pmd-javascript/src/main/resources/category/ecmascript/bestpractices.xml index 5b39319cd2..28c82246d6 100644 --- a/pmd-javascript/src/main/resources/category/ecmascript/bestpractices.xml +++ b/pmd-javascript/src/main/resources/category/ecmascript/bestpractices.xml @@ -37,6 +37,7 @@ with (object) { commons-io commons-io - + + com.github.stefanbirkner + system-rules + compile + org.mockito mockito-core diff --git a/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java b/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java index 23834d26b2..164e0590a3 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java @@ -33,6 +33,7 @@ import javax.xml.parsers.SAXParserFactory; import org.apache.commons.io.FilenameUtils; import org.junit.BeforeClass; import org.junit.Test; +import org.junit.contrib.java.lang.system.SystemErrRule; import org.xml.sax.InputSource; import org.xml.sax.SAXException; import org.xml.sax.SAXParseException; @@ -50,6 +51,9 @@ import net.sourceforge.pmd.util.ResourceLoader; * subclassed for each language. */ public abstract class AbstractRuleSetFactoryTest { + @org.junit.Rule + public final SystemErrRule systemErrRule = new SystemErrRule().enableLog().muteForSuccessfulTests(); + private static SAXParserFactory saxParserFactory; private static ValidateDefaultHandler validateDefaultHandler; private static SAXParser saxParser; diff --git a/pmd-visualforce/src/main/resources/category/vf/security.xml b/pmd-visualforce/src/main/resources/category/vf/security.xml index 1d07a8d3b9..fcb7c49a37 100644 --- a/pmd-visualforce/src/main/resources/category/vf/security.xml +++ b/pmd-visualforce/src/main/resources/category/vf/security.xml @@ -10,6 +10,7 @@ Rules that flag potential security flaws. diff --git a/pmd-vm/src/main/resources/category/vm/errorprone.xml b/pmd-vm/src/main/resources/category/vm/errorprone.xml index ad2c1b05bd..646d63aa69 100644 --- a/pmd-vm/src/main/resources/category/vm/errorprone.xml +++ b/pmd-vm/src/main/resources/category/vm/errorprone.xml @@ -10,6 +10,7 @@ Rules to detect constructs that are either broken, extremely confusing or prone