[XPath] Simplify step optimization and fix a bug with attribute iteration.

16243 72.1% successes
    0  0.0% skipped
 4012 17.8% failures
 2259 10.0% errors

Author: renggli

lib/src/xpath/expressions/path.dart

@@ -13,7 +13,10 @@ import 'node.dart';
 import 'step.dart';
 
 class PathExpression implements XPathExpression {
-  factory PathExpression(List<Step> steps, {required bool isAbsolute}) {
+  factory PathExpression(
+    List<XPathExpression> steps, {
+    required bool isAbsolute,
+  }) {
     if (steps.isEmpty) {
       assert(isAbsolute);
       return const PathExpression._(
@@ -22,134 +25,41 @@ class PathExpression implements XPathExpression {
         isOrderPreserved: true,
       );
     }
-    final optimizedSteps = <Step>[steps.first];
-    for (final step in steps.skip(1)) {
-      Step? merged;
-      final last = optimizedSteps.last;
-      if (last.axis is DescendantOrSelfAxis &&
-          last.nodeTest is NodeTypeTest &&
-          last.predicates.isEmpty &&
-          step.predicates.isEmpty) {
-        // Try to merge the '//' step with the next step:
-        // The next step must not have any predicates,
-        // because the predicates might depend on the `position` of the context node
-        // and we cannot guarantee the `position` is still preserved after merging.
-        switch (step.axis) {
-          // child::x => descendant::x
+    final optimizedSteps = <XPathExpression>[steps.first];
+    for (var i = 1; i < steps.length; i++) {
+      final previous = optimizedSteps.last;
+      final current = steps[i];
+      if (previous is StepExpression &&
+          previous.predicates.isEmpty &&
+          previous.axis is DescendantOrSelfAxis &&
+          previous.nodeTest is NodeTypeTest &&
+          current is StepExpression &&
+          current.predicates.isEmpty) {
+        switch (current.axis) {
           case ChildAxis():
-            merged = Step(
+            optimizedSteps.last = StepExpression(
               const DescendantAxis(),
-              nodeTest: step.nodeTest,
-              predicates: step.predicates,
+              nodeTest: current.nodeTest,
             );
-          // self::x => descendant-or-self::x
           case SelfAxis():
-            merged = Step(
+            optimizedSteps.last = StepExpression(
               const DescendantOrSelfAxis(),
-              nodeTest: step.nodeTest,
-              predicates: step.predicates,
+              nodeTest: current.nodeTest,
             );
-          // descendant::x => descendant::x
           case DescendantAxis():
-          // descendant-or-self::x => descendant-or-self::x
           case DescendantOrSelfAxis():
-            merged = step;
+            optimizedSteps.last = current;
           default:
+            optimizedSteps.add(current);
         }
-      }
-      if (merged != null) {
-        optimizedSteps.removeLast();
-        optimizedSteps.add(merged);
       } else {
-        optimizedSteps.add(step);
-      }
-    }
-
-    // The resulting nodes' document order and uniqueness is preserved,
-    // if the steps match the following patterns:
-    //
-    // 1. anyStep (selfStep | attributeStep)*
-    // 2. (selfStep | childStep)+ (descendantStep | descendantOrSelfStep)? (selfStep | attributeStep)*
-    //
-    // Proof:
-    //
-    // Suppose we have node-set S_prev which is the result of the previous step and it's in document order.
-    // And we are going to apply step f to it, where f is at least the second step obviously, then we get node-set S_next = f(S_prev).
-    //
-    // For any node pair i, j in S_next, where i comes before j in S_next, we need to prove that i comes before j in document order (1).
-    //
-    // Let's denote any nodes in S_prev that generate i and j as i' and j' respectively.
-    // If i' = j', then i comes before j in document order because step f is a forward step, so (1) is true.
-    // So we only need to discuss the case where **i' != j'**.
-    //
-    // To prove (1), we need to prove that:
-    // - i' comes not after j' in S_prev (2)
-    // - all nodes in f(i') come before all nodes in f(j') in document order (3).
-    //
-    // If i' comes after j' and we still get i before j in S_next, that means both f(i') and f(j') will produce i (4).
-    // To prove (2), we just need to prove that (4) is impossible.
-    //
-    // Let's discuss each pattern:
-    //
-    // **Pattern 1**
-    // Step f can only be selfStep or attributeStep.
-    // For such step f, f(i') and f(j') cannot produce the same node i, so (4) is impossible and (2) is true.
-    // Now that i' comes before j' in S_prev, we have:
-    // - If f is selfStep, then i = i' and j = j', so (3) is true.
-    // - If f is attributeStep, then all attributes of i' must come before all attributes of j' in document order, so (3) is true.
-    //
-    // **Pattern 2**
-    // We only need to discuss a simpler form of pattern2:
-    // - (selfStep | childStep)+ (descendantStep | descendantOrSelfStep)?
-    // Because if this simpler form will produce nodes in document order, then these steps can be viewed as a single complex step.
-    // Adding `(selfStep | attributeStep)*` after it is the same as pattern 1, which has been proved.
-    // So step f can be selfStep, childStep, descendantStep, or descendantOrSelfStep.
-    // For selfStep, the proof is the same as pattern 1.
-    // And after all the steps before f, the resulting nodes have the same depth, which means j' cannot be the descendant of i' and vice versa (5).
-    // For descendantOrSelfStep:
-    // - Because of (5), f(i') and f(j') cannot produce the same node i, so (4) is impossible and (2) is true.
-    // - Because of (2) and (5), all nodes in f(i') must come before all nodes in f(j') in document order, so (3) is true.
-    // Same proof applies to descendantStep and childStep.
-    //
-    // So for all the patterns, (2) and (3) are true, so (1) is true.
-    // By induction, the document order and uniqueness is preserved after applying all the steps.
-    bool isPattern1() {
-      for (var i = 1; i < optimizedSteps.length; i++) {
-        final step = optimizedSteps[i];
-        if (step.axis is! SelfAxis && step.axis is! AttributeAxis) {
-          return false;
-        }
-      }
-      return true;
-    }
-
-    bool isPattern2() {
-      var i = 0;
-      for (i = 0; i < optimizedSteps.length; i++) {
-        final axis = optimizedSteps[i].axis;
-        if (axis is! SelfAxis && axis is! AttributeAxis && axis is! ChildAxis) {
-          break;
-        }
+        optimizedSteps.add(current);
       }
-      if (i < optimizedSteps.length) {
-        final axis = optimizedSteps[i].axis;
-        if (axis is DescendantAxis || axis is DescendantOrSelfAxis) {
-          i++;
-        }
-      }
-      for (; i < optimizedSteps.length; i++) {
-        final axis = optimizedSteps[i].axis;
-        if (axis is! SelfAxis && axis is! AttributeAxis) {
-          return false;
-        }
-      }
-      return true;
     }
-
     return PathExpression._(
       optimizedSteps,
       isAbsolute: isAbsolute,
-      isOrderPreserved: isPattern1() || isPattern2(),
+      isOrderPreserved: _isOrderPreserved(optimizedSteps),
     );
   }
 
@@ -159,10 +69,8 @@ class PathExpression implements XPathExpression {
     required this.isOrderPreserved,
   });
 
-  final List<Step> steps;
+  final List<XPathExpression> steps;
   final bool isAbsolute;
-
-  /// Whether the document order of the resulting nodes is preserved after applying all the steps.
   final bool isOrderPreserved;
 
   @override
@@ -200,31 +108,68 @@ class PathExpression implements XPathExpression {
       return XPathSequence(_sortAndDeduplicate(nodes, contextNode.root));
     }
   }
+}
 
-  static List<Object> _sortAndDeduplicate(Iterable<Object> iter, XmlNode root) {
-    final nodes = <XmlNode>{};
-    final others = <Object>{};
-    for (final item in iter) {
-      if (item is XmlNode) {
-        nodes.add(item);
-      } else {
-        others.add(item);
-      }
+bool _isOrderPreserved(List<XPathExpression> expressions) {
+  if (expressions.length <= 1) {
+    return true;
+  }
+  if (expressions.any((expression) => expression is! StepExpression)) {
+    return false;
+  }
+  final steps = expressions.cast<StepExpression>().toList();
+  if (steps
+      .skip(1)
+      .every((s) => s.axis is SelfAxis || s.axis is AttributeAxis)) {
+    return true;
+  }
+  var i = 0;
+  while (i < steps.length) {
+    final axis = steps[i].axis;
+    if (axis is SelfAxis || axis is AttributeAxis || axis is ChildAxis) {
+      i++;
+    } else {
+      break;
     }
-    final result = <Object>[];
-    if (nodes.length <= 50) {
-      result.addAll(nodes.sorted((a, b) => a.compareNodePosition(b)));
+  }
+  if (i < steps.length) {
+    final axis = steps[i].axis;
+    if (axis is DescendantAxis || axis is DescendantOrSelfAxis) i++;
+  }
+  while (i < steps.length) {
+    final axis = steps[i].axis;
+    if (axis is SelfAxis || axis is AttributeAxis) {
+      i++;
     } else {
-      if (nodes.remove(root)) result.add(root);
-      for (final node in root.descendants) {
-        if (nodes.isEmpty) break;
-        if (nodes.remove(node)) result.add(node);
-      }
-      if (nodes.isNotEmpty) {
-        result.addAll(nodes.sorted((a, b) => a.compareNodePosition(b)));
-      }
+      break;
+    }
+  }
+  return i == steps.length;
+}
+
+List<Object> _sortAndDeduplicate(Iterable<Object> iter, XmlNode root) {
+  final nodes = <XmlNode>{};
+  final others = <Object>{};
+  for (final item in iter) {
+    if (item is XmlNode) {
+      nodes.add(item);
+    } else {
+      others.add(item);
+    }
+  }
+  final result = <Object>[];
+  if (nodes.length <= 50) {
+    result.addAll(nodes.sorted((a, b) => a.compareNodePosition(b)));
+  } else {
+    if (nodes.remove(root)) result.add(root);
+    for (final node in root.descendants) {
+      if (nodes.isEmpty) break;
+      if (nodes.remove(node)) result.add(node);
+    }
+    if (nodes.isNotEmpty) {
+      result.addAll(nodes.sorted((a, b) => a.compareNodePosition(b)));
     }
-    result.addAll(others);
-    return result;
   }
+  result.addAll(others);
+  return result;
 }

lib/src/xpath/expressions/step.dart

@@ -3,13 +3,17 @@ import 'package:meta/meta.dart';
 import '../evaluation/context.dart';
 import '../evaluation/expression.dart' show XPathExpression;
 import '../types/node.dart';
+import '../types/sequence.dart';
 import 'axis.dart';
 import 'node.dart';
 import 'predicate.dart';
 
+/// A step in a path expression.
+///
+/// Returns the resulting nodes in document order.
 @immutable
-class Step {
-  const Step(
+class StepExpression implements XPathExpression {
+  const StepExpression(
     this.axis, {
     this.nodeTest = const NodeTypeTest(),
     this.predicates = const [],
@@ -19,8 +23,8 @@ class Step {
   final NodeTest nodeTest;
   final List<Predicate> predicates;
 
-  /// Apply this step to the given context, returning the resulting nodes in document order.
-  Iterable<Object> call(XPathContext context) {
+  @override
+  XPathSequence call(XPathContext context) {
     var result = <Object>[];
     for (final node in axis.find(xsNode.cast(context.item))) {
       if (nodeTest.matches(node)) {
@@ -43,24 +47,6 @@ class Step {
         result = matched;
       }
     }
-    return result;
+    return XPathSequence(result);
   }
 }
-
-class ExpressionStep implements Step {
-  const ExpressionStep(this.expression);
-
-  final XPathExpression expression;
-
-  @override
-  Axis get axis => const SelfAxis();
-
-  @override
-  NodeTest get nodeTest => const NodeTypeTest();
-
-  @override
-  List<Predicate> get predicates => const [];
-
-  @override
-  Iterable<Object> call(XPathContext context) => expression(context);
-}