From 632ed17464af8b59d6095ae866af6dcd16c1ef80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 21 Nov 2017 11:24:46 +0100 Subject: [PATCH] Corrections for PR #723 --- .../net/sourceforge/pmd/RuleSetFactory.java | 1 + .../pmd/{ => rules}/RuleBuilder.java | 11 ++++---- .../pmd/{ => rules}/RuleFactory.java | 28 +++++++++---------- 3 files changed, 21 insertions(+), 19 deletions(-) rename pmd-core/src/main/java/net/sourceforge/pmd/{ => rules}/RuleBuilder.java (95%) rename pmd-core/src/main/java/net/sourceforge/pmd/{ => rules}/RuleFactory.java (94%) 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 6e01f9805c..466c1068c9 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -35,6 +35,7 @@ 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; import net.sourceforge.pmd.util.ResourceLoader; /** diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleBuilder.java b/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleBuilder.java similarity index 95% rename from pmd-core/src/main/java/net/sourceforge/pmd/RuleBuilder.java rename to pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleBuilder.java index b01ad9da8a..5794341df3 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleBuilder.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleBuilder.java @@ -2,7 +2,7 @@ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ -package net.sourceforge.pmd; +package net.sourceforge.pmd.rules; import java.util.ArrayList; import java.util.List; @@ -10,6 +10,8 @@ import java.util.List; import org.apache.commons.lang3.StringUtils; import org.w3c.dom.Element; +import net.sourceforge.pmd.Rule; +import net.sourceforge.pmd.RulePriority; import net.sourceforge.pmd.lang.Language; import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.LanguageVersion; @@ -44,7 +46,7 @@ class RuleBuilder { private boolean isUsesTyperesolution; - RuleBuilder(String name, String clazz, String language) { + /* default */ RuleBuilder(String name, String clazz, String language) { this.name = name; language(language); className(clazz); @@ -113,11 +115,10 @@ class RuleBuilder { } - public RuleBuilder since(String sinceStr) { + public void since(String sinceStr) { if (StringUtils.isNotBlank(sinceStr)) { since = sinceStr; } - return this; } @@ -213,7 +214,7 @@ class RuleBuilder { rule.setExternalInfoUrl(externalInfoUrl); rule.setDeprecated(isDeprecated); rule.setDescription(description); - rule.setPriority(priority == null ? RulePriority.valueOf(666) : priority); + rule.setPriority(priority == null ? RulePriority.LOW : priority); for (String example : examples) { rule.addExample(example); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleFactory.java similarity index 94% rename from pmd-core/src/main/java/net/sourceforge/pmd/RuleFactory.java rename to pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleFactory.java index 861ec769fb..5f35faee5d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleFactory.java @@ -2,16 +2,18 @@ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ -package net.sourceforge.pmd; +package net.sourceforge.pmd.rules; import static net.sourceforge.pmd.properties.PropertyDescriptorField.DEFAULT_VALUE; import java.util.AbstractMap.SimpleEntry; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.logging.Logger; import org.apache.commons.lang3.StringUtils; import org.w3c.dom.Attr; @@ -20,6 +22,8 @@ import org.w3c.dom.NamedNodeMap; import org.w3c.dom.Node; import org.w3c.dom.NodeList; +import net.sourceforge.pmd.Rule; +import net.sourceforge.pmd.RulePriority; import net.sourceforge.pmd.lang.rule.RuleReference; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.properties.PropertyDescriptorField; @@ -35,6 +39,7 @@ import net.sourceforge.pmd.properties.builders.PropertyDescriptorExternalBuilder */ public class RuleFactory { + private static final Logger LOG = Logger.getLogger(RuleFactory.class.getName()); private static final String DEPRECATED = "deprecated"; private static final String NAME = "name"; @@ -49,12 +54,8 @@ public class RuleFactory { private static final String DESCRIPTION = "description"; private static final String PROPERTY = "property"; private static final String CLASS = "class"; - - - /** Create a new rule factory. */ - public RuleFactory() { - - } + + public static final List REQUIRED_ATTRIBUTES = Collections.unmodifiableList(Arrays.asList(NAME, CLASS)); /** @@ -194,8 +195,8 @@ public class RuleFactory { try { rule = builder.build(); } catch (ClassNotFoundException | IllegalAccessException | InstantiationException e) { - e.printStackTrace(); - return null; + LOG.severe(e.getMessage()); + throw new RuntimeException(e); } @@ -209,9 +210,8 @@ public class RuleFactory { private void checkRequiredAttributesArePresent(Element ruleElement) { // add an attribute name here to make it required - final List required = Arrays.asList(NAME, CLASS); - for (String att : required) { + for (String att : REQUIRED_ATTRIBUTES) { if (!ruleElement.hasAttribute(att)) { throw new IllegalArgumentException("Missing '" + att + "' attribute"); } @@ -308,7 +308,7 @@ public class RuleFactory { * @return True if this element defines a new property, false if this is just stating a value */ private static boolean isPropertyDefinition(Element node) { - return StringUtils.isNotBlank(node.getAttribute(PropertyDescriptorField.TYPE.attributeName())); + return node.hasAttribute(PropertyDescriptorField.TYPE.attributeName()); } @@ -325,7 +325,7 @@ public class RuleFactory { PropertyDescriptorExternalBuilder pdFactory = PropertyDescriptorUtil.factoryFor(typeId); if (pdFactory == null) { - throw new RuntimeException("No property descriptor factory for type: " + typeId); + throw new IllegalArgumentException("No property descriptor factory for type: " + typeId); } Map values = new HashMap<>(); @@ -342,7 +342,7 @@ public class RuleFactory { if (children.getLength() == 1) { values.put(DEFAULT_VALUE, children.item(0).getTextContent()); } else { - throw new RuntimeException("No value defined!"); + throw new IllegalArgumentException("No value defined!"); } }