From 7754224f9d0c0c9dda5b39d8baa321a75b7829d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 26 Mar 2018 20:29:10 +0200 Subject: [PATCH] Fix deprecated XPath attribute not found Deprecated attributes are not eliminated from the attribute iterator anymore, which made deprecating attributes API breaking. Instead, deprecated attribute usage are now detected and logged as warnings --- .../pmd/lang/ast/xpath/Attribute.java | 23 +++++++- .../lang/ast/xpath/AttributeAxisIterator.java | 7 ++- .../pmd/lang/ast/AbstractNodeTest.java | 58 ++++++++++++++++++- .../ast/DummyNodeWithDeprecatedAttribute.java | 23 ++++++++ .../ast/xpath/AttributeAxisIteratorTest.java | 27 +++++++-- 5 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNodeWithDeprecatedAttribute.java 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; + } }