[core] Fix XPath rulechain optimization bug
For XPath query "//dummyNode[ends-with(@Image, 'foo')][pmd-dummy:typeIs('bar')]" we lost the first condition (ends-with...) and only applied the second one (pmd-dummy:typeIs).
This commit is contained in:
@ -4,8 +4,10 @@
|
|||||||
|
|
||||||
package net.sourceforge.pmd.lang.rule.xpath.internal;
|
package net.sourceforge.pmd.lang.rule.xpath.internal;
|
||||||
|
|
||||||
|
import java.util.ArrayDeque;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
import java.util.Comparator;
|
import java.util.Comparator;
|
||||||
|
import java.util.Deque;
|
||||||
|
|
||||||
import net.sourceforge.pmd.lang.ast.Node;
|
import net.sourceforge.pmd.lang.ast.Node;
|
||||||
|
|
||||||
@ -69,7 +71,17 @@ public class RuleChainAnalyzer extends SaxonExprVisitor {
|
|||||||
Expression step = newPath.getStepExpression();
|
Expression step = newPath.getStepExpression();
|
||||||
if (step instanceof FilterExpression) {
|
if (step instanceof FilterExpression) {
|
||||||
FilterExpression filterExpression = (FilterExpression) newPath.getStepExpression();
|
FilterExpression filterExpression = (FilterExpression) newPath.getStepExpression();
|
||||||
result = new FilterExpression(new AxisExpression(Axis.SELF, null), filterExpression.getFilter());
|
|
||||||
|
Deque<Expression> filters = new ArrayDeque<>();
|
||||||
|
Expression walker = filterExpression;
|
||||||
|
while (walker instanceof FilterExpression) {
|
||||||
|
filters.push(((FilterExpression) walker).getFilter());
|
||||||
|
walker = ((FilterExpression) walker).getBaseExpression();
|
||||||
|
}
|
||||||
|
result = new FilterExpression(new AxisExpression(Axis.SELF, null), filters.pop());
|
||||||
|
while (!filters.isEmpty()) {
|
||||||
|
result = new FilterExpression(result, filters.pop());
|
||||||
|
}
|
||||||
rootElementReplaced = true;
|
rootElementReplaced = true;
|
||||||
} else if (step instanceof AxisExpression) {
|
} else if (step instanceof AxisExpression) {
|
||||||
if (newPath.getStartExpression() instanceof RootExpression) {
|
if (newPath.getStartExpression() instanceof RootExpression) {
|
||||||
|
@ -193,6 +193,16 @@ public class SaxonXPathRuleQueryTest {
|
|||||||
assertExpression("DocumentSorter((LetExpression(LazyExpression(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer($testClassPattern))))), (((/)/descendant::element(dummyNode, xs:anyType))[matches(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(SimpleName, xs:anyAtomicType))))), $zz:zz952562199)]))/child::element(foo, xs:anyType)))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
|
assertExpression("DocumentSorter((LetExpression(LazyExpression(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer($testClassPattern))))), (((/)/descendant::element(dummyNode, xs:anyType))[matches(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(SimpleName, xs:anyAtomicType))))), $zz:zz952562199)]))/child::element(foo, xs:anyType)))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void ruleChainVisitWithTwoFunctions() {
|
||||||
|
SaxonXPathRuleQuery query = createQuery("//dummyNode[ends-with(@Image, 'foo')][pmd-dummy:typeIs('bar')]");
|
||||||
|
List<String> ruleChainVisits = query.getRuleChainVisits();
|
||||||
|
Assert.assertEquals(1, ruleChainVisits.size());
|
||||||
|
Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
|
||||||
|
Assert.assertEquals(2, query.nodeNameToXPaths.size());
|
||||||
|
assertExpression("((self::node()[ends-with(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(Image, xs:anyAtomicType))))), \"foo\")])[pmd-dummy:typeIs(\"bar\")])", query.nodeNameToXPaths.get("dummyNode").get(0));
|
||||||
|
}
|
||||||
|
|
||||||
private static void assertExpression(String expected, Expression actual) {
|
private static void assertExpression(String expected, Expression actual) {
|
||||||
Assert.assertEquals(normalizeExprDump(expected),
|
Assert.assertEquals(normalizeExprDump(expected),
|
||||||
normalizeExprDump(actual.toString()));
|
normalizeExprDump(actual.toString()));
|
||||||
|
@ -150,4 +150,19 @@ public class Violation {
|
|||||||
}
|
}
|
||||||
]]></code>
|
]]></code>
|
||||||
</test-code>
|
</test-code>
|
||||||
|
|
||||||
|
<test-code>
|
||||||
|
<description>false positive when accessible object is used as primary prefix</description>
|
||||||
|
<expected-problems>0</expected-problems>
|
||||||
|
<code><![CDATA[
|
||||||
|
import java.lang.reflect.Constructor;
|
||||||
|
import java.util.List;
|
||||||
|
|
||||||
|
public class NoViolation { {
|
||||||
|
List<Constructor<?>> list = new ArrayList<>();
|
||||||
|
Constructor<?> ctor = NoViolation.class.getConstructor();
|
||||||
|
list.add(ctor);
|
||||||
|
} }
|
||||||
|
]]></code>
|
||||||
|
</test-code>
|
||||||
</test-data>
|
</test-data>
|
Reference in New Issue
Block a user