Merge branch 'pr-1006'

This commit is contained in:
Juan Martín Sotuyo Dodero
2018-04-15 02:06:41 -03:00
8 changed files with 121 additions and 16 deletions

View File

@ -121,6 +121,11 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-library</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>pl.pragmatists</groupId>
<artifactId>JUnitParams</artifactId>

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.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<String, Boolean> 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;
}

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

View File

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

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.3.0
*/
public class DummyNodeWithDeprecatedAttribute extends DummyNode {
public DummyNodeWithDeprecatedAttribute(int id) {
super(id);
}
// this is the deprecated attribute
@Deprecated
public int getSize() {
return 2;
}
}

View File

@ -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<String, Attribute> 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<String, Attribute> toMap(AttributeAxisIterator it) {
Map<String, Attribute> 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;
}
}

View File

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

View File

@ -800,6 +800,12 @@ Additionally it includes CPD, the copy-paste-detector. CPD finds duplicated code
<artifactId>junit</artifactId>
<version>4.12</version>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-library</artifactId>
<version>1.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>pl.pragmatists</groupId>
<artifactId>JUnitParams</artifactId>