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..d54c2ccd42 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.HashSet; +import java.util.Set; +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 Set DETECTED_DEPRECATED_ATTRIBUTES = new HashSet<>(); + + 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.contains(getLoggableAttributeName())) { + DETECTED_DEPRECATED_ATTRIBUTES.add(getLoggableAttributeName()); + 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) { + } 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..18619ecd27 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,26 @@ 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 java.io.StringWriter; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; + +import org.jaxen.JaxenException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import net.sourceforge.pmd.lang.ast.xpath.Attribute; + import junitparams.JUnitParamsRunner; import junitparams.Parameters; + /** * Unit test for {@link AbstractNode}. */ @@ -65,9 +76,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 +228,48 @@ public class AbstractNodeTest { // Check that this node still does not have any children assertEquals(0, grandChild.jjtGetNumChildren()); } + + + @Test + public void testDeprecatedAttributeXPathQuery() throws JaxenException { + final StringWriter writer = new StringWriter(); + + class MyRootNode extends DummyNode implements RootNode { + + private MyRootNode(int id) { + super(id); + } + } + + // intercept log + Logger.getLogger(Attribute.class.getName()).setLevel(Level.WARNING); + Logger.getLogger(Attribute.class.getName()).addHandler(new Handler() { + @Override + public void publish(LogRecord record) { + writer.write(record.getMessage()); + writer.write('\n'); + } + + + @Override + public void flush() { + writer.flush(); + } + + + @Override + public void close() throws SecurityException { + // empty + } + }); + addChild(new MyRootNode(nextId()), new DummyNodeWithDeprecatedAttribute(2)).findChildNodesWithXPath("//dummyNode[@Size=1]"); + + writer.flush(); + + assertTrue(writer.toString().contains("deprecated")); + assertTrue(writer.toString().contains("attribute")); + assertTrue(writer.toString().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..68e278ad4a --- /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.2.0 + */ +public class DummyNodeWithDeprecatedAttribute extends DummyNode { + + + public DummyNodeWithDeprecatedAttribute(int id) { + super(id); + } + + + @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..e04d981fad 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,6 +4,8 @@ package net.sourceforge.pmd.lang.ast.xpath; +import static junit.framework.TestCase.assertTrue; + import java.util.HashMap; import java.util.Map; @@ -11,12 +13,21 @@ 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); + assertTrue(toMap(new AttributeAxisIterator(dummy)).containsKey("Size")); + } + /** * Test hasNext and next. */ @@ -27,11 +38,7 @@ public class AttributeAxisIteratorTest { dummyNode.testingOnlySetBeginColumn(1); AttributeAxisIterator it = new AttributeAxisIterator(dummyNode); - Map atts = new HashMap<>(); - while (it.hasNext()) { - Attribute attribute = it.next(); - atts.put(attribute.getName(), attribute); - } + Map atts = toMap(it); Assert.assertEquals(7, atts.size()); Assert.assertTrue(atts.containsKey("BeginColumn")); Assert.assertTrue(atts.containsKey("BeginLine")); @@ -41,4 +48,14 @@ public class AttributeAxisIteratorTest { Assert.assertTrue(atts.containsKey("EndColumn")); Assert.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); + } + return atts; + } }