phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'xpath-doc-nodes'

Browse Source
This commit is contained in:
Clément Fournier
2020-10-28 22:49:02 +01:00
parent 9f701ee460 3e23e6645e
commit ec89913368
7 changed files with 99 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
docs/pages/pmd/userdocs/extending/writing_xpath_rules.md
View File

@ -127,6 +127,18 @@ represented by our 1.0 implementation as strings, meaning that `@BeginLine > "1"
worked ---that's not the case in 2.0 mode.
* <code>@ArgumentCount > <b style="color:red">'</b>1<b style="color:red">'</b></code> &rarr; `@ArgumentCount > 1`
* In XPath 1.0, the expression `/Foo` matches the *children* of the root named `Foo`.
In XPath 2.0, that expression matches the root, if it is named `Foo`. Consider the following tree:
```java
Foo
└─ Foo
└─ Foo
```
Then `/Foo` will match the root in XPath 2, and the other nodes (but not the root) in XPath 1.
See eg [an issue caused by this](https://github.com/pmd/pmd/issues/1919#issuecomment-512865434) in Apex,
with nested classes.
## Rule properties
**See [Defining rule properties](pmd_userdocs_extending_defining_properties.html#for-xpath-rules)**

5
docs/pages/release_notes.md
View File

@ -16,9 +16,12 @@ This is a {{ site.pmd.release_type }} release.
### Fixed Issues
* pmd-core
* [#1939](https://github.com/pmd/pmd/issues/1939): \[core] XPath expressions return handling
### API Changes
### External Contributions
{% endtocmaker %}

15
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/AstNodeOwner.java Normal file
View File

@ -0,0 +1,15 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.ast.xpath.internal;
import net.sourceforge.pmd.lang.ast.Node;
/**
* Marker interface.
*/
public interface AstNodeOwner {
Node getUnderlyingNode();
}

12
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/DocumentNode.java
View File

@ -10,6 +10,7 @@ import java.util.Map;
import net.sourceforge.pmd.annotation.InternalApi;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.ast.xpath.internal.AstNodeOwner;
import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttrLogger;
import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery;
@ -27,7 +28,7 @@ import net.sf.saxon.type.Type;
*/
@Deprecated
@InternalApi
public class DocumentNode extends BaseNodeInfo implements DocumentInfo {
public class DocumentNode extends BaseNodeInfo implements DocumentInfo, AstNodeOwner {
/**
* The root ElementNode of the DocumentNode.
@ -94,11 +95,20 @@ public class DocumentNode extends BaseNodeInfo implements DocumentInfo {
return new Navigator.DescendantEnumeration(this, true, true);
case Axis.CHILD:
return SingleNodeIterator.makeIterator(rootNode);
case Axis.SELF:
return SingleNodeIterator.makeIterator(this);
default:
return super.iterateAxis(axisNumber);
}
}
@Override
public Node getUnderlyingNode() {
// this is a concession to the model, so that the expression "/"
// may be interpreted as the root node
return rootNode.getUnderlyingNode();
}
public DeprecatedAttrLogger getAttrCtx() {
return attrCtx == null ? DeprecatedAttrLogger.noop() : attrCtx;
}

5
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java
View File

@ -11,6 +11,7 @@ import java.util.Map;
import net.sourceforge.pmd.annotation.InternalApi;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.ast.xpath.Attribute;
import net.sourceforge.pmd.lang.ast.xpath.internal.AstNodeOwner;
import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery;
import net.sf.saxon.om.Axis;
@ -38,7 +39,7 @@ import net.sf.saxon.value.Value;
*/
@Deprecated
@InternalApi
public class ElementNode extends BaseNodeInfo {
public class ElementNode extends BaseNodeInfo implements AstNodeOwner {
protected final DocumentNode document;
protected final ElementNode parent;
@ -94,7 +95,7 @@ public class ElementNode extends BaseNodeInfo {
}
@Override
public Object getUnderlyingNode() {
public Node getUnderlyingNode() {
return node;
}

17
pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java
View File

@ -17,6 +17,7 @@ import java.util.regex.Pattern;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.annotation.InternalApi;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.ast.xpath.internal.AstNodeOwner;
import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttrLogger;
import net.sourceforge.pmd.lang.ast.xpath.saxon.DocumentNode;
import net.sourceforge.pmd.lang.ast.xpath.saxon.ElementNode;
@ -120,25 +121,21 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
assert rootElementNode != null : "Cannot find " + node;
final XPathDynamicContext xpathDynamicContext = createDynamicContext(rootElementNode);
final List<ElementNode> nodes = new LinkedList<>();
final List<Node> results = new LinkedList<>();
List<Expression> expressions = getXPathExpressionForNodeOrDefault(node.getXPathNodeName());
for (Expression expression : expressions) {
SequenceIterator iterator = expression.iterate(xpathDynamicContext.getXPathContextObject());
Item current = iterator.next();
while (current != null) {
nodes.add((ElementNode) current);
if (current instanceof AstNodeOwner) {
results.add(((AstNodeOwner) current).getUnderlyingNode());
} else {
throw new RuntimeException("XPath rule expression returned a non-node (" + current.getClass() + "): " + current);
}
current = iterator.next();
}
}
/*
Map List of Saxon Nodes -> List of AST Nodes, which were detected to match the XPath expression
(i.e. violation found)
*/
final List<Node> results = new ArrayList<>(nodes.size());
for (final ElementNode elementNode : nodes) {
results.add((Node) elementNode.getUnderlyingNode());
}
Collections.sort(results, RuleChainAnalyzer.documentOrderComparator());
return results;
} catch (final XPathException e) {

48
pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQueryTest.java
View File

@ -9,10 +9,14 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.hamcrest.CoreMatchers;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.ast.DummyNode;
import net.sourceforge.pmd.lang.ast.DummyNodeWithListAndEnum;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.properties.PropertyDescriptor;
@ -22,6 +26,9 @@ import net.sf.saxon.expr.Expression;
public class SaxonXPathRuleQueryTest {
@Rule
public final ExpectedException expected = ExpectedException.none();
@Test
public void testListAttribute() {
DummyNodeWithListAndEnum dummy = new DummyNodeWithListAndEnum(1);
@ -37,6 +44,44 @@ public class SaxonXPathRuleQueryTest {
assertQuery(0, "//dummyNode[@EmptyList = (\"A\")]", dummy);
}
@Test
public void testInvalidReturn() {
DummyNodeWithListAndEnum dummy = new DummyNodeWithListAndEnum(1);
expected.expect(RuntimeException.class);
expected.expectMessage(CoreMatchers.containsString("XPath rule expression returned a non-node"));
expected.expectMessage(CoreMatchers.containsString("Int64Value"));
createQuery("1+2").evaluate(dummy, new RuleContext());
}
@Test
public void testRootExpression() {
DummyNode dummy = new DummyNode(); // todo in pmd 7 this should be a RootNode
List<Node> result = assertQuery(1, "/", dummy);
Assert.assertEquals(dummy, result.get(0));
}
@Test
public void testRootExpressionIsADocumentNode() {
DummyNode dummy = new DummyNode(); // todo in pmd 7 this should be a RootNode
List<Node> result = assertQuery(1, "(/)[self::document-node()]", dummy);
Assert.assertEquals(dummy, result.get(0));
}
@Test
public void testRootExpressionWithName() {
DummyNode dummy = new DummyNode(0, false, "DummyNodeA"); // todo in pmd 7 this should be a RootNode
List<Node> result = assertQuery(1, "(/)[self::document-node(element(DummyNodeA))]", dummy);
Assert.assertEquals(dummy, result.get(0));
assertQuery(0, "(/)[self::document-node(element(DummyNodeX))]", dummy);
}
@Test
public void ruleChainVisits() {
SaxonXPathRuleQuery query = createQuery("//dummyNode[@Image='baz']/foo | //bar[@Public = 'true'] | //dummyNode[@Public = false()] | //dummyNode");
@ -175,10 +220,11 @@ public class SaxonXPathRuleQueryTest {
assertExpression("((DocumentSorter(((((/)/descendant::element(dummyNode, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Image, xs:anyAtomicType)), ($qq:qq692331943 singleton eq \"baz\"))])/child::element(foo, xs:anyType))) | (((/)/descendant::element(bar, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Public, xs:anyAtomicType)), ($qq:qq2127036371 singleton eq \"true\"))])) | (((/)/descendant::element(dummyNode, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Public, xs:anyAtomicType)), ($qq:qq1529060733 singleton eq \"false\"))]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
}
private static void assertQuery(int resultSize, String xpath, Node node) {
private static List<Node> assertQuery(int resultSize, String xpath, Node node) {
SaxonXPathRuleQuery query = createQuery(xpath);
List<Node> result = query.evaluate(node, new RuleContext());
Assert.assertEquals(resultSize, result.size());
return result;
}
private static SaxonXPathRuleQuery createQuery(String xpath, PropertyDescriptor<?> ...descriptors) {