From 7a456e5b935242acaf16619b4ae8e9a12e4e52f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 25 Aug 2020 15:16:30 +0200 Subject: [PATCH 1/7] Remove ruleContext attributes Refs #2676 Was forgotten in #2672 --- .../java/net/sourceforge/pmd/RuleContext.java | 92 +------------------ .../net/sourceforge/pmd/RuleContextTest.java | 54 ----------- 2 files changed, 1 insertion(+), 145 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java index 76f30aae84..5690591011 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java @@ -5,8 +5,6 @@ package net.sourceforge.pmd; import java.io.File; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.logging.Logger; import net.sourceforge.pmd.lang.LanguageVersion; @@ -34,14 +32,13 @@ public class RuleContext { private Report report = new Report(); private File sourceCodeFile; private LanguageVersion languageVersion; - private final ConcurrentMap attributes; private boolean ignoreExceptions = true; /** * Default constructor. */ public RuleContext() { - attributes = new ConcurrentHashMap<>(); + // nothing to do } /** @@ -52,7 +49,6 @@ public class RuleContext { * the context from which the values are shared */ public RuleContext(RuleContext ruleContext) { - this.attributes = ruleContext.attributes; this.report.addListeners(ruleContext.getReport().getListeners()); } @@ -146,92 +142,6 @@ public class RuleContext { this.languageVersion = languageVersion; } - /** - * Set an attribute value on the RuleContext, if it does not already exist. - *

- * Attributes can be shared between RuleContext instances. This operation is - * thread-safe. - *

- * Attribute values should be modified directly via the reference provided. - * It is not necessary to call setAttribute(String, Object) to - * update an attribute value. Modifications made to the attribute value will - * automatically be seen by other threads. Because of this, you must ensure - * the attribute values are themselves thread safe. - * - * @param name - * The attribute name. - * @param value - * The attribute value. - * @exception IllegalArgumentException - * if name or value are - * null - * @return true if the attribute was set, false - * otherwise. - * - * @deprecated Stateful methods of the rule context will be removed. - * Their interaction with incremental analysis are unspecified. - */ - @Deprecated - public boolean setAttribute(String name, Object value) { - if (name == null) { - throw new IllegalArgumentException("Parameter 'name' cannot be null."); - } - if (value == null) { - throw new IllegalArgumentException("Parameter 'value' cannot be null."); - } - return this.attributes.putIfAbsent(name, value) == null; - } - - /** - * Get an attribute value on the RuleContext. - *

- * Attributes can be shared between RuleContext instances. This operation is - * thread-safe. - *

- * Attribute values should be modified directly via the reference provided. - * It is not necessary to call setAttribute(String, Object) to - * update an attribute value. Modifications made to the attribute value will - * automatically be seen by other threads. Because of this, you must ensure - * the attribute values are themselves thread safe. - * - * @param name - * The attribute name. - * @return The current attribute value, or null if the - * attribute does not exist. - * - * @deprecated Stateful methods of the rule context will be removed. - * Their interaction with incremental analysis are unspecified. - */ - @Deprecated - public Object getAttribute(String name) { - return this.attributes.get(name); - } - - /** - * Remove an attribute value on the RuleContext. - *

- * Attributes can be shared between RuleContext instances. This operation is - * thread-safe. - *

- * Attribute values should be modified directly via the reference provided. - * It is not necessary to call setAttribute(String, Object) to - * update an attribute value. Modifications made to the attribute value will - * automatically be seen by other threads. Because of this, you must ensure - * the attribute values are themselves thread safe. - * - * @param name - * The attribute name. - * @return The current attribute value, or null if the - * attribute does not exist. - * - * @deprecated Stateful methods of the rule context will be removed. - * Their interaction with incremental analysis are unspecified. - */ - @Deprecated - public Object removeAttribute(String name) { - return this.attributes.remove(name); - } - /** * Configure whether exceptions during applying a rule should be ignored or * not. If set to true then such exceptions are logged as diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java index 279d60c66b..7ae5aba5eb 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java @@ -5,11 +5,7 @@ package net.sourceforge.pmd; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; import java.io.File; @@ -45,56 +41,6 @@ public class RuleContextTest { assertEquals("filename mismatch", new File("somefile.java"), ctx.getSourceCodeFile()); } - @Test - public void testAttributes() { - RuleContext ctx1 = new RuleContext(); - Object obj1 = new Object(); - Object obj2 = new Object(); - assertNull("attribute should be null", ctx1.getAttribute("attribute")); - boolean set = ctx1.setAttribute("attribute", obj1); - assertTrue("attribute should have been set", set); - assertNotNull("attribute should not be null", ctx1.getAttribute("attribute")); - assertSame("attribute should be expected instance", ctx1.getAttribute("attribute"), obj1); - set = ctx1.setAttribute("attribute", obj2); - assertFalse("attribute should not have been set", set); - assertSame("attribute should be expected instance", ctx1.getAttribute("attribute"), obj1); - Object value = ctx1.removeAttribute("attribute"); - assertSame("attribute value should be expected instance", value, obj1); - assertNull("attribute should be null", ctx1.getAttribute("attribute")); - } - - @Test - public void testSharedAttributes() { - RuleContext ctx1 = new RuleContext(); - RuleContext ctx2 = new RuleContext(ctx1); - StringBuilder obj1 = new StringBuilder(); - StringBuilder obj2 = new StringBuilder(); - - ctx1.setAttribute("attribute1", obj1); - ctx2.setAttribute("attribute2", obj2); - assertNotNull("attribute should not be null", ctx1.getAttribute("attribute1")); - assertNotNull("attribute should not be null", ctx1.getAttribute("attribute2")); - assertNotNull("attribute should not be null", ctx2.getAttribute("attribute1")); - assertNotNull("attribute should not be null", ctx2.getAttribute("attribute2")); - assertSame("attribute should be expected instance", ctx1.getAttribute("attribute1"), obj1); - assertSame("attribute should be expected instance", ctx1.getAttribute("attribute2"), obj2); - assertSame("attribute should be expected instance", ctx2.getAttribute("attribute1"), obj1); - assertSame("attribute should be expected instance", ctx2.getAttribute("attribute2"), obj2); - - ctx1.removeAttribute("attribute1"); - assertNull("attribute should be null", ctx1.getAttribute("attribute1")); - assertNull("attribute should be null", ctx2.getAttribute("attribute1")); - assertNotNull("attribute should not be null", ctx1.getAttribute("attribute2")); - assertNotNull("attribute should not be null", ctx2.getAttribute("attribute2")); - - StringBuilder value = (StringBuilder) ctx1.getAttribute("attribute2"); - assertEquals("attribute value should be empty", "", value.toString()); - value.append("x"); - StringBuilder value1 = (StringBuilder) ctx1.getAttribute("attribute2"); - assertEquals("attribute value should be 'x'", "x", value1.toString()); - StringBuilder value2 = (StringBuilder) ctx2.getAttribute("attribute2"); - assertEquals("attribute value should be 'x'", "x", value2.toString()); - } public static junit.framework.Test suite() { return new JUnit4TestAdapter(RuleContextTest.class); From 5f8e5fc1ffb07e152be50e4af0b75f833bae3cdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 25 Aug 2020 15:22:38 +0200 Subject: [PATCH 2/7] Remove ImmutableLanguage --- .../pmd/lang/apex/rule/AbstractApexRule.java | 3 +-- .../net/sourceforge/pmd/RuleSetWriter.java | 18 +++++++++--------- .../pmd/lang/rule/AbstractRule.java | 2 +- .../pmd/lang/rule/ImmutableLanguage.java | 17 ----------------- .../pmd/lang/java/rule/AbstractJavaRule.java | 3 +-- .../rule/AbstractEcmascriptRule.java | 3 +-- .../pmd/lang/jsp/rule/AbstractJspRule.java | 3 +-- .../modelica/rule/AbstractModelicaRule.java | 3 +-- .../pmd/lang/plsql/rule/AbstractPLSQLRule.java | 3 +-- .../pmd/lang/vf/rule/AbstractVfRule.java | 3 +-- .../pmd/lang/vm/rule/AbstractVmRule.java | 3 +-- .../pmd/lang/xml/rule/AbstractXmlRule.java | 3 +-- 12 files changed, 19 insertions(+), 45 deletions(-) delete mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/ImmutableLanguage.java diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRule.java index b6f5883189..6c8ca348c6 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRule.java @@ -12,10 +12,9 @@ import net.sourceforge.pmd.lang.apex.ApexParserOptions; import net.sourceforge.pmd.lang.apex.ast.ApexParserVisitor; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.lang.rule.ImmutableLanguage; public abstract class AbstractApexRule extends AbstractRule - implements ApexParserVisitor, ImmutableLanguage { + implements ApexParserVisitor { public AbstractApexRule() { super.setLanguage(LanguageRegistry.getLanguage(ApexLanguageModule.NAME)); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java index a736777dc8..d918d566ef 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java @@ -31,7 +31,6 @@ import org.w3c.dom.Text; import net.sourceforge.pmd.lang.Language; import net.sourceforge.pmd.lang.LanguageVersion; -import net.sourceforge.pmd.lang.rule.ImmutableLanguage; import net.sourceforge.pmd.lang.rule.RuleReference; import net.sourceforge.pmd.lang.rule.XPathRule; import net.sourceforge.pmd.properties.PropertyDescriptor; @@ -190,13 +189,13 @@ public class RuleSetWriter { propertyDescriptors, propertiesByPropertyDescriptor, examples); } } else { - return createSingleRuleElement(rule instanceof ImmutableLanguage ? null : rule.getLanguage(), - rule.getMinimumLanguageVersion(), rule.getMaximumLanguageVersion(), rule.isDeprecated(), - rule.getName(), rule.getSince(), null, rule.getMessage(), rule.getExternalInfoUrl(), - rule.getRuleClass(), - rule.getDescription(), - rule.getPriority(), rule.getPropertyDescriptors(), rule.getPropertiesByPropertyDescriptor(), - rule.getExamples()); + return createSingleRuleElement(rule.getLanguage(), + rule.getMinimumLanguageVersion(), rule.getMaximumLanguageVersion(), rule.isDeprecated(), + rule.getName(), rule.getSince(), null, rule.getMessage(), rule.getExternalInfoUrl(), + rule.getRuleClass(), + rule.getDescription(), + rule.getPriority(), rule.getPropertyDescriptors(), rule.getPropertiesByPropertyDescriptor(), + rule.getExamples()); } } @@ -212,7 +211,8 @@ public class RuleSetWriter { String description, RulePriority priority, List> propertyDescriptors, Map, Object> propertiesByPropertyDescriptor, List examples) { Element ruleElement = createRuleElement(); - if (language != null) { + // language is now a required attribute, unless this is a rule reference + if (clazz != null) { ruleElement.setAttribute("language", language.getTerseName()); } if (minimumLanguageVersion != null) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java index f3922cf93b..06e5201b6f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java @@ -93,7 +93,7 @@ public abstract class AbstractRule extends AbstractPropertySource implements Rul @Override public void setLanguage(Language language) { - if (this.language != null && this instanceof ImmutableLanguage && !this.language.equals(language)) { + if (this.language != null && !this.language.equals(language)) { throw new UnsupportedOperationException("The Language for Rule class " + this.getClass().getName() + " is immutable and cannot be changed."); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/ImmutableLanguage.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/ImmutableLanguage.java deleted file mode 100644 index 73aeeb4265..0000000000 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/ImmutableLanguage.java +++ /dev/null @@ -1,17 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.rule; - -/** - * This is a tag interface to indicate that a Rule implementation class does not - * support changes to it's Language. The Language is integral to the proper - * functioning of the Rule. - * - * @deprecated No rule supports a change to their language. This will - * be made the default behaviour with PMD 7.0.0. - */ -@Deprecated -public interface ImmutableLanguage { -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java index fcca69a5c8..d51065d86a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java @@ -18,7 +18,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; import net.sourceforge.pmd.lang.java.ast.JavaParserVisitor; import net.sourceforge.pmd.lang.java.internal.JavaProcessingStage; import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.lang.rule.ImmutableLanguage; /** @@ -28,7 +27,7 @@ import net.sourceforge.pmd.lang.rule.ImmutableLanguage; * TODO add documentation * */ -public abstract class AbstractJavaRule extends AbstractRule implements JavaParserVisitor, ImmutableLanguage { +public abstract class AbstractJavaRule extends AbstractRule implements JavaParserVisitor { public AbstractJavaRule() { super.setLanguage(LanguageRegistry.getLanguage(JavaLanguageModule.NAME)); diff --git a/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/rule/AbstractEcmascriptRule.java b/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/rule/AbstractEcmascriptRule.java index 599f507b66..c916c1a71d 100644 --- a/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/rule/AbstractEcmascriptRule.java +++ b/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/rule/AbstractEcmascriptRule.java @@ -13,12 +13,11 @@ import net.sourceforge.pmd.lang.ecmascript.EcmascriptParserOptions; import net.sourceforge.pmd.lang.ecmascript.EcmascriptParserOptions.Version; import net.sourceforge.pmd.lang.ecmascript.ast.EcmascriptParserVisitor; import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.lang.rule.ImmutableLanguage; import net.sourceforge.pmd.properties.PropertyDescriptor; public abstract class AbstractEcmascriptRule extends AbstractRule - implements EcmascriptParserVisitor, ImmutableLanguage { + implements EcmascriptParserVisitor { private static final PropertyDescriptor RECORDING_COMMENTS_DESCRIPTOR = EcmascriptParserOptions.RECORDING_COMMENTS_DESCRIPTOR; private static final PropertyDescriptor RECORDING_LOCAL_JSDOC_COMMENTS_DESCRIPTOR = EcmascriptParserOptions.RECORDING_LOCAL_JSDOC_COMMENTS_DESCRIPTOR; diff --git a/pmd-jsp/src/main/java/net/sourceforge/pmd/lang/jsp/rule/AbstractJspRule.java b/pmd-jsp/src/main/java/net/sourceforge/pmd/lang/jsp/rule/AbstractJspRule.java index dbb4b2c243..e24a7e9e9e 100644 --- a/pmd-jsp/src/main/java/net/sourceforge/pmd/lang/jsp/rule/AbstractJspRule.java +++ b/pmd-jsp/src/main/java/net/sourceforge/pmd/lang/jsp/rule/AbstractJspRule.java @@ -10,9 +10,8 @@ import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.jsp.JspLanguageModule; import net.sourceforge.pmd.lang.jsp.ast.JspParserVisitor; import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.lang.rule.ImmutableLanguage; -public abstract class AbstractJspRule extends AbstractRule implements JspParserVisitor, ImmutableLanguage { +public abstract class AbstractJspRule extends AbstractRule implements JspParserVisitor { public AbstractJspRule() { super.setLanguage(LanguageRegistry.getLanguage(JspLanguageModule.NAME)); diff --git a/pmd-modelica/src/main/java/net/sourceforge/pmd/lang/modelica/rule/AbstractModelicaRule.java b/pmd-modelica/src/main/java/net/sourceforge/pmd/lang/modelica/rule/AbstractModelicaRule.java index a0b549c1dc..2e4c2b502f 100644 --- a/pmd-modelica/src/main/java/net/sourceforge/pmd/lang/modelica/rule/AbstractModelicaRule.java +++ b/pmd-modelica/src/main/java/net/sourceforge/pmd/lang/modelica/rule/AbstractModelicaRule.java @@ -13,12 +13,11 @@ import net.sourceforge.pmd.lang.modelica.ast.ModelicaNode; import net.sourceforge.pmd.lang.modelica.ast.ModelicaParserVisitor; import net.sourceforge.pmd.lang.modelica.internal.ModelicaProcessingStage; import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.lang.rule.ImmutableLanguage; /** * Base class for rules for Modelica language. */ -public abstract class AbstractModelicaRule extends AbstractRule implements ModelicaParserVisitor, ImmutableLanguage { +public abstract class AbstractModelicaRule extends AbstractRule implements ModelicaParserVisitor { public AbstractModelicaRule() { super.setLanguage(LanguageRegistry.getLanguage(ModelicaLanguageModule.NAME)); } diff --git a/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/rule/AbstractPLSQLRule.java b/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/rule/AbstractPLSQLRule.java index 6cbd82f289..150b2eb2e6 100644 --- a/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/rule/AbstractPLSQLRule.java +++ b/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/rule/AbstractPLSQLRule.java @@ -22,9 +22,8 @@ import net.sourceforge.pmd.lang.plsql.ast.ExecutableCode; import net.sourceforge.pmd.lang.plsql.ast.PLSQLNode; import net.sourceforge.pmd.lang.plsql.ast.PLSQLParserVisitor; import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.lang.rule.ImmutableLanguage; -public abstract class AbstractPLSQLRule extends AbstractRule implements PLSQLParserVisitor, ImmutableLanguage { +public abstract class AbstractPLSQLRule extends AbstractRule implements PLSQLParserVisitor { private static final Logger LOGGER = Logger.getLogger(AbstractPLSQLRule.class.getName()); private static final String CLASS_NAME = AbstractPLSQLRule.class.getName(); diff --git a/pmd-visualforce/src/main/java/net/sourceforge/pmd/lang/vf/rule/AbstractVfRule.java b/pmd-visualforce/src/main/java/net/sourceforge/pmd/lang/vf/rule/AbstractVfRule.java index b97f918eab..1944385c46 100644 --- a/pmd-visualforce/src/main/java/net/sourceforge/pmd/lang/vf/rule/AbstractVfRule.java +++ b/pmd-visualforce/src/main/java/net/sourceforge/pmd/lang/vf/rule/AbstractVfRule.java @@ -8,12 +8,11 @@ import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.lang.rule.ImmutableLanguage; import net.sourceforge.pmd.lang.vf.VfLanguageModule; import net.sourceforge.pmd.lang.vf.ast.VfNode; import net.sourceforge.pmd.lang.vf.ast.VfParserVisitor; -public abstract class AbstractVfRule extends AbstractRule implements VfParserVisitor, ImmutableLanguage { +public abstract class AbstractVfRule extends AbstractRule implements VfParserVisitor { public AbstractVfRule() { super.setLanguage(LanguageRegistry.getLanguage(VfLanguageModule.NAME)); diff --git a/pmd-vm/src/main/java/net/sourceforge/pmd/lang/vm/rule/AbstractVmRule.java b/pmd-vm/src/main/java/net/sourceforge/pmd/lang/vm/rule/AbstractVmRule.java index 2fad6d06e8..e780371a69 100644 --- a/pmd-vm/src/main/java/net/sourceforge/pmd/lang/vm/rule/AbstractVmRule.java +++ b/pmd-vm/src/main/java/net/sourceforge/pmd/lang/vm/rule/AbstractVmRule.java @@ -8,12 +8,11 @@ import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.lang.rule.ImmutableLanguage; import net.sourceforge.pmd.lang.vm.VmLanguageModule; import net.sourceforge.pmd.lang.vm.ast.VmNode; import net.sourceforge.pmd.lang.vm.ast.VmParserVisitor; -public abstract class AbstractVmRule extends AbstractRule implements VmParserVisitor, ImmutableLanguage { +public abstract class AbstractVmRule extends AbstractRule implements VmParserVisitor { public AbstractVmRule() { super.setLanguage(LanguageRegistry.getLanguage(VmLanguageModule.NAME)); diff --git a/pmd-xml/src/main/java/net/sourceforge/pmd/lang/xml/rule/AbstractXmlRule.java b/pmd-xml/src/main/java/net/sourceforge/pmd/lang/xml/rule/AbstractXmlRule.java index 7aac852637..136ba4634a 100644 --- a/pmd-xml/src/main/java/net/sourceforge/pmd/lang/xml/rule/AbstractXmlRule.java +++ b/pmd-xml/src/main/java/net/sourceforge/pmd/lang/xml/rule/AbstractXmlRule.java @@ -10,7 +10,6 @@ import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.ParserOptions; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.lang.rule.ImmutableLanguage; import net.sourceforge.pmd.lang.xml.XmlLanguageModule; import net.sourceforge.pmd.lang.xml.XmlParserOptions; import net.sourceforge.pmd.lang.xml.ast.XmlNode; @@ -21,7 +20,7 @@ import net.sourceforge.pmd.properties.PropertyDescriptor; * {@link #visit(XmlNode, RuleContext)} and can call super to visit * children. */ -public class AbstractXmlRule extends AbstractRule implements ImmutableLanguage { +public class AbstractXmlRule extends AbstractRule { @Deprecated public static final PropertyDescriptor COALESCING_DESCRIPTOR = XmlParserOptions.COALESCING_DESCRIPTOR; From d9de4a5f4e62693261d6855b926517cde4c86ace Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 25 Aug 2020 15:25:38 +0200 Subject: [PATCH 3/7] Remove AbstractRuleViolationFactory Since we have a compat branch for the designer, this doesn't need to exist --- .../lang/rule/AbstractRuleViolationFactory.java | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRuleViolationFactory.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRuleViolationFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRuleViolationFactory.java deleted file mode 100644 index 361111e08c..0000000000 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRuleViolationFactory.java +++ /dev/null @@ -1,15 +0,0 @@ -/* - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.rule; - -import net.sourceforge.pmd.lang.rule.impl.DefaultRuleViolationFactory; - -/** - * @deprecated This is kept for binary compatibility with the 6.x designer, yet will - * go away in 7.0. Use {@link DefaultRuleViolationFactory} - */ -@Deprecated -public class AbstractRuleViolationFactory extends DefaultRuleViolationFactory { -} From 7922fb756499b6470c9099153dca67658847bb6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 25 Aug 2020 15:36:20 +0200 Subject: [PATCH 4/7] Move MockRule into test sources --- .../net/sourceforge/pmd/RuleSetFactory.java | 8 --- .../java/net/sourceforge/pmd/RuleSetTest.java | 54 ++++++++++++------- .../sourceforge/pmd/lang/rule/MockRule.java | 9 +--- 3 files changed, 37 insertions(+), 34 deletions(-) rename pmd-core/src/{main => test}/java/net/sourceforge/pmd/lang/rule/MockRule.java (83%) 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 9331c6f792..5135a30a32 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -36,7 +36,6 @@ import org.xml.sax.SAXException; import net.sourceforge.pmd.RuleSet.RuleSetBuilder; import net.sourceforge.pmd.lang.Language; import net.sourceforge.pmd.lang.LanguageRegistry; -import net.sourceforge.pmd.lang.rule.MockRule; import net.sourceforge.pmd.lang.rule.RuleReference; import net.sourceforge.pmd.lang.rule.XPathRule; import net.sourceforge.pmd.rules.RuleFactory; @@ -697,13 +696,6 @@ public class RuleSetFactory { + ". PMD " + PMDVersion.getNextMajorRelease() + " will remove support for this deprecated Rule name usage."); } - } else if (referencedRule instanceof MockRule) { - if (LOG.isLoggable(Level.WARNING)) { - LOG.warning("Discontinue using Rule name " + otherRuleSetReferenceId - + " as it has been removed from PMD and no longer functions." - + " PMD " + PMDVersion.getNextMajorRelease() - + " will remove support for this Rule."); - } } else { if (LOG.isLoggable(Level.WARNING)) { LOG.warning("Discontinue using Rule name " + otherRuleSetReferenceId diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java index c66405e0cf..900355273b 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java @@ -31,16 +31,18 @@ import net.sourceforge.pmd.Report.ProcessingError; import net.sourceforge.pmd.RuleSet.RuleSetBuilder; import net.sourceforge.pmd.lang.Dummy2LanguageModule; import net.sourceforge.pmd.lang.DummyLanguageModule; +import net.sourceforge.pmd.lang.Language; import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.ast.DummyNode; import net.sourceforge.pmd.lang.ast.DummyRoot; import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.rule.MockRule; import net.sourceforge.pmd.lang.rule.RuleReference; import net.sourceforge.pmd.lang.rule.RuleTargetSelector; public class RuleSetTest { + private final Language dummyLang = LanguageRegistry.getLanguage(DummyLanguageModule.NAME); + @Test(expected = NullPointerException.class) public void testRuleSetRequiresName() { new RuleSetBuilder(new Random().nextLong()) @@ -169,7 +171,7 @@ public class RuleSetTest { } @Test - public void testApply0Rules() throws Exception { + public void testApply0Rules() { RuleSet ruleset = createRuleSetBuilder("ruleset").build(); verifyRuleSet(ruleset, 0, new HashSet()); } @@ -241,25 +243,23 @@ public class RuleSetTest { Rule rule = new MockRule(); - rule.setLanguage(LanguageRegistry.getLanguage(DummyLanguageModule.NAME)); assertFalse("Different languages should not apply", RuleSet.applies(rule, LanguageRegistry.getLanguage(Dummy2LanguageModule.NAME).getDefaultVersion())); - rule.setLanguage(LanguageRegistry.getLanguage(DummyLanguageModule.NAME)); assertTrue("Same language with no min/max should apply", - RuleSet.applies(rule, LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.5"))); + RuleSet.applies(rule, dummyLang.getVersion("1.5"))); - rule.setMinimumLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.5")); + rule.setMinimumLanguageVersion(dummyLang.getVersion("1.5")); assertTrue("Same language with valid min only should apply", - RuleSet.applies(rule, LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.5"))); + RuleSet.applies(rule, dummyLang.getVersion("1.5"))); - rule.setMaximumLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.6")); + rule.setMaximumLanguageVersion(dummyLang.getVersion("1.6")); assertTrue("Same language with valid min and max should apply", - RuleSet.applies(rule, LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.5"))); + RuleSet.applies(rule, dummyLang.getVersion("1.5"))); assertFalse("Same language with outside range of min/max should not apply", - RuleSet.applies(rule, LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.4"))); + RuleSet.applies(rule, dummyLang.getVersion("1.4"))); assertFalse("Same language with outside range of min/max should not apply", - RuleSet.applies(rule, LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.7"))); + RuleSet.applies(rule, dummyLang.getVersion("1.7"))); } @Test @@ -404,7 +404,6 @@ public class RuleSetTest { Rule rule = new FooRule(); rule.setName("FooRule1"); - rule.setLanguage(LanguageRegistry.getLanguage(DummyLanguageModule.NAME)); RuleSet ruleSet1 = createRuleSetBuilder("RuleSet1") .addRule(rule) .build(); @@ -420,7 +419,7 @@ public class RuleSetTest { Report r = new Report(); ctx.setReport(r); ctx.setSourceCodeFile(file); - ctx.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + ctx.setLanguageVersion(dummyLang.getDefaultVersion()); ruleSets.apply(makeCompilationUnits(), ctx); assertEquals("Violations", 2, r.getViolations().size()); @@ -442,7 +441,6 @@ public class RuleSetTest { public void copyConstructorDeepCopies() { Rule rule = new FooRule(); rule.setName("FooRule1"); - rule.setLanguage(LanguageRegistry.getLanguage(DummyLanguageModule.NAME)); RuleSet ruleSet1 = createRuleSetBuilder("RuleSet1") .addRule(rule) .build(); @@ -495,7 +493,7 @@ public class RuleSetTest { .build(); RuleContext context = new RuleContext(); context.setReport(new Report()); - context.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + context.setLanguageVersion(dummyLang.getDefaultVersion()); context.setSourceCodeFile(new File(RuleSetTest.class.getName() + ".ruleExceptionShouldBeReported")); context.setIgnoreExceptions(true); // the default ruleset.apply(makeCompilationUnits(), context); @@ -519,7 +517,7 @@ public class RuleSetTest { .build(); RuleContext context = new RuleContext(); context.setReport(new Report()); - context.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + context.setLanguageVersion(dummyLang.getDefaultVersion()); context.setSourceCodeFile(new File(RuleSetTest.class.getName() + ".ruleExceptionShouldBeThrownIfNotIgnored")); context.setIgnoreExceptions(false); ruleset.apply(makeCompilationUnits(), context); @@ -540,7 +538,7 @@ public class RuleSetTest { }).build(); RuleContext context = new RuleContext(); context.setReport(new Report()); - context.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + context.setLanguageVersion(dummyLang.getDefaultVersion()); context.setSourceCodeFile(new File(RuleSetTest.class.getName() + ".ruleExceptionShouldBeReported")); context.setIgnoreExceptions(true); // the default ruleset.apply(makeCompilationUnits(), context); @@ -581,7 +579,7 @@ public class RuleSetTest { }).build(); RuleContext context = new RuleContext(); context.setReport(new Report()); - context.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + context.setLanguageVersion(dummyLang.getDefaultVersion()); context.setSourceCodeFile(new File(RuleSetTest.class.getName() + ".ruleExceptionShouldBeReported")); context.setIgnoreExceptions(true); // the default RuleSets rulesets = new RuleSets(ruleset); @@ -595,4 +593,24 @@ public class RuleSetTest { assertEquals("There should be a violation", 1, context.getReport().getViolations().size()); } + + class MockRule extends net.sourceforge.pmd.lang.rule.MockRule { + + MockRule() { + super(); + setLanguage(dummyLang); + } + + MockRule(String name, String description, String message, String ruleSetName, RulePriority priority) { + super(name, description, message, ruleSetName, priority); + setLanguage(dummyLang); + } + + MockRule(String name, String description, String message, String ruleSetName) { + super(name, description, message, ruleSetName); + setLanguage(dummyLang); + } + + } + } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/MockRule.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/MockRule.java similarity index 83% rename from pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/MockRule.java rename to pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/MockRule.java index b613a336fc..0168ff1931 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/MockRule.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/MockRule.java @@ -1,4 +1,4 @@ -/** +/* * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ @@ -8,7 +8,6 @@ import static net.sourceforge.pmd.properties.constraints.NumericConstraints.inRa import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RulePriority; -import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.properties.PropertyFactory; @@ -18,17 +17,11 @@ import net.sourceforge.pmd.properties.PropertyFactory; * functional Rule is not needed. For example, during unit testing, or as an * editable surrogate used by IDE plugins. The Language of this Rule defaults to * Java. - * - * @deprecated This is not a supported API. You need the pmd-test module - * on your classpath, or pmd-core's test sources. This will be removed - * in 7.0.0 */ -@Deprecated public class MockRule extends AbstractRule { public MockRule() { super(); - setLanguage(LanguageRegistry.getLanguage("Dummy")); definePropertyDescriptor(PropertyFactory.intProperty("testIntProperty").desc("testIntProperty").require(inRange(1, 100)).defaultValue(1).build()); } From bceee2b54f961498fff53102bc4be0eda22246f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 25 Aug 2020 15:29:52 +0200 Subject: [PATCH 5/7] Disallow overriding language in rule reference --- .../net/sourceforge/pmd/RuleSetWriter.java | 3 +- .../pmd/lang/rule/AbstractRule.java | 6 +++ .../pmd/lang/rule/RuleReference.java | 23 +--------- .../sourceforge/pmd/RuleReferenceTest.java | 46 +++++++++++-------- .../pmd/AbstractRuleSetFactoryTest.java | 2 - 5 files changed, 37 insertions(+), 43 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java index d918d566ef..4719748ad0 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java @@ -168,7 +168,6 @@ public class RuleSetWriter { return null; } } else { - Language language = ruleReference.getOverriddenLanguage(); LanguageVersion minimumLanguageVersion = ruleReference.getOverriddenMinimumLanguageVersion(); LanguageVersion maximumLanguageVersion = ruleReference.getOverriddenMaximumLanguageVersion(); Boolean deprecated = ruleReference.isOverriddenDeprecated(); @@ -184,7 +183,7 @@ public class RuleSetWriter { .getOverriddenPropertiesByPropertyDescriptor(); List examples = ruleReference.getOverriddenExamples(); - return createSingleRuleElement(language, minimumLanguageVersion, maximumLanguageVersion, deprecated, + return createSingleRuleElement(null, minimumLanguageVersion, maximumLanguageVersion, deprecated, name, null, ref, message, externalInfoUrl, null, description, priority, propertyDescriptors, propertiesByPropertyDescriptor, examples); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java index 06e5201b6f..8cb94b80a6 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java @@ -107,6 +107,9 @@ public abstract class AbstractRule extends AbstractPropertySource implements Rul @Override public void setMinimumLanguageVersion(LanguageVersion minimumLanguageVersion) { + if (minimumLanguageVersion != null && !minimumLanguageVersion.getLanguage().equals(getLanguage())) { + throw new IllegalArgumentException("Version " + minimumLanguageVersion + " does not belong to language " + getLanguage()); + } this.minimumLanguageVersion = minimumLanguageVersion; } @@ -117,6 +120,9 @@ public abstract class AbstractRule extends AbstractPropertySource implements Rul @Override public void setMaximumLanguageVersion(LanguageVersion maximumLanguageVersion) { + if (maximumLanguageVersion != null && !maximumLanguageVersion.getLanguage().equals(getLanguage())) { + throw new IllegalArgumentException("Version " + maximumLanguageVersion + " does not belong to language " + getLanguage()); + } this.maximumLanguageVersion = maximumLanguageVersion; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/RuleReference.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/RuleReference.java index 29dec60d4c..ac121b2844 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/RuleReference.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/RuleReference.java @@ -15,7 +15,6 @@ import java.util.Objects; import net.sourceforge.pmd.Rule; import net.sourceforge.pmd.RulePriority; import net.sourceforge.pmd.RuleSetReference; -import net.sourceforge.pmd.lang.Language; import net.sourceforge.pmd.lang.LanguageVersion; import net.sourceforge.pmd.lang.ast.AstProcessingStage; import net.sourceforge.pmd.properties.PropertyDescriptor; @@ -30,7 +29,6 @@ import net.sourceforge.pmd.util.StringUtil; */ public class RuleReference extends AbstractDelegateRule { - private Language language; private LanguageVersion minimumLanguageVersion; private LanguageVersion maximumLanguageVersion; private Boolean deprecated; @@ -69,7 +67,6 @@ public class RuleReference extends AbstractDelegateRule { /** copy constructor */ private RuleReference(RuleReference ref) { - this.language = ref.language; this.minimumLanguageVersion = ref.minimumLanguageVersion; this.maximumLanguageVersion = ref.maximumLanguageVersion; this.deprecated = ref.deprecated; @@ -86,22 +83,6 @@ public class RuleReference extends AbstractDelegateRule { this.setRule(ref.getRule().deepCopy()); } - public Language getOverriddenLanguage() { - return language; - } - - // FIXME should we really allow overriding the language of a rule? - // I don't see any case where this wouldn't just make the rule fail during execution - @Override - public void setLanguage(Language language) { - // Only override if different than current value, or if already - // overridden. - if (!Objects.equals(language, super.getLanguage()) || this.language != null) { - this.language = language; - super.setLanguage(language); - } - } - public LanguageVersion getOverriddenMinimumLanguageVersion() { return minimumLanguageVersion; } @@ -111,8 +92,8 @@ public class RuleReference extends AbstractDelegateRule { // Only override if different than current value, or if already // overridden. if (!Objects.equals(minimumLanguageVersion, super.getMinimumLanguageVersion()) || this.minimumLanguageVersion != null) { + super.setMinimumLanguageVersion(minimumLanguageVersion); // might throw this.minimumLanguageVersion = minimumLanguageVersion; - super.setMinimumLanguageVersion(minimumLanguageVersion); } } @@ -125,8 +106,8 @@ public class RuleReference extends AbstractDelegateRule { // Only override if different than current value, or if already // overridden. if (!Objects.equals(maximumLanguageVersion, super.getMaximumLanguageVersion()) || this.maximumLanguageVersion != null) { + super.setMaximumLanguageVersion(maximumLanguageVersion); // might throw this.maximumLanguageVersion = maximumLanguageVersion; - super.setMaximumLanguageVersion(maximumLanguageVersion); } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleReferenceTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleReferenceTest.java index 4105d8ae5a..3d34233a90 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleReferenceTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleReferenceTest.java @@ -9,10 +9,12 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import org.junit.Assert; import org.junit.Test; import net.sourceforge.pmd.lang.Dummy2LanguageModule; import net.sourceforge.pmd.lang.DummyLanguageModule; +import net.sourceforge.pmd.lang.Language; import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.rule.MockRule; import net.sourceforge.pmd.lang.rule.RuleReference; @@ -34,7 +36,8 @@ public class RuleReferenceTest { final PropertyDescriptor PROPERTY1_DESCRIPTOR = PropertyFactory.stringProperty("property1").desc("Test property").defaultValue("").build(); MockRule rule = new MockRule(); rule.definePropertyDescriptor(PROPERTY1_DESCRIPTOR); - rule.setLanguage(LanguageRegistry.getLanguage(Dummy2LanguageModule.NAME)); + Language dummyLang = LanguageRegistry.getLanguage(DummyLanguageModule.NAME); + rule.setLanguage(dummyLang); rule.setName("name1"); rule.setProperty(PROPERTY1_DESCRIPTOR, "value1"); rule.setMessage("message1"); @@ -47,11 +50,8 @@ public class RuleReferenceTest { RuleReference ruleReference = new RuleReference(); ruleReference.setRule(rule); ruleReference.definePropertyDescriptor(PROPERTY2_DESCRIPTOR); - ruleReference.setLanguage(LanguageRegistry.getLanguage(DummyLanguageModule.NAME)); - ruleReference - .setMinimumLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.3")); - ruleReference - .setMaximumLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.7")); + ruleReference.setMinimumLanguageVersion(dummyLang.getVersion("1.3")); + ruleReference.setMaximumLanguageVersion(dummyLang.getVersion("1.7")); ruleReference.setDeprecated(true); ruleReference.setName("name2"); ruleReference.setProperty(PROPERTY1_DESCRIPTOR, "value2"); @@ -65,12 +65,30 @@ public class RuleReferenceTest { validateOverridenValues(PROPERTY1_DESCRIPTOR, PROPERTY2_DESCRIPTOR, ruleReference); } + @Test + public void testLanguageOverrideDisallowed() { + MockRule rule = new MockRule(); + Language dummyLang = LanguageRegistry.getLanguage(DummyLanguageModule.NAME); + rule.setLanguage(dummyLang); + + RuleReference ruleReference = new RuleReference(); + ruleReference.setRule(rule); + + Assert.assertThrows(UnsupportedOperationException.class, () -> ruleReference.setLanguage(LanguageRegistry.getLanguage(Dummy2LanguageModule.NAME))); + Assert.assertEquals(dummyLang, ruleReference.getLanguage()); + Assert.assertThrows(IllegalArgumentException.class, () -> ruleReference.setMaximumLanguageVersion(LanguageRegistry.getLanguage(Dummy2LanguageModule.NAME).getVersion("1.0"))); + Assert.assertEquals(rule.getMaximumLanguageVersion(), ruleReference.getOverriddenMaximumLanguageVersion()); + Assert.assertThrows(IllegalArgumentException.class, () -> ruleReference.setMinimumLanguageVersion(LanguageRegistry.getLanguage(Dummy2LanguageModule.NAME).getVersion("1.0"))); + Assert.assertEquals(rule.getMinimumLanguageVersion(), ruleReference.getMinimumLanguageVersion()); + } + @Test public void testDeepCopyOverride() { final PropertyDescriptor PROPERTY1_DESCRIPTOR = PropertyFactory.stringProperty("property1").desc("Test property").defaultValue("").build(); MockRule rule = new MockRule(); rule.definePropertyDescriptor(PROPERTY1_DESCRIPTOR); - rule.setLanguage(LanguageRegistry.getLanguage(Dummy2LanguageModule.NAME)); + Language dummyLang = LanguageRegistry.getLanguage(DummyLanguageModule.NAME); + rule.setLanguage(dummyLang); rule.setName("name1"); rule.setProperty(PROPERTY1_DESCRIPTOR, "value1"); rule.setMessage("message1"); @@ -83,11 +101,9 @@ public class RuleReferenceTest { RuleReference ruleReference = new RuleReference(); ruleReference.setRule(rule); ruleReference.definePropertyDescriptor(PROPERTY2_DESCRIPTOR); - ruleReference.setLanguage(LanguageRegistry.getLanguage(DummyLanguageModule.NAME)); - ruleReference - .setMinimumLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.3")); - ruleReference - .setMaximumLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.7")); + ruleReference.setLanguage(dummyLang); + ruleReference.setMinimumLanguageVersion(dummyLang.getVersion("1.3")); + ruleReference.setMaximumLanguageVersion(dummyLang.getVersion("1.7")); ruleReference.setDeprecated(true); ruleReference.setName("name2"); ruleReference.setProperty(PROPERTY1_DESCRIPTOR, "value2"); @@ -105,8 +121,6 @@ public class RuleReferenceTest { final PropertyDescriptor propertyDescriptor2, RuleReference ruleReference) { assertEquals("Override failed", LanguageRegistry.getLanguage(DummyLanguageModule.NAME), ruleReference.getLanguage()); - assertEquals("Override failed", LanguageRegistry.getLanguage(DummyLanguageModule.NAME), - ruleReference.getOverriddenLanguage()); assertEquals("Override failed", LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.3"), ruleReference.getMinimumLanguageVersion()); @@ -176,7 +190,6 @@ public class RuleReferenceTest { RuleReference ruleReference = new RuleReference(); ruleReference.setRule(rule); - ruleReference.setLanguage(LanguageRegistry.getLanguage(DummyLanguageModule.NAME)); ruleReference .setMinimumLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.3")); ruleReference @@ -190,9 +203,6 @@ public class RuleReferenceTest { ruleReference.setExternalInfoUrl("externalInfoUrl1"); ruleReference.setPriority(RulePriority.HIGH); - assertEquals("Override failed", LanguageRegistry.getLanguage(DummyLanguageModule.NAME), - ruleReference.getLanguage()); - assertNull("Override failed", ruleReference.getOverriddenLanguage()); assertEquals("Override failed", LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getVersion("1.3"), ruleReference.getMinimumLanguageVersion()); 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 bfa7f0a765..0394a990a4 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java @@ -446,8 +446,6 @@ public abstract class AbstractRuleSetFactoryTest { if (rule1 instanceof RuleReference) { RuleReference ruleReference1 = (RuleReference) rule1; RuleReference ruleReference2 = (RuleReference) rule2; - assertEquals(message + ", RuleReference overridden language", ruleReference1.getOverriddenLanguage(), - ruleReference2.getOverriddenLanguage()); assertEquals(message + ", RuleReference overridden minimum language version", ruleReference1.getOverriddenMinimumLanguageVersion(), ruleReference2.getOverriddenMinimumLanguageVersion()); From a8ad1ac4f6f873d0f2dd82e1ff2553c578ca6e02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 12 Sep 2020 14:39:01 +0200 Subject: [PATCH 6/7] Remove RuleViolationComparator --- .../net/sourceforge/pmd/RuleViolation.java | 10 ++- .../pmd/RuleViolationComparator.java | 74 ------------------- 2 files changed, 8 insertions(+), 76 deletions(-) delete mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/RuleViolationComparator.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleViolation.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleViolation.java index b832c7d8a3..ebefadb2ee 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleViolation.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleViolation.java @@ -23,8 +23,14 @@ public interface RuleViolation { * of a violation, filename first. The remaining parameters are compared * in an unspecified order. */ - // TODO in java 8 this can be a chained Comparator.comparing call - Comparator DEFAULT_COMPARATOR = RuleViolationComparator.INSTANCE; + Comparator DEFAULT_COMPARATOR = + Comparator.comparing(RuleViolation::getFilename) + .thenComparingInt(RuleViolation::getBeginLine) + .thenComparingInt(RuleViolation::getBeginColumn) + .thenComparing(RuleViolation::getDescription, Comparator.nullsLast(Comparator.naturalOrder())) + .thenComparingInt(RuleViolation::getEndLine) + .thenComparingInt(RuleViolation::getEndColumn) + .thenComparing(rv -> rv.getRule().getName()); /** * Get the Rule which identified this violation. diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleViolationComparator.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleViolationComparator.java deleted file mode 100644 index bc1b1adf39..0000000000 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleViolationComparator.java +++ /dev/null @@ -1,74 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd; - -import java.util.Comparator; - -/** - * Compares RuleViolations using the following criteria: - *

    - *
  1. Source file name
    2. - *
  2. Begin line
    3. - *
  3. Description
    4. - *
  4. Begin column
    5. - *
  5. End line
    6. - *
  6. End column
    7. - *
  7. Rule name
    8. - *
- * - * TODO why is begin line/begin column split?? would make more sense to use - * - filename - * - begin line - * - begin column - * - description - * - rule name - * - end line - * - end column - * - * @deprecated Use {@link RuleViolation#DEFAULT_COMPARATOR} - */ -@Deprecated -public final class RuleViolationComparator implements Comparator { - - public static final RuleViolationComparator INSTANCE = new RuleViolationComparator(); - - private RuleViolationComparator() { - } - - @Override - public int compare(final RuleViolation r1, final RuleViolation r2) { - int cmp = r1.getFilename().compareTo(r2.getFilename()); - if (cmp == 0) { - cmp = r1.getBeginLine() - r2.getBeginLine(); - if (cmp == 0) { - cmp = compare(r1.getDescription(), r2.getDescription()); - if (cmp == 0) { - cmp = r1.getBeginColumn() - r2.getBeginColumn(); - if (cmp == 0) { - cmp = r1.getEndLine() - r2.getEndLine(); - if (cmp == 0) { - cmp = r1.getEndColumn() - r2.getEndColumn(); - if (cmp == 0) { - cmp = r1.getRule().getName().compareTo(r2.getRule().getName()); - } - } - } - } - } - } - return cmp; - } - - private static int compare(final String s1, final String s2) { - // Treat null as larger - if (s1 == null) { - return 1; - } else if (s2 == null) { - return -1; - } else { - return s1.compareTo(s2); - } - } -} From 7a63ec3bd4e5b150f3b4481d38131b200b79f94d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 13 Sep 2020 02:18:17 +0200 Subject: [PATCH 7/7] Fix java tests --- .../pmd/lang/java/rule/codestyle/xml/FieldNamingConventions.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/FieldNamingConventions.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/FieldNamingConventions.xml index af2fc5fdbe..f2e821e3fd 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/FieldNamingConventions.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/FieldNamingConventions.xml @@ -111,8 +111,8 @@ public class Bar { cons_[A-Z][A-Z0-9]+ 2 - The enum constant name 'NET' doesn't match 'cons_[A-Z][A-Z0-9]+' The enum constant name 'ORG' doesn't match 'cons_[A-Z][A-Z0-9]+' + The enum constant name 'NET' doesn't match 'cons_[A-Z][A-Z0-9]+'