From abdb96238dd8f25bff31af3ec71cae1479dbd629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 28 Nov 2017 18:49:23 +0100 Subject: [PATCH 1/6] Improve builders interface --- .../builders/MultiValuePropertyBuilder.java | 6 +++-- .../SinglePackagedPropertyBuilder.java | 26 ++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/MultiValuePropertyBuilder.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/MultiValuePropertyBuilder.java index e3be94eac2..6405dd5770 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/MultiValuePropertyBuilder.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/MultiValuePropertyBuilder.java @@ -4,7 +4,9 @@ package net.sourceforge.pmd.properties.builders; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; import net.sourceforge.pmd.properties.MultiValuePropertyDescriptor; @@ -36,8 +38,8 @@ public abstract class MultiValuePropertyBuilder val) { - this.defaultValues = val; + public T defaultValues(Collection val) { + this.defaultValues = new ArrayList<>(val); return (T) this; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/SinglePackagedPropertyBuilder.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/SinglePackagedPropertyBuilder.java index 3fb0b755bf..2f7219d14c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/SinglePackagedPropertyBuilder.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/SinglePackagedPropertyBuilder.java @@ -5,6 +5,7 @@ package net.sourceforge.pmd.properties.builders; import java.util.Arrays; +import java.util.Collection; /** @@ -22,12 +23,35 @@ public abstract class SinglePackagedPropertyBuilder packs) { + if (packs != null) { + this.legalPackageNames = packs.toArray(new String[0]); + } + return (T) this; + } + } From 539d541ffdde810fbc1c0911a7001593013f5694 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 28 Nov 2017 19:00:59 +0100 Subject: [PATCH 2/6] Hide factories of MethodProperty, MethodMultiProperty, FileProperty from XPath rules refs #762 --- .../properties/PropertyDescriptorUtil.java | 12 ++-- .../pmd/properties/MethodPropertyTest.java | 57 +++++++++++++++---- 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorUtil.java index 43db194d09..c55f80765e 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorUtil.java @@ -15,13 +15,14 @@ import net.sourceforge.pmd.properties.builders.PropertyDescriptorExternalBuilder * @author Clément Fournier * @since 6.0.0 */ +// TODO make an enum public class PropertyDescriptorUtil { private static final Map> DESCRIPTOR_FACTORIES_BY_TYPE; static { - Map> temp = new HashMap<>(18); + Map> temp = new HashMap<>(16); temp.put("Boolean", BooleanProperty.extractor()); temp.put("List[Boolean]", BooleanMultiProperty.extractor()); @@ -30,7 +31,6 @@ public class PropertyDescriptorUtil { temp.put("Character", CharacterProperty.extractor()); temp.put("List[Character]", CharacterMultiProperty.extractor()); - temp.put("Integer", IntegerProperty.extractor()); temp.put("List[Integer]", IntegerMultiProperty.extractor()); temp.put("Long", LongProperty.extractor()); @@ -44,10 +44,10 @@ public class PropertyDescriptorUtil { temp.put("Class", TypeProperty.extractor()); temp.put("List[Class]", TypeMultiProperty.extractor()); - temp.put("Method", MethodProperty.extractor()); - temp.put("List[Method]", MethodMultiProperty.extractor()); - temp.put("File", FileProperty.extractor()); + // temp.put("Method", MethodProperty.extractor()); // hidden, see #762 + // temp.put("List[Method]", MethodMultiProperty.extractor()); // ditto + // temp.put("File", FileProperty.extractor()); // ditto DESCRIPTOR_FACTORIES_BY_TYPE = Collections.unmodifiableMap(temp); } @@ -56,7 +56,7 @@ public class PropertyDescriptorUtil { private PropertyDescriptorUtil() { } - + /** * Returns the full mappings from type ids to extractors. * diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java index 5d675607b4..392e99f12b 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java @@ -10,12 +10,15 @@ import static org.junit.Assert.assertTrue; import java.lang.reflect.Method; import java.util.HashMap; import java.util.List; +import java.util.Map; +import org.junit.Assume; import org.junit.Test; import net.sourceforge.pmd.properties.modules.MethodPropertyModule; import net.sourceforge.pmd.util.ClassUtil; + /** * Evaluates the functionality of the MethodProperty descriptor by testing its * ability to catch creation errors (illegal args), flag invalid methods per the @@ -33,7 +36,7 @@ public class MethodPropertyTest extends AbstractPackagedPropertyDescriptorTester private static final String[] METHOD_SIGNATURES = {"String#indexOf(int)", "String#substring(int,int)", "java.lang.String#substring(int,int)", "Integer#parseInt(String)", "java.util.HashMap#put(Object,Object)", - "HashMap#containsKey(Object)", }; + "HashMap#containsKey(Object)",}; public MethodPropertyTest() { @@ -41,14 +44,24 @@ public class MethodPropertyTest extends AbstractPackagedPropertyDescriptorTester } + @Override + @Test + public void testMissingPackageNames() { + Map attributes = getPropertyDescriptorValues(); + attributes.remove(PropertyDescriptorField.LEGAL_PACKAGES); + new MethodProperty("p", "d", ALL_METHODS[1], null, 1.0f); // no exception, null is ok + new MethodMultiProperty("p", "d", new Method[]{ALL_METHODS[2], ALL_METHODS[3]}, null, 1.0f); // no exception, null is ok + } + + @Test public void testAsStringOn() { Method method; - for (int i = 0; i < METHOD_SIGNATURES.length; i++) { - method = ValueParserConstants.METHOD_PARSER.valueOf(METHOD_SIGNATURES[i]); - assertNotNull("Unable to identify method: " + METHOD_SIGNATURES[i], method); + for (String methodSignature : METHOD_SIGNATURES) { + method = ValueParserConstants.METHOD_PARSER.valueOf(methodSignature); + assertNotNull("Unable to identify method: " + methodSignature, method); } } @@ -86,28 +99,50 @@ public class MethodPropertyTest extends AbstractPackagedPropertyDescriptorTester @Override protected PropertyDescriptor createProperty() { - return new MethodProperty("methodProperty", "asdf", ALL_METHODS[1], new String[] {"java.lang", "org.apache"}, - 1.0f); + return new MethodProperty("methodProperty", "asdf", ALL_METHODS[1], new String[]{"java.lang", "org.apache"}, + 1.0f); } @Override protected PropertyDescriptor> createMultiProperty() { - return new MethodMultiProperty("methodProperty", "asdf", new Method[] {ALL_METHODS[2], ALL_METHODS[3]}, - new String[] {"java.lang"}, 1.0f); + return new MethodMultiProperty("methodProperty", "asdf", new Method[]{ALL_METHODS[2], ALL_METHODS[3]}, + new String[]{"java.lang"}, 1.0f); } @Override protected PropertyDescriptor createBadProperty() { - return new MethodProperty("methodProperty", "asdf", ALL_METHODS[1], new String[] {"java.util"}, 1.0f); + return new MethodProperty("methodProperty", "asdf", ALL_METHODS[1], new String[]{"java.util"}, 1.0f); } @Override protected PropertyDescriptor> createBadMultiProperty() { - return new MethodMultiProperty("methodProperty", "asdf", new Method[] {ALL_METHODS[2], ALL_METHODS[3]}, - new String[] {"java.util"}, 1.0f); + return new MethodMultiProperty("methodProperty", "asdf", new Method[]{ALL_METHODS[2], ALL_METHODS[3]}, + new String[]{"java.util"}, 1.0f); } + + + @Override + @Test + public void testFactorySingleValue() { + Assume.assumeTrue("MethodProperty cannot be built from XPath (#762)", false); + } + + + @Override + @Test + public void testFactoryMultiValueCustomDelimiter() { + Assume.assumeTrue("MethodProperty cannot be built from XPath (#762)", false); + } + + + @Override + @Test + public void testFactoryMultiValueDefaultDelimiter() { + Assume.assumeTrue("MethodProperty cannot be built from XPath (#762)", false); + } + } From b81158ecf066f9aac4a51e7f4b7d093d77a46322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 28 Nov 2017 19:39:54 +0100 Subject: [PATCH 3/6] Convert PropertyDescriptorUtil to an enum and enrich its interface refs #763 --- .../net/sourceforge/pmd/RuleSetWriter.java | 4 +- .../properties/PropertyDescriptorField.java | 2 +- .../properties/PropertyDescriptorUtil.java | 101 ---------- .../pmd/properties/PropertyTypeId.java | 174 ++++++++++++++++++ .../sourceforge/pmd/rules/RuleFactory.java | 4 +- .../AbstractPropertyDescriptorTester.java | 4 +- .../pmd/properties/MethodPropertyTest.java | 2 +- 7 files changed, 182 insertions(+), 109 deletions(-) delete mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorUtil.java create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java 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 856ce4549f..0081100627 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java @@ -34,7 +34,7 @@ import net.sourceforge.pmd.lang.rule.RuleReference; import net.sourceforge.pmd.lang.rule.XPathRule; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.properties.PropertyDescriptorField; -import net.sourceforge.pmd.properties.PropertyDescriptorUtil; +import net.sourceforge.pmd.properties.PropertyTypeId; /** * This class represents a way to serialize a RuleSet to an XML configuration @@ -348,7 +348,7 @@ public class RuleSetWriter { final Element propertyElement = createPropertyValueElement(propertyDescriptor, propertyDescriptor.defaultValue()); propertyElement.setAttribute(PropertyDescriptorField.TYPE.attributeName(), - PropertyDescriptorUtil.typeIdFor(propertyDescriptor.type(), + PropertyTypeId.typeIdFor(propertyDescriptor.type(), propertyDescriptor.isMultiValue())); Map propertyValuesById = propertyDescriptor.attributeValuesById(); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorField.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorField.java index e6ea6468d2..e1d3f1dd09 100755 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorField.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorField.java @@ -15,7 +15,7 @@ import net.sourceforge.pmd.RuleSetFactory; * * @author Brian Remedios * @see RuleSetFactory - * @see PropertyDescriptorUtil + * @see PropertyTypeId */ public enum PropertyDescriptorField { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorUtil.java deleted file mode 100644 index c55f80765e..0000000000 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorUtil.java +++ /dev/null @@ -1,101 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.properties; - -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; - -import net.sourceforge.pmd.properties.builders.PropertyDescriptorExternalBuilder; - - -/** - * @author Clément Fournier - * @since 6.0.0 - */ -// TODO make an enum -public class PropertyDescriptorUtil { - - private static final Map> DESCRIPTOR_FACTORIES_BY_TYPE; - - - static { - Map> temp = new HashMap<>(16); - temp.put("Boolean", BooleanProperty.extractor()); - temp.put("List[Boolean]", BooleanMultiProperty.extractor()); - - temp.put("String", StringProperty.extractor()); - temp.put("List[String]", StringMultiProperty.extractor()); - temp.put("Character", CharacterProperty.extractor()); - temp.put("List[Character]", CharacterMultiProperty.extractor()); - - temp.put("Integer", IntegerProperty.extractor()); - temp.put("List[Integer]", IntegerMultiProperty.extractor()); - temp.put("Long", LongProperty.extractor()); - temp.put("List[Long]", LongMultiProperty.extractor()); - temp.put("Float", FloatProperty.extractor()); - temp.put("List[Float]", FloatMultiProperty.extractor()); - temp.put("Double", DoubleProperty.extractor()); - temp.put("List[Double]", DoubleMultiProperty.extractor()); - // temp.put("Enum", EnumeratedProperty.FACTORY); // TODO:cf we need new syntax in the xml to support that - // temp.put("List[Enum]", EnumeratedMultiProperty.FACTORY); - - temp.put("Class", TypeProperty.extractor()); - temp.put("List[Class]", TypeMultiProperty.extractor()); - - // temp.put("Method", MethodProperty.extractor()); // hidden, see #762 - // temp.put("List[Method]", MethodMultiProperty.extractor()); // ditto - // temp.put("File", FileProperty.extractor()); // ditto - - DESCRIPTOR_FACTORIES_BY_TYPE = Collections.unmodifiableMap(temp); - } - - - private PropertyDescriptorUtil() { - } - - - /** - * Returns the full mappings from type ids to extractors. - * - * @return The full mapping. - */ - public static Map> typeIdsToExtractors() { - return DESCRIPTOR_FACTORIES_BY_TYPE; - } - - - /** - * Gets the factory for the descriptor identified by the string id. - * - * @param typeId The identifier of the type - * - * @return The factory used to build new instances of a descriptor - */ - public static PropertyDescriptorExternalBuilder factoryFor(String typeId) { - return DESCRIPTOR_FACTORIES_BY_TYPE.get(typeId); - } - - - /** - * Gets the string representation of this type, as it should be given when defining a descriptor in the xml. - * - * @param valueType The type to look for - * @param multiValue Whether the descriptor is multivalued or not - * - * @return The type id - */ - public static String typeIdFor(Class valueType, boolean multiValue) { - - for (Map.Entry> entry : DESCRIPTOR_FACTORIES_BY_TYPE.entrySet()) { - if (entry.getValue().valueType() == valueType && entry.getValue().isMultiValue() == multiValue) { - return entry.getKey(); - } - } - return null; - } - - -} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java new file mode 100644 index 0000000000..2668c7fed7 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java @@ -0,0 +1,174 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.properties; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import net.sourceforge.pmd.properties.builders.PropertyDescriptorBuilderConversionWrapper; +import net.sourceforge.pmd.properties.builders.PropertyDescriptorExternalBuilder; + + +/** + * Enumerates the properties that can be built from the XML. Defining a property in + * the XML requires the {@code type} attribute, and the mapping between the values of + * this attribute and the concrete property that is built is encoded in the constants + * of this enum. + * + * @author Clément Fournier + * @see PropertyDescriptorExternalBuilder + * @since 6.0.0 + */ +public enum PropertyTypeId { + BOOLEAN("Boolean", BooleanProperty.extractor()), + BOOLEAN_LIST("List[Boolean]", BooleanMultiProperty.extractor()), + + STRING("String", StringProperty.extractor()), + STRING_LIST("List[String]", StringMultiProperty.extractor()), + CHARACTER("Character", CharacterProperty.extractor()), + CHARACTER_LIST("List[Character]", CharacterMultiProperty.extractor()), + + INTEGER("Integer", IntegerProperty.extractor()), + INTEGER_LIST("List[Integer]", IntegerMultiProperty.extractor()), + LONG("Long", LongProperty.extractor()), + LONG_LIST("List[Long]", LongMultiProperty.extractor()), + FLOAT("Float", FloatProperty.extractor()), + FLOAT_LIST("List[Float]", FloatMultiProperty.extractor()), + DOUBLE("Double", DoubleProperty.extractor()), + DOUBLE_LIST("List[Double]", DoubleMultiProperty.extractor()), + // ENUM("Enum", EnumeratedProperty.FACTORY), // TODO:cf we need new syntax in the xml to support that + // ENUM_LIST("List[Enum]", EnumeratedMultiProperty.FACTORY), + + CLASS("Class", TypeProperty.extractor()), + CLASS_LIST("List[Class]", TypeMultiProperty.extractor()); + + + private static final Map> DESCRIPTOR_FACTORIES_BY_TYPE; + private final String typeId; + private final PropertyDescriptorExternalBuilder factory; + + static { + Map> temp = new HashMap<>(); + for (PropertyTypeId id : values()) { + temp.put(id.typeId, id.factory); + } + DESCRIPTOR_FACTORIES_BY_TYPE = Collections.unmodifiableMap(temp); + } + + + PropertyTypeId(String id, PropertyDescriptorExternalBuilder factory) { + this.typeId = id; + this.factory = factory; + } + + + /** + * Gets the value of the type attribute represented by this constant. + * + * @return The type id + */ + public String getTypeId() { + return typeId; + } + + + /** + * Gets the factory associated to the type id, that can build the + * property from strings extracted from the XML. + * + * @return The factory + */ + public PropertyDescriptorExternalBuilder getFactory() { + return factory; + } + + + /** + * Returns true if the property corresponding to this factory is numeric, + * which means it can be safely cast to a {@link NumericPropertyDescriptor}. + * + * @return whether the property is numeric + */ + public boolean isPropertyNumeric() { + return factory instanceof PropertyDescriptorBuilderConversionWrapper.SingleValue.Numeric + || factory instanceof PropertyDescriptorBuilderConversionWrapper.MultiValue.Numeric; + } + + + /** + * Returns true if the property corresponding to this factory is packaged, + * which means it can be safely cast to a {@link PackagedPropertyDescriptor}. + * + * @return whether the property is packaged + */ + public boolean isPropertyPackaged() { + return factory instanceof PropertyDescriptorBuilderConversionWrapper.SingleValue.Packaged + || factory instanceof PropertyDescriptorBuilderConversionWrapper.MultiValue.Packaged; + } + + + /** + * Returns true if the property corresponding to this factory takes + * lists of values as its value. + * + * @return whether the property is multivalue + */ + public boolean isPropertyMultivalue() { + return factory.isMultiValue(); + } + + + /** + * Returns the value type of the property corresponding to this factory. + * This is the component type of the list if the property is multivalued. + * + * @return The value type of the property + */ + public Class propertyValueType() { + return factory.valueType(); + } + + + /** + * Returns the full mappings from type ids to factory. + * + * @return The full mapping. + */ + public static Map> typeIdsToExtractors() { + return DESCRIPTOR_FACTORIES_BY_TYPE; + } + + + /** + * Gets the factory for the descriptor identified by the string id. + * + * @param typeId The identifier of the type + * + * @return The factory used to build new instances of a descriptor + */ + public static PropertyDescriptorExternalBuilder factoryFor(String typeId) { + return DESCRIPTOR_FACTORIES_BY_TYPE.get(typeId); + } + + + /** + * Gets the string representation of this type, as it should be given + * when defining a descriptor in the xml. + * + * @param valueType The type to look for + * @param multiValue Whether the descriptor is multivalued or not + * + * @return The type id + */ + public static String typeIdFor(Class valueType, boolean multiValue) { + for (Map.Entry> entry : DESCRIPTOR_FACTORIES_BY_TYPE.entrySet()) { + if (entry.getValue().valueType() == valueType && entry.getValue().isMultiValue() == multiValue) { + return entry.getKey(); + } + } + return null; + } +} 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 84ce6b95d4..bdca4f45a2 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 @@ -28,7 +28,7 @@ import net.sourceforge.pmd.RulePriority; import net.sourceforge.pmd.lang.rule.RuleReference; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.properties.PropertyDescriptorField; -import net.sourceforge.pmd.properties.PropertyDescriptorUtil; +import net.sourceforge.pmd.properties.PropertyTypeId; import net.sourceforge.pmd.properties.builders.PropertyDescriptorExternalBuilder; @@ -304,7 +304,7 @@ public class RuleFactory { private static PropertyDescriptor parsePropertyDefinition(Element propertyElement) { String typeId = propertyElement.getAttribute(PropertyDescriptorField.TYPE.attributeName()); - PropertyDescriptorExternalBuilder pdFactory = PropertyDescriptorUtil.factoryFor(typeId); + PropertyDescriptorExternalBuilder pdFactory = PropertyTypeId.factoryFor(typeId); if (pdFactory == null) { throw new IllegalArgumentException("No property descriptor factory for type: " + typeId); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/properties/AbstractPropertyDescriptorTester.java b/pmd-core/src/test/java/net/sourceforge/pmd/properties/AbstractPropertyDescriptorTester.java index b0c56663ae..cbf11c0743 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/properties/AbstractPropertyDescriptorTester.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/properties/AbstractPropertyDescriptorTester.java @@ -61,7 +61,7 @@ public abstract class AbstractPropertyDescriptorTester { @SuppressWarnings("unchecked") protected final PropertyDescriptorExternalBuilder getSingleFactory() { - return (PropertyDescriptorExternalBuilder) PropertyDescriptorUtil.factoryFor(typeName); + return (PropertyDescriptorExternalBuilder) PropertyTypeId.factoryFor(typeName); } @@ -95,7 +95,7 @@ public abstract class AbstractPropertyDescriptorTester { @SuppressWarnings("unchecked") protected final PropertyDescriptorExternalBuilder> getMultiFactory() { - return (PropertyDescriptorExternalBuilder>) PropertyDescriptorUtil.factoryFor("List[" + typeName + "]"); + return (PropertyDescriptorExternalBuilder>) PropertyTypeId.factoryFor("List[" + typeName + "]"); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java index 392e99f12b..3ed12aa0fa 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java @@ -36,7 +36,7 @@ public class MethodPropertyTest extends AbstractPackagedPropertyDescriptorTester private static final String[] METHOD_SIGNATURES = {"String#indexOf(int)", "String#substring(int,int)", "java.lang.String#substring(int,int)", "Integer#parseInt(String)", "java.util.HashMap#put(Object,Object)", - "HashMap#containsKey(Object)",}; + "HashMap#containsKey(Object)", }; public MethodPropertyTest() { From 53f6c39f65143d5c6ec122a495c9583c188e30b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 29 Nov 2017 00:18:40 +0100 Subject: [PATCH 4/6] Make the constants available from the string type id --- .../pmd/properties/PropertyTypeId.java | 60 ++++++++++++------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java index 2668c7fed7..a3236133f9 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java @@ -14,9 +14,9 @@ import net.sourceforge.pmd.properties.builders.PropertyDescriptorExternalBuilder /** * Enumerates the properties that can be built from the XML. Defining a property in - * the XML requires the {@code type} attribute, and the mapping between the values of - * this attribute and the concrete property that is built is encoded in the constants - * of this enum. + * the XML requires the {@code type} attribute, which acts as a mnemonic for the type + * of the property that should be built. The mapping between the values of this attribute + * and the concrete property that is built is encoded in the constants of this enum. * * @author Clément Fournier * @see PropertyDescriptorExternalBuilder @@ -46,21 +46,21 @@ public enum PropertyTypeId { CLASS_LIST("List[Class]", TypeMultiProperty.extractor()); - private static final Map> DESCRIPTOR_FACTORIES_BY_TYPE; - private final String typeId; + private static final Map CONSTANTS_BY_MNEMONIC; + private final String stringId; private final PropertyDescriptorExternalBuilder factory; static { - Map> temp = new HashMap<>(); + Map temp = new HashMap<>(); for (PropertyTypeId id : values()) { - temp.put(id.typeId, id.factory); + temp.put(id.stringId, id); } - DESCRIPTOR_FACTORIES_BY_TYPE = Collections.unmodifiableMap(temp); + CONSTANTS_BY_MNEMONIC = Collections.unmodifiableMap(temp); } - + PropertyTypeId(String id, PropertyDescriptorExternalBuilder factory) { - this.typeId = id; + this.stringId = id; this.factory = factory; } @@ -68,10 +68,10 @@ public enum PropertyTypeId { /** * Gets the value of the type attribute represented by this constant. * - * @return The type id + * @return The string id */ - public String getTypeId() { - return typeId; + public String getStringId() { + return stringId; } @@ -133,39 +133,53 @@ public enum PropertyTypeId { /** - * Returns the full mappings from type ids to factory. + * Returns the full mappings from type ids to enum constants. * * @return The full mapping. */ - public static Map> typeIdsToExtractors() { - return DESCRIPTOR_FACTORIES_BY_TYPE; + public static Map typeIdsToConstants() { + return CONSTANTS_BY_MNEMONIC; } /** * Gets the factory for the descriptor identified by the string id. * - * @param typeId The identifier of the type + * @param stringId The identifier of the type * * @return The factory used to build new instances of a descriptor */ - public static PropertyDescriptorExternalBuilder factoryFor(String typeId) { - return DESCRIPTOR_FACTORIES_BY_TYPE.get(typeId); + public static PropertyDescriptorExternalBuilder factoryFor(String stringId) { + PropertyTypeId cons = CONSTANTS_BY_MNEMONIC.get(stringId); + return cons == null ? null : cons.factory; } /** - * Gets the string representation of this type, as it should be given + * Gets the enum constant corresponding to the given mnemonic. + * + * @param stringId A mnemonic for the property type + * + * @return A PropertyTypeId + */ + public static PropertyTypeId lookupMnemonic(String stringId) { + return CONSTANTS_BY_MNEMONIC.get(stringId); + } + + + /** + * Gets the string representation of this type, as it should be given * when defining a descriptor in the xml. * * @param valueType The type to look for * @param multiValue Whether the descriptor is multivalued or not * - * @return The type id + * @return The string id */ public static String typeIdFor(Class valueType, boolean multiValue) { - for (Map.Entry> entry : DESCRIPTOR_FACTORIES_BY_TYPE.entrySet()) { - if (entry.getValue().valueType() == valueType && entry.getValue().isMultiValue() == multiValue) { + for (Map.Entry entry : CONSTANTS_BY_MNEMONIC.entrySet()) { + if (entry.getValue().propertyValueType() == valueType + && entry.getValue().isPropertyMultivalue() == multiValue) { return entry.getKey(); } } From 36ba0e95814a84ead5d784f7643b1fb01825f76d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 30 Nov 2017 11:34:44 +0100 Subject: [PATCH 5/6] Add a handle on the value parser of the property --- .../pmd/properties/PropertyTypeId.java | 48 ++++++++++++------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java index a3236133f9..9d8fec5d6d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java @@ -23,32 +23,33 @@ import net.sourceforge.pmd.properties.builders.PropertyDescriptorExternalBuilder * @since 6.0.0 */ public enum PropertyTypeId { - BOOLEAN("Boolean", BooleanProperty.extractor()), - BOOLEAN_LIST("List[Boolean]", BooleanMultiProperty.extractor()), + BOOLEAN("Boolean", BooleanProperty.extractor(), ValueParserConstants.BOOLEAN_PARSER), + BOOLEAN_LIST("List[Boolean]", BooleanMultiProperty.extractor(), ValueParserConstants.BOOLEAN_PARSER), - STRING("String", StringProperty.extractor()), - STRING_LIST("List[String]", StringMultiProperty.extractor()), - CHARACTER("Character", CharacterProperty.extractor()), - CHARACTER_LIST("List[Character]", CharacterMultiProperty.extractor()), + STRING("String", StringProperty.extractor(), ValueParserConstants.STRING_PARSER), + STRING_LIST("List[String]", StringMultiProperty.extractor(), ValueParserConstants.STRING_PARSER), + CHARACTER("Character", CharacterProperty.extractor(), ValueParserConstants.CHARACTER_PARSER), + CHARACTER_LIST("List[Character]", CharacterMultiProperty.extractor(), ValueParserConstants.CHARACTER_PARSER), - INTEGER("Integer", IntegerProperty.extractor()), - INTEGER_LIST("List[Integer]", IntegerMultiProperty.extractor()), - LONG("Long", LongProperty.extractor()), - LONG_LIST("List[Long]", LongMultiProperty.extractor()), - FLOAT("Float", FloatProperty.extractor()), - FLOAT_LIST("List[Float]", FloatMultiProperty.extractor()), - DOUBLE("Double", DoubleProperty.extractor()), - DOUBLE_LIST("List[Double]", DoubleMultiProperty.extractor()), + INTEGER("Integer", IntegerProperty.extractor(), ValueParserConstants.INTEGER_PARSER), + INTEGER_LIST("List[Integer]", IntegerMultiProperty.extractor(), ValueParserConstants.INTEGER_PARSER), + LONG("Long", LongProperty.extractor(), ValueParserConstants.LONG_PARSER), + LONG_LIST("List[Long]", LongMultiProperty.extractor(), ValueParserConstants.LONG_PARSER), + FLOAT("Float", FloatProperty.extractor(), ValueParserConstants.FLOAT_PARSER), + FLOAT_LIST("List[Float]", FloatMultiProperty.extractor(), ValueParserConstants.FLOAT_PARSER), + DOUBLE("Double", DoubleProperty.extractor(), ValueParserConstants.DOUBLE_PARSER), + DOUBLE_LIST("List[Double]", DoubleMultiProperty.extractor(), ValueParserConstants.DOUBLE_PARSER), // ENUM("Enum", EnumeratedProperty.FACTORY), // TODO:cf we need new syntax in the xml to support that // ENUM_LIST("List[Enum]", EnumeratedMultiProperty.FACTORY), - CLASS("Class", TypeProperty.extractor()), - CLASS_LIST("List[Class]", TypeMultiProperty.extractor()); + CLASS("Class", TypeProperty.extractor(), ValueParserConstants.CLASS_PARSER), + CLASS_LIST("List[Class]", TypeMultiProperty.extractor(), ValueParserConstants.CLASS_PARSER); private static final Map CONSTANTS_BY_MNEMONIC; private final String stringId; private final PropertyDescriptorExternalBuilder factory; + private final ValueParser valueParser; static { Map temp = new HashMap<>(); @@ -59,9 +60,10 @@ public enum PropertyTypeId { } - PropertyTypeId(String id, PropertyDescriptorExternalBuilder factory) { + PropertyTypeId(String id, PropertyDescriptorExternalBuilder factory, ValueParser valueParser) { this.stringId = id; this.factory = factory; + this.valueParser = valueParser; } @@ -132,6 +134,18 @@ public enum PropertyTypeId { } + /** + * Gets the object used to parse the values of this property from a string. + * If the property is multivalued, the parser only parses individual components + * of the list. A list parser can be obtained with {@link ValueParserConstants#multi(ValueParser, char)}. + * + * @return The value parser + */ + public ValueParser getValueParser() { + return valueParser; + } + + /** * Returns the full mappings from type ids to enum constants. * From 060848a4553a308647936024b5a2c9868edf88a4 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 9 Dec 2017 10:11:10 +0100 Subject: [PATCH 6/6] Update release notes, refs #764 closes #762 closes #763 --- docs/pages/release_notes.md | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index eb1a123a63..4dc043141d 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -316,6 +316,16 @@ under debug / verbose builds you can even see individual hits / misses to the ca Finally, as this feature keeps maturing, we are gently pushing this forward. If not using incremental analysis, a warning will now be produced suggesting users to adopt it for better performance. +#### Rule and Report Properties + +The implementation around the properties support for rule properties and report properties has been revamped +to be fully typesafe. Along with that change, the support classes have been moved into an own +package `net.sourceforge.pmd.properties`. While there is no change necessary in the ruleset XML files, +when using/setting values for rules, there are adjustments necessary when declaring properties in Java-implemented +rules. + +The properties are now very well documented: [Working with properties](pmd_devdocs_working_with_properties.html). + ### Fixed Issues * all @@ -328,6 +338,8 @@ a warning will now be produced suggesting users to adopt it for better performan * [#618](https://github.com/pmd/pmd/issues/618): \[core] Incremental Analysis doesn't close file correctly on Windows upon a cache hit * [#643](https://github.com/pmd/pmd/issues/643): \[core] PMD Properties (dev-properties) breaks markup on CodeClimateRenderer * [#680](https://github.com/pmd/pmd/pull/680): \[core] Isolate classloaders for runtime and auxclasspath + * [#762](https://github.com/pmd/pmd/issues/762): \[core] Remove method and file property from available property descriptors for XPath rules + * [#763](https://github.com/pmd/pmd/issues/763): \[core] Turn property descriptor util into an enum and enrich its interface * apex * [#265](https://github.com/pmd/pmd/issues/265): \[apex] Make Rule suppression work * [#488](https://github.com/pmd/pmd/pull/488): \[apex] Use Apex lexer for CPD @@ -377,13 +389,16 @@ a warning will now be produced suggesting users to adopt it for better performan * The class `net.sourceforge.pmd.lang.dfa.NodeType` has been converted to an enum. All node types are enum members now instead of int constants. The names for node types are retained. -* The properties API (rule and report properties) have been revamped to be fully typesafe. This is everything - around `net.sourceforge.pmd.PropertyDescriptor`. +* The *Properties API* (rule and report properties) has been revamped to be fully typesafe. This is everything + around `net.sourceforge.pmd.properties.PropertyDescriptor`. + + Note: All classes related to properties have been moved into the package `net.sourceforge.pmd.properties`. * The rule classes `net.sourceforge.pmd.lang.apex.rule.apexunit.ApexUnitTestClassShouldHaveAsserts` and `net.sourceforge.pmd.lang.apex.rule.apexunit.ApexUnitTestShouldNotUseSeeAllDataTrue` have been renamed to `ApexUnitTestClassShouldHaveAssertsRule` and `ApexUnitTestShouldNotUseSeeAllDataTrueRule`, respectively. This is to comply with the naming convention, that each rule class should be suffixed with "Rule". + This change has no impact on custom rulesets, since the rule names themselves didn't change. * The never implemented method `PMD.processFiles(PMDConfiguration, RuleSetFactory, Collection, RuleContext, ProgressMonitor)` along with the interface `ProgressMonitor` has been removed. @@ -554,4 +569,5 @@ a warning will now be produced suggesting users to adopt it for better performan * [#746](https://github.com/pmd/pmd/pull/746): \[doc] Fix typo in incremental analysis log message - [Clément Fournier](https://github.com/oowekyala) * [#749](https://github.com/pmd/pmd/pull/749): \[doc] Update the documentation for properties - [Clément Fournier](https://github.com/oowekyala) * [#758](https://github.com/pmd/pmd/pull/758): \[core] Expose the full mapping from property type id to property extractor - [Clément Fournier](https://github.com/oowekyala) +* [#764](https://github.com/pmd/pmd/pull/764): \[core] Prevent method and file property use in XPath rules - [Clément Fournier](https://github.com/oowekyala)#