From 99cb4bfd6f4918fbab2c9e5f2abdb4a3a0c6e3c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 19 Sep 2019 00:02:40 +0200 Subject: [PATCH] Turn violation suppression descriptors into Optional --- .../main/java/net/sourceforge/pmd/Rule.java | 20 ++++++++++++---- .../net/sourceforge/pmd/RuleSetWriter.java | 2 +- .../pmd/properties/internal/SyntaxSet.java | 24 ++++--------------- .../properties/internal/XmlSyntaxUtils.java | 15 ++++++++++++ .../pmd/properties/internal/XmlUtils.java | 4 ++-- .../pmd/renderers/HTMLRenderer.java | 12 ++++++---- .../java/net/sourceforge/pmd/ReportTest.java | 6 +++-- .../pmd/AbstractRuleSetFactoryTest.java | 4 ++-- 8 files changed, 51 insertions(+), 36 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/Rule.java b/pmd-core/src/main/java/net/sourceforge/pmd/Rule.java index 215966f98e..9005643592 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/Rule.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/Rule.java @@ -5,6 +5,7 @@ package net.sourceforge.pmd; import java.util.List; +import java.util.Optional; import net.sourceforge.pmd.annotation.Experimental; import net.sourceforge.pmd.lang.Language; @@ -13,8 +14,9 @@ import net.sourceforge.pmd.lang.ParserOptions; import net.sourceforge.pmd.lang.ast.AstProcessingStage; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.rule.RuleTargetSelector; +import net.sourceforge.pmd.properties.PropertyDescriptor; +import net.sourceforge.pmd.properties.PropertyFactory; import net.sourceforge.pmd.properties.PropertySource; -import net.sourceforge.pmd.properties.StringProperty; /** * This is the basic Rule interface for PMD rules. @@ -32,16 +34,24 @@ public interface Rule extends PropertySource { * matching a regular expression. */ // TODO 7.0.0 use PropertyDescriptor> - StringProperty VIOLATION_SUPPRESS_REGEX_DESCRIPTOR = new StringProperty("violationSuppressRegex", - "Suppress violations with messages matching a regular expression", null, Integer.MAX_VALUE - 1); + PropertyDescriptor> VIOLATION_SUPPRESS_REGEX_DESCRIPTOR = + PropertyFactory.stringProperty("violationSuppressRegex") + .desc("Suppress violations with messages matching a regular expression") + .toOptional() + .defaultValue(Optional.empty()) + .build(); /** * Name of the property to universally suppress violations on nodes which * match a given relative XPath expression. */ // TODO 7.0.0 use PropertyDescriptor> - StringProperty VIOLATION_SUPPRESS_XPATH_DESCRIPTOR = new StringProperty("violationSuppressXPath", - "Suppress violations on nodes which match a given relative XPath expression.", null, Integer.MAX_VALUE - 2); + PropertyDescriptor> VIOLATION_SUPPRESS_XPATH_DESCRIPTOR = + PropertyFactory.stringProperty("violationSuppressXPath") + .desc("Suppress violations on nodes which match a given relative XPath expression.") + .toOptional() + .defaultValue(Optional.empty()) + .build(); /** * Get the Language of this Rule. 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 7ca76d7c97..f2c2c3371f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java @@ -309,7 +309,7 @@ public class RuleSetWriter { XmlSyntax xmlStrategy = propertyDescriptor.xmlStrategy(); - Element valueElt = createPropertyValueElement(xmlStrategy.getWriteElementName()); + Element valueElt = createPropertyValueElement(xmlStrategy.getWriteElementName(value)); xmlStrategy.toXml(valueElt, value); element.appendChild(valueElt); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/SyntaxSet.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/SyntaxSet.java index 3d88e187e6..1fc91b607c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/SyntaxSet.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/SyntaxSet.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd.properties.internal; +import static net.sourceforge.pmd.properties.internal.XmlSyntaxUtils.enquote; + import java.util.Collection; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -116,7 +118,8 @@ public final class SyntaxSet extends XmlSyntax { if (syntax == null) { throw err.error( element, - "Unexpected element name " + enquote(element.getTagName()) + ", expecting " + formatPossibilities() + "Unexpected element name " + enquote(element.getTagName()) + ", expecting " + + XmlSyntaxUtils.formatPossibilities(readIndex.keySet()) ); } else { return syntax.fromXml(element, err); @@ -128,25 +131,6 @@ public final class SyntaxSet extends XmlSyntax { forWrite.toXml(container, value); } - // nullable - private String formatPossibilities() { - Set strings = readIndex.keySet(); - if (strings.isEmpty()) { - return null; - } else if (strings.size() == 1) { - return enquote(strings.iterator().next()); - } else { - return "one of " + strings.stream().map(SyntaxSet::enquote).collect(Collectors.joining(", ")); - } - } - - private static Set readNames(Collection> syntaxes) { - return syntaxes.stream() - .flatMap(it -> it.getSupportedReadElementNames().stream()) - .collect(Collectors.toSet()); - } - - private static String enquote(String it) {return "'" + it + "'";} @Override public List examples() { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/XmlSyntaxUtils.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/XmlSyntaxUtils.java index 15c0510923..633f3cd301 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/XmlSyntaxUtils.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/XmlSyntaxUtils.java @@ -9,6 +9,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.function.Function; import java.util.function.Supplier; import java.util.regex.Pattern; @@ -97,4 +98,18 @@ public final class XmlSyntaxUtils { } ); } + + static String enquote(String it) {return "'" + it + "'";} + + + // nullable + static String formatPossibilities(Set names) { + if (names.isEmpty()) { + return null; + } else if (names.size() == 1) { + return enquote(names.iterator().next()); + } else { + return "one of " + names.stream().map(XmlSyntaxUtils::enquote).collect(Collectors.joining(", ")); + } + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/XmlUtils.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/XmlUtils.java index d644867c5c..a070d73385 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/XmlUtils.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/internal/XmlUtils.java @@ -34,8 +34,8 @@ public final class XmlUtils { public static T expectElement(XmlErrorReporter err, Element elt, XmlSyntax syntax) { - if (!elt.getTagName().equals(syntax.getWriteElementName())) { - err.warn(elt, "Expecting an element with name '" + syntax.getWriteElementName() + "'"); + if (!syntax.getSupportedReadElementNames().contains(elt.getTagName())) { + err.warn(elt, "Wrong name, expect " + XmlSyntaxUtils.formatPossibilities(syntax.getSupportedReadElementNames())); } else { return syntax.fromXml(elt, err); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/HTMLRenderer.java b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/HTMLRenderer.java index 21bda3970f..d50f25d76a 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/HTMLRenderer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/HTMLRenderer.java @@ -8,6 +8,7 @@ import java.io.IOException; import java.io.Writer; import java.util.Iterator; import java.util.List; +import java.util.Optional; import org.apache.commons.lang3.StringEscapeUtils; import org.apache.commons.lang3.StringUtils; @@ -32,8 +33,11 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { public static final String NAME = "html"; // TODO use PropertyDescriptor> : we need a "blank" default value - public static final StringProperty LINE_PREFIX = new StringProperty("linePrefix", - "Prefix for line number anchor in the source file.", null, 1); + public static final PropertyDescriptor> LINE_PREFIX = + PropertyFactory.stringProperty("linePrefix").desc("Prefix for line number anchor in the source file.") + .toOptional() + .defaultValue(Optional.empty()) + .build(); public static final PropertyDescriptor LINK_PREFIX = PropertyFactory.stringProperty("linkPrefix").desc("Path to HTML source.").defaultValue("").build(); @@ -74,7 +78,7 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { */ public void renderBody(Writer writer, Report report) throws IOException { linkPrefix = getProperty(LINK_PREFIX); - linePrefix = getProperty(LINE_PREFIX); + linePrefix = getProperty(LINE_PREFIX).orElse(null); replaceHtmlExtension = getProperty(HTML_EXTENSION); writer.write("

PMD report

"); @@ -94,7 +98,7 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { @Override public void start() throws IOException { linkPrefix = getProperty(LINK_PREFIX); - linePrefix = getProperty(LINE_PREFIX); + linePrefix = getProperty(LINE_PREFIX).orElse(null); replaceHtmlExtension = getProperty(HTML_EXTENSION); writer.write("PMD" + PMD.EOL); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/ReportTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/ReportTest.java index d6f0be3acc..f5a58668cb 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/ReportTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/ReportTest.java @@ -8,6 +8,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import java.util.Optional; + import org.junit.Test; import net.sourceforge.pmd.lang.LanguageRegistry; @@ -30,7 +32,7 @@ public class ReportTest extends RuleTst { public void testExclusionsInReportWithRuleViolationSuppressRegex() { Report rpt = new Report(); Rule rule = new FooRule(); - rule.setProperty(Rule.VIOLATION_SUPPRESS_REGEX_DESCRIPTOR, ".*No Foo.*"); + rule.setProperty(Rule.VIOLATION_SUPPRESS_REGEX_DESCRIPTOR, Optional.of(".*No Foo.*")); runTestFromString(TEST1, rule, rpt, defaultLanguage); assertTrue(rpt.getViolations().isEmpty()); assertEquals(1, rpt.getSuppressedViolations().size()); @@ -40,7 +42,7 @@ public class ReportTest extends RuleTst { public void testExclusionsInReportWithRuleViolationSuppressXPath() { Report rpt = new Report(); Rule rule = new FooRule(); - rule.setProperty(Rule.VIOLATION_SUPPRESS_XPATH_DESCRIPTOR, ".[@SimpleName = 'Foo']"); + rule.setProperty(Rule.VIOLATION_SUPPRESS_XPATH_DESCRIPTOR, Optional.of(".[@SimpleName = 'Foo']")); runTestFromString(TEST1, rule, rpt, defaultLanguage); assertTrue(rpt.getViolations().isEmpty()); assertEquals(1, rpt.getSuppressedViolations().size()); 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 a2266b2705..866166a7b5 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java @@ -176,7 +176,7 @@ public abstract class AbstractRuleSetFactoryTest { .append(PMD.EOL); } // Should not have violation suppress regex property - if (rule.getProperty(Rule.VIOLATION_SUPPRESS_REGEX_DESCRIPTOR) != null) { + if (rule.getProperty(Rule.VIOLATION_SUPPRESS_REGEX_DESCRIPTOR).isPresent()) { invalidRegexSuppress++; messages.append("Rule ") .append(fileName) @@ -188,7 +188,7 @@ public abstract class AbstractRuleSetFactoryTest { .append(PMD.EOL); } // Should not have violation suppress xpath property - if (rule.getProperty(Rule.VIOLATION_SUPPRESS_XPATH_DESCRIPTOR) != null) { + if (rule.getProperty(Rule.VIOLATION_SUPPRESS_XPATH_DESCRIPTOR).isPresent()) { invalidXPathSuppress++; messages.append("Rule ").append(fileName).append("/").append(rule.getName()).append(" should not have '").append(Rule.VIOLATION_SUPPRESS_XPATH_DESCRIPTOR.name()).append("', this is intended for end user customization only.").append(PMD.EOL); }