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

Fix deprecated XPath attribute not found

Browse Source 
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
This commit is contained in:
Clément Fournier
2018-03-26 20:29:10 +02:00
parent 8625d769f1
commit 7754224f9d
5 changed files with 126 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java
View File

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

7
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java
View File

@ -101,6 +101,9 @@ public class AttributeAxisIterator implements Iterator<Attribute> {
return new Attribute(node, m.name, m.method);
}
private static final Set<Class<?>> CONSIDERED_RETURN_TYPES
= new HashSet<>(Arrays.<Class<?>>asList(Integer.TYPE, Boolean.TYPE, Double.TYPE, String.class, Long.TYPE, Character.TYPE, Float.TYPE));
@ -109,10 +112,8 @@ public class AttributeAxisIterator implements Iterator<Attribute> {
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);

58
pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java
View File

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

23
pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNodeWithDeprecatedAttribute.java Normal file
View File

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

27
pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java
View File

@ -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<String, Attribute> atts = new HashMap<>();
while (it.hasNext()) {
Attribute attribute = it.next();
atts.put(attribute.getName(), attribute);
}
Map<String, Attribute> 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<String, Attribute> toMap(AttributeAxisIterator it) {
Map<String, Attribute> atts = new HashMap<>();
while (it.hasNext()) {
Attribute attribute = it.next();
atts.put(attribute.getName(), attribute);
}
return atts;
}
}