[core] saxon rulechain: don't use rule chain for other path expressions

This commit is contained in:
Andreas Dangel
2020-03-27 12:58:38 +01:00
parent 17ddfd2a22
commit a0e1e40bcb
6 changed files with 65 additions and 7 deletions

View File

@ -77,7 +77,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
/**
* Representation of an XPath query, created at {@link #initializeXPathExpression()} using {@link #xpath}.
*/
private XPathExpression xpathExpression;
XPathExpression xpathExpression;
/**
* Holds the static context later used to match the variables in the dynamic context in

View File

@ -13,6 +13,12 @@ import net.sf.saxon.om.Axis;
/**
* Simple printer for saxon expressions. Might be useful for debugging / during development.
*
* <p>Example:
* <pre>
* ExpressionPrinter printer = new ExpressionPrinter();
* printer.visit(query.xpathExpression.getInternalExpression());
* </pre>
*/
public class ExpressionPrinter extends Visitor {
private int depth = 0;

View File

@ -13,6 +13,7 @@ import net.sf.saxon.Configuration;
import net.sf.saxon.expr.AxisExpression;
import net.sf.saxon.expr.Expression;
import net.sf.saxon.expr.FilterExpression;
import net.sf.saxon.expr.LazyExpression;
import net.sf.saxon.expr.PathExpression;
import net.sf.saxon.expr.RootExpression;
import net.sf.saxon.om.Axis;
@ -96,6 +97,15 @@ public class RuleChainAnalyzer extends Visitor {
return super.visit(e);
}
@Override
public Expression visit(LazyExpression e) {
if (e.getBaseExpression() instanceof PathExpression) {
this.rootElement = null;
return e;
}
return super.visit(e);
}
public static Comparator<Node> documentOrderComparator() {
return net.sourceforge.pmd.lang.rule.xpath.internal.DocumentSorter.INSTANCE;
}

View File

@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang.rule.xpath.internal;
import net.sf.saxon.expr.AxisExpression;
import net.sf.saxon.expr.Expression;
import net.sf.saxon.expr.FilterExpression;
import net.sf.saxon.expr.LazyExpression;
import net.sf.saxon.expr.LetExpression;
import net.sf.saxon.expr.PathExpression;
import net.sf.saxon.expr.QuantifiedExpression;
@ -63,6 +64,11 @@ abstract class Visitor {
return result;
}
public Expression visit(LazyExpression e) {
Expression base = visit(e.getBaseExpression());
return LazyExpression.makeLazyExpression(base);
}
public Expression visit(Expression expr) {
Expression result;
if (expr instanceof DocumentSorter) {
@ -81,6 +87,8 @@ abstract class Visitor {
result = visit((QuantifiedExpression) expr);
} else if (expr instanceof LetExpression) {
result = visit((LetExpression) expr);
} else if (expr instanceof LazyExpression) {
result = visit((LazyExpression) expr);
} else {
result = expr;
}

View File

@ -19,11 +19,13 @@ import net.sourceforge.pmd.RuleViolation;
import net.sourceforge.pmd.lang.ast.DummyNode;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.ast.ParseException;
import net.sourceforge.pmd.lang.ast.xpath.AbstractASTXPathHandler;
import net.sourceforge.pmd.lang.ast.xpath.DocumentNavigator;
import net.sourceforge.pmd.lang.rule.AbstractRuleChainVisitor;
import net.sourceforge.pmd.lang.rule.AbstractRuleViolationFactory;
import net.sourceforge.pmd.lang.rule.ParametricRuleViolation;
import net.sf.saxon.expr.XPathContext;
import net.sf.saxon.sxpath.IndependentContext;
/**
@ -67,11 +69,18 @@ public class DummyLanguageModule extends BaseLanguageModule {
}
public static class Handler extends AbstractLanguageVersionHandler {
public static class TestFunctions {
public static boolean typeIs(final XPathContext context, final String fullTypeName) {
return false;
}
}
@Override
public XPathHandler getXPathHandler() {
return new XPathHandler() {
return new AbstractASTXPathHandler() {
@Override
public void initialize(IndependentContext context) {
super.initialize(context, LanguageRegistry.getLanguage(DummyLanguageModule.NAME), TestFunctions.class);
}
@Override

View File

@ -18,7 +18,6 @@ import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyFactory;
import net.sf.saxon.expr.Expression;
import net.sf.saxon.expr.XPathContext;
public class SaxonXPathRuleQueryTest {
@ -64,10 +63,36 @@ public class SaxonXPathRuleQueryTest {
assertExpression("((((/)/descendant::element(dummyNode, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Test2, xs:anyAtomicType)), ($qq:qq1741979653 singleton eq true()))])[QuantifiedExpression(Atomizer(attribute::attribute(Test1, xs:anyAtomicType)), ($qq:qq1529060733 singleton eq false()))])", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
}
public static class TestFunctions {
public static boolean typeIs(final XPathContext context, final String fullTypeName) {
return false;
}
@Test
public void ruleChainVisitsCustomFunctions() {
SaxonXPathRuleQuery query = createQuery("//dummyNode[pmd-dummy:typeIs(@Image)]");
List<String> ruleChainVisits = query.getRuleChainVisits();
Assert.assertEquals(1, ruleChainVisits.size());
Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
Assert.assertEquals(2, query.nodeNameToXPaths.size());
System.out.println(query.nodeNameToXPaths);
assertExpression("(self::node()[pmd-dummy:typeIs(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(Image, xs:anyAtomicType))))))])", query.nodeNameToXPaths.get("dummyNode").get(0));
assertExpression("DocumentSorter((((/)/descendant-or-self::node())/(child::element(dummyNode, xs:anyType)[pmd-dummy:typeIs(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(Image, xs:anyAtomicType))))))])))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
}
/**
* If a query contains another unbounded path expression other than the first one, it must be
* excluded from rule chain execution. Saxon itself optimizes this quite good already.
*/
@Test
public void ruleChainVisitsUnboundedPathExpressions() {
SaxonXPathRuleQuery query = createQuery("//dummyNode[//ClassOrInterfaceType]");
List<String> ruleChainVisits = query.getRuleChainVisits();
Assert.assertEquals(0, ruleChainVisits.size());
Assert.assertEquals(1, query.nodeNameToXPaths.size());
assertExpression("LetExpression(LazyExpression(((/)/descendant::element(ClassOrInterfaceType, xs:anyType))), (((/)/descendant::element(dummyNode, xs:anyType))[$zz:zz771775563]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
// second sample, more complex
query = createQuery("//dummyNode[ancestor::ClassOrInterfaceDeclaration[//ClassOrInterfaceType]]");
ruleChainVisits = query.getRuleChainVisits();
Assert.assertEquals(0, ruleChainVisits.size());
Assert.assertEquals(1, query.nodeNameToXPaths.size());
assertExpression("LetExpression(LazyExpression(((/)/descendant::element(ClassOrInterfaceType, xs:anyType))), (((/)/descendant::element(dummyNode, xs:anyType))[(ancestor::element(ClassOrInterfaceDeclaration, xs:anyType)[$zz:zz106374177])]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
}
@Test