diff --git a/pmd-core/pom.xml b/pmd-core/pom.xml index f762c76d97..55b84f03c8 100644 --- a/pmd-core/pom.xml +++ b/pmd-core/pom.xml @@ -121,6 +121,11 @@ junit test + + org.hamcrest + hamcrest-library + test + pl.pragmatists JUnitParams diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java index 9e2de28f7a..0e7a9b721f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java @@ -6,6 +6,10 @@ package net.sourceforge.pmd.lang.ast.xpath; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.logging.Level; +import java.util.logging.Logger; import net.sourceforge.pmd.lang.ast.Node; @@ -14,6 +18,11 @@ import net.sourceforge.pmd.lang.ast.Node; */ public class Attribute { + + private static final Logger LOG = Logger.getLogger(Attribute.class.getName()); + private static final ConcurrentMap DETECTED_DEPRECATED_ATTRIBUTES = new ConcurrentHashMap<>(); + + private static final Object[] EMPTY_OBJ_ARRAY = new Object[0]; private Node parent; private String name; @@ -38,13 +47,17 @@ public class Attribute { if (value != null) { return value; } + + if (method.isAnnotationPresent(Deprecated.class) && LOG.isLoggable(Level.WARNING) + && DETECTED_DEPRECATED_ATTRIBUTES.putIfAbsent(getLoggableAttributeName(), Boolean.TRUE) == null) { + LOG.warning("Use of deprecated attribute '" + getLoggableAttributeName() + "' in xpath query"); + } + // this lazy loading reduces calls to Method.invoke() by about 90% try { - return method.invoke(parent, EMPTY_OBJ_ARRAY); - } catch (IllegalAccessException iae) { + return value = method.invoke(parent, EMPTY_OBJ_ARRAY); + } catch (IllegalAccessException | InvocationTargetException iae) { iae.printStackTrace(); - } catch (InvocationTargetException ite) { - ite.printStackTrace(); } return null; } @@ -65,6 +78,10 @@ public class Attribute { return stringValue; } + private String getLoggableAttributeName() { + return parent.getXPathNodeName() + "/@" + name; + } + public String getName() { return name; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java index 27a4800b7d..2096309099 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java @@ -101,6 +101,9 @@ public class AttributeAxisIterator implements Iterator { return new Attribute(node, m.name, m.method); } + + + private static final Set> CONSIDERED_RETURN_TYPES = new HashSet<>(Arrays.>asList(Integer.TYPE, Boolean.TYPE, Double.TYPE, String.class, Long.TYPE, Character.TYPE, Float.TYPE)); @@ -109,10 +112,8 @@ public class AttributeAxisIterator implements Iterator { protected boolean isAttributeAccessor(Method method) { String methodName = method.getName(); - boolean deprecated = method.getAnnotation(Deprecated.class) != null; - return !deprecated - && CONSIDERED_RETURN_TYPES.contains(method.getReturnType()) + return CONSIDERED_RETURN_TYPES.contains(method.getReturnType()) && method.getParameterTypes().length == 0 && !methodName.startsWith("jjt") && !FILTERED_OUT_NAMES.contains(methodName); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java index c0deef7585..f17a8c2a3e 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java @@ -6,15 +6,22 @@ package net.sourceforge.pmd.lang.ast; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import org.jaxen.JaxenException; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import net.sourceforge.pmd.junit.JavaUtilLoggingRule; +import net.sourceforge.pmd.lang.ast.xpath.Attribute; + import junitparams.JUnitParamsRunner; import junitparams.Parameters; + /** * Unit test for {@link AbstractNode}. */ @@ -23,6 +30,9 @@ public class AbstractNodeTest { private static final int NUM_CHILDREN = 3; private static final int NUM_GRAND_CHILDREN = 3; + @Rule + public JavaUtilLoggingRule loggingRule = new JavaUtilLoggingRule(Attribute.class.getName()); + // Note that in order to successfully run JUnitParams, we need to explicitly use `Integer` instead of `int` private Integer[] childrenIndexes() { @@ -65,9 +75,10 @@ public class AbstractNodeTest { return new DummyNode(nextId()); } - private static void addChild(final Node parent, final Node child) { + private static Node addChild(final Node parent, final Node child) { parent.jjtAddChild(child, parent.jjtGetNumChildren()); // Append child at the end child.jjtSetParent(parent); + return parent; } @Before @@ -216,4 +227,25 @@ public class AbstractNodeTest { // Check that this node still does not have any children assertEquals(0, grandChild.jjtGetNumChildren()); } + + + @Test + public void testDeprecatedAttributeXPathQuery() throws JaxenException { + class MyRootNode extends DummyNode implements RootNode { + + private MyRootNode(int id) { + super(id); + } + } + + addChild(new MyRootNode(nextId()), new DummyNodeWithDeprecatedAttribute(2)).findChildNodesWithXPath("//dummyNode[@Size=1]"); + + String log = loggingRule.getLog(); + + assertTrue(log.contains("deprecated")); + assertTrue(log.contains("attribute")); + assertTrue(log.contains("dummyNode/@Size")); + } + + } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNodeWithDeprecatedAttribute.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNodeWithDeprecatedAttribute.java new file mode 100644 index 0000000000..7f6f67d8f7 --- /dev/null +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNodeWithDeprecatedAttribute.java @@ -0,0 +1,23 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.ast; + +/** + * @author Clément Fournier + * @since 6.3.0 + */ +public class DummyNodeWithDeprecatedAttribute extends DummyNode { + + + public DummyNodeWithDeprecatedAttribute(int id) { + super(id); + } + + // this is the deprecated attribute + @Deprecated + public int getSize() { + return 2; + } +} diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java index 89bdc7772d..e8326ca32d 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java @@ -4,19 +4,33 @@ package net.sourceforge.pmd.lang.ast.xpath; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; + import java.util.HashMap; import java.util.Map; +import org.hamcrest.collection.IsMapContaining; import org.junit.Assert; import org.junit.Test; import net.sourceforge.pmd.lang.ast.DummyNode; +import net.sourceforge.pmd.lang.ast.DummyNodeWithDeprecatedAttribute; +import net.sourceforge.pmd.lang.ast.Node; + /** * Unit test for {@link AttributeAxisIterator} */ public class AttributeAxisIteratorTest { + @Test + public void testAttributeDeprecation() { + Node dummy = new DummyNodeWithDeprecatedAttribute(2); + assertThat(toMap(new AttributeAxisIterator(dummy)), IsMapContaining.hasKey("Size")); + } + /** * Test hasNext and next. */ @@ -27,18 +41,24 @@ public class AttributeAxisIteratorTest { dummyNode.testingOnlySetBeginColumn(1); AttributeAxisIterator it = new AttributeAxisIterator(dummyNode); + Map atts = toMap(it); + Assert.assertEquals(7, atts.size()); + assertTrue(atts.containsKey("BeginColumn")); + assertTrue(atts.containsKey("BeginLine")); + assertTrue(atts.containsKey("FindBoundary")); + assertTrue(atts.containsKey("Image")); + assertTrue(atts.containsKey("SingleLine")); + assertTrue(atts.containsKey("EndColumn")); + assertTrue(atts.containsKey("EndLine")); + } + + + private Map toMap(AttributeAxisIterator it) { Map atts = new HashMap<>(); while (it.hasNext()) { Attribute attribute = it.next(); atts.put(attribute.getName(), attribute); } - Assert.assertEquals(7, atts.size()); - Assert.assertTrue(atts.containsKey("BeginColumn")); - Assert.assertTrue(atts.containsKey("BeginLine")); - Assert.assertTrue(atts.containsKey("FindBoundary")); - Assert.assertTrue(atts.containsKey("Image")); - Assert.assertTrue(atts.containsKey("SingleLine")); - Assert.assertTrue(atts.containsKey("EndColumn")); - Assert.assertTrue(atts.containsKey("EndLine")); + return atts; } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/coverage/PMDCoverageTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/coverage/PMDCoverageTest.java index addf48577f..a4b82e3736 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/coverage/PMDCoverageTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/coverage/PMDCoverageTest.java @@ -58,6 +58,7 @@ public class PMDCoverageTest { assertFalse("There was at least one exception mentioned in the output", output.getLog().contains("Exception applying rule")); assertFalse("Wrong configuration? Ruleset not found", output.getLog().contains("Ruleset not found")); + assertFalse("Some XPath rules use deprecated attributes", output.getLog().contains("Use of deprecated attribute")); String report = FileUtils.readFileToString(f); assertEquals("No processing errors expected", 0, StringUtils.countMatches(report, "Error while processing")); diff --git a/pom.xml b/pom.xml index 456a9a74e8..48fc8fd04d 100644 --- a/pom.xml +++ b/pom.xml @@ -800,6 +800,12 @@ Additionally it includes CPD, the copy-paste-detector. CPD finds duplicated code junit 4.12 + + org.hamcrest + hamcrest-library + 1.3 + test + pl.pragmatists JUnitParams