hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Merge branch 'pr-764'

Browse Source
This commit is contained in:
Andreas Dangel
2017-12-09 10:12:12 +01:00
parent 2f4e1ba20d 060848a455
commit 1bdb412c09
10 changed files with 301 additions and 123 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
docs/pages/release_notes.md
View File

@ -332,6 +332,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
@ -344,6 +354,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
@ -397,13 +409,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<File>, RuleContext, ProgressMonitor)` along with the interface `ProgressMonitor` has been removed.
@ -574,6 +589,7 @@ 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)
* [#771](https://github.com/pmd/pmd/pull/771): \[apex] Fix Apex metrics framework failing on triggers, refs #768 - [Clément Fournier](https://github.com/oowekyala)
* [#774](https://github.com/pmd/pmd/pull/774): \[java] Avoid using FileInput/Output - see JDK-8080225 - [Chas Honton](https://github.com/chonton)

4
pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java
View File

@ -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<PropertyDescriptorField, String> propertyValuesById = propertyDescriptor.attributeValuesById();

2
pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorField.java
View File

@ -15,7 +15,7 @@ import net.sourceforge.pmd.RuleSetFactory;
*
* @author Brian Remedios
* @see RuleSetFactory
* @see PropertyDescriptorUtil
* @see PropertyTypeId
*/
public enum PropertyDescriptorField {

101
pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorUtil.java
View File

@ -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
*/
public class PropertyDescriptorUtil {
private static final Map<String, PropertyDescriptorExternalBuilder<?>> DESCRIPTOR_FACTORIES_BY_TYPE;
static {
Map<String, PropertyDescriptorExternalBuilder<?>> temp = new HashMap<>(18);
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());
temp.put("List[Method]", MethodMultiProperty.extractor());
temp.put("File", FileProperty.extractor());
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<String, PropertyDescriptorExternalBuilder<?>> 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<String, PropertyDescriptorExternalBuilder<?>> entry : DESCRIPTOR_FACTORIES_BY_TYPE.entrySet()) {
if (entry.getValue().valueType() == valueType && entry.getValue().isMultiValue() == multiValue) {
return entry.getKey();
}
}
return null;
}
}

202
pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyTypeId.java Normal file
View File

@ -0,0 +1,202 @@
/**
* 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, 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
* @since 6.0.0
*/
public enum PropertyTypeId {
BOOLEAN("Boolean", BooleanProperty.extractor(), ValueParserConstants.BOOLEAN_PARSER),
BOOLEAN_LIST("List[Boolean]", BooleanMultiProperty.extractor(), ValueParserConstants.BOOLEAN_PARSER),
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(), 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(), ValueParserConstants.CLASS_PARSER),
CLASS_LIST("List[Class]", TypeMultiProperty.extractor(), ValueParserConstants.CLASS_PARSER);
private static final Map<String, PropertyTypeId> CONSTANTS_BY_MNEMONIC;
private final String stringId;
private final PropertyDescriptorExternalBuilder<?> factory;
private final ValueParser<?> valueParser;
static {
Map<String, PropertyTypeId> temp = new HashMap<>();
for (PropertyTypeId id : values()) {
temp.put(id.stringId, id);
}
CONSTANTS_BY_MNEMONIC = Collections.unmodifiableMap(temp);
}
PropertyTypeId(String id, PropertyDescriptorExternalBuilder<?> factory, ValueParser<?> valueParser) {
this.stringId = id;
this.factory = factory;
this.valueParser = valueParser;
}
/**
* Gets the value of the type attribute represented by this constant.
*
* @return The string id
*/
public String getStringId() {
return stringId;
}
/**
* 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();
}
/**
* 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.
*
* @return The full mapping.
*/
public static Map<String, PropertyTypeId> typeIdsToConstants() {
return CONSTANTS_BY_MNEMONIC;
}
/**
* Gets the factory for the descriptor identified by the string id.
*
* @param stringId The identifier of the type
*
* @return The factory used to build new instances of a descriptor
*/
public static PropertyDescriptorExternalBuilder<?> factoryFor(String stringId) {
PropertyTypeId cons = CONSTANTS_BY_MNEMONIC.get(stringId);
return cons == null ? null : cons.factory;
}
/**
* 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 string id
*/
public static String typeIdFor(Class<?> valueType, boolean multiValue) {
for (Map.Entry<String, PropertyTypeId> entry : CONSTANTS_BY_MNEMONIC.entrySet()) {
if (entry.getValue().propertyValueType() == valueType
&& entry.getValue().isPropertyMultivalue() == multiValue) {
return entry.getKey();
}
}
return null;
}
}

6
pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/MultiValuePropertyBuilder.java
View File

@ -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<V, T extends MultiValuePropertyB
* @return The same builder
*/
@SuppressWarnings("unchecked")
public T defaultValues(List<V> val) {
this.defaultValues = val;
public T defaultValues(Collection<? extends V> val) {
this.defaultValues = new ArrayList<>(val);
return (T) this;
}

26
pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/SinglePackagedPropertyBuilder.java
View File

@ -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<V, T extends SinglePackagedP
}
/**
* Specify the allowed package prefixes.
*
* @param packs The package prefixes
*
* @return The same builder
*/
@SuppressWarnings("unchecked")
public T legalPackageNames(String[] packs) {
public T legalPackageNames(String... packs) {
if (packs != null) {
this.legalPackageNames = Arrays.copyOf(packs, packs.length);
}
return (T) this;
}
/**
* Specify the allowed package prefixes.
*
* @param packs The package prefixes
*
* @return The same builder
*/
@SuppressWarnings("unchecked")
public T legalPackageNames(Collection<String> packs) {
if (packs != null) {
this.legalPackageNames = packs.toArray(new String[0]);
}
return (T) this;
}
}

4
pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleFactory.java
View File

@ -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);
}

4
pmd-core/src/test/java/net/sourceforge/pmd/properties/AbstractPropertyDescriptorTester.java
View File

@ -61,7 +61,7 @@ public abstract class AbstractPropertyDescriptorTester<T> {
@SuppressWarnings("unchecked")
protected final PropertyDescriptorExternalBuilder<T> getSingleFactory() {
return (PropertyDescriptorExternalBuilder<T>) PropertyDescriptorUtil.factoryFor(typeName);
return (PropertyDescriptorExternalBuilder<T>) PropertyTypeId.factoryFor(typeName);
}
@ -95,7 +95,7 @@ public abstract class AbstractPropertyDescriptorTester<T> {
@SuppressWarnings("unchecked")
protected final PropertyDescriptorExternalBuilder<List<T>> getMultiFactory() {
return (PropertyDescriptorExternalBuilder<List<T>>) PropertyDescriptorUtil.factoryFor("List[" + typeName + "]");
return (PropertyDescriptorExternalBuilder<List<T>>) PropertyTypeId.factoryFor("List[" + typeName + "]");
}

55
pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java
View File

@ -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
@ -41,14 +44,24 @@ public class MethodPropertyTest extends AbstractPackagedPropertyDescriptorTester
}
@Override
@Test
public void testMissingPackageNames() {
Map<PropertyDescriptorField, String> 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<Method> 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<List<Method>> 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<Method> 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<List<Method>> 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);
}
}