Merge pull request #95 from shepmaster/unique

Nodeset should keep a unique set of nodes

src/axis.rs

@@ -4,7 +4,7 @@ use std::fmt;
 
 use ::context;
 use ::node_test::NodeTest;
-use ::nodeset::{self, Nodeset, Node};
+use ::nodeset::{self, OrderedNodes, Node};
 
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub enum PrincipalNodeType {
@@ -20,7 +20,7 @@ pub trait AxisLike: fmt::Debug {
     fn select_nodes<'c, 'd>(&self,
                             context:   &context::Evaluation<'c, 'd>,
                             node_test: &NodeTest,
-                            result:    &mut Nodeset<'d>);
+                            result:    &mut OrderedNodes<'d>);
 
     /// Describes what node type is naturally selected by this axis.
     fn principal_node_type(&self) -> PrincipalNodeType {
@@ -49,7 +49,7 @@ impl AxisLike for Axis {
     fn select_nodes<'c, 'd>(&self,
                             context:   &context::Evaluation<'c, 'd>,
                             node_test: &NodeTest,
-                            result:    &mut Nodeset<'d>)
+                            result:    &mut OrderedNodes<'d>)
     {
         use self::Axis::*;
         match *self {
@@ -144,7 +144,7 @@ impl AxisLike for Axis {
 
 fn preceding_following_sibling<'c, 'd>(context:   &context::Evaluation<'c, 'd>,
                                        node_test: &NodeTest,
-                                       result:    &mut Nodeset<'d>,
+                                       result:    &mut OrderedNodes<'d>,
                                        f: fn(&Node<'d>) -> Vec<Node<'d>>)
 {
     let sibs = f(&context.node);
@@ -156,7 +156,7 @@ fn preceding_following_sibling<'c, 'd>(context:   &context::Evaluation<'c, 'd>,
 
 fn preceding_following<'c, 'd>(context:   &context::Evaluation<'c, 'd>,
                                node_test: &NodeTest,
-                               result:    &mut Nodeset<'d>,
+                               result:    &mut OrderedNodes<'d>,
                                f: fn(&Node<'d>) -> Vec<Node<'d>>)
 {
     let mut node = context.node;
@@ -182,26 +182,26 @@ mod test {
 
     use ::context::{self, Context};
     use ::node_test::NodeTest;
-    use ::nodeset::{Nodeset, Node};
+    use ::nodeset::{OrderedNodes, Node};
 
     use super::*;
     use super::Axis::*;
 
     #[derive(Debug)]
     struct DummyNodeTest;
     impl NodeTest for DummyNodeTest {
-        fn test<'c, 'd>(&self, context: &context::Evaluation<'c, 'd>, result: &mut Nodeset<'d>) {
+        fn test<'c, 'd>(&self, context: &context::Evaluation<'c, 'd>, result: &mut OrderedNodes<'d>) {
             result.add(context.node)
         }
     }
 
-    fn execute<'n, N>(axis: Axis, node: N) -> Nodeset<'n>
+    fn execute<'n, N>(axis: Axis, node: N) -> OrderedNodes<'n>
         where N: Into<Node<'n>>,
     {
         let context = Context::without_core_functions();
         let context = context::Evaluation::new(&context, node.into());
         let node_test = &DummyNodeTest;
-        let mut result = Nodeset::new();
+        let mut result = OrderedNodes::new();
 
         axis.select_nodes(&context, node_test, &mut result);
 
@@ -222,7 +222,7 @@ mod test {
 
         let result = execute(Ancestor, level2);
 
-        assert_eq!(result, nodeset![level1, level0]);
+        assert_eq!(result, ordered_nodes![level1, level0]);
     }
 
     #[test]
@@ -239,7 +239,7 @@ mod test {
 
         let result = execute(AncestorOrSelf, level2);
 
-        assert_eq!(result, nodeset![level2, level1, level0]);
+        assert_eq!(result, ordered_nodes![level2, level1, level0]);
     }
 
     #[test]
@@ -258,7 +258,7 @@ mod test {
 
         let result = execute(PrecedingSibling, child3);
 
-        assert_eq!(result, nodeset![child2, child1]);
+        assert_eq!(result, ordered_nodes![child2, child1]);
     }
 
     #[test]
@@ -277,7 +277,7 @@ mod test {
 
         let result = execute(FollowingSibling, child1);
 
-        assert_eq!(result, nodeset![child2, child3]);
+        assert_eq!(result, ordered_nodes![child2, child3]);
     }
 
     fn setup_preceding_following<'d>(doc: &'d dom::Document<'d>) ->
@@ -313,7 +313,7 @@ mod test {
 
         let result = execute(Preceding, b2);
 
-        assert_eq!(result, nodeset![b1, a1]);
+        assert_eq!(result, ordered_nodes![b1, a1]);
     }
 
     #[test]
@@ -324,6 +324,6 @@ mod test {
 
         let result = execute(Following, b2);
 
-        assert_eq!(result, nodeset![b3, a3]);
+        assert_eq!(result, ordered_nodes![b3, a3]);
     }
 }

src/context.rs

@@ -7,7 +7,7 @@ use std::collections::HashMap;
 use std::iter;
 
 use ::{Value, OwnedQName};
-use ::nodeset::{self, Node, Nodeset};
+use ::nodeset::{Node, OrderedNodes};
 use ::function;
 
 /// A mapping of names to XPath functions.
@@ -195,11 +195,11 @@ impl<'c, 'd> Evaluation<'c, 'd> {
     }
 
     /// Yields a new `Evaluation` context for each node in the nodeset.
-    pub fn new_contexts_for(self, nodes: Nodeset<'d>) -> EvaluationNodesetIter<'c, 'd> {
+    pub fn new_contexts_for(self, nodes: OrderedNodes<'d>) -> EvaluationNodesetIter<'c, 'd> {
         let sz = nodes.size();
         EvaluationNodesetIter {
             parent: self,
-            nodes: nodes.into_iter().enumerate(),
+            nodes: Vec::from(nodes).into_iter().enumerate(),
             size: sz,
         }
     }
@@ -208,7 +208,7 @@ impl<'c, 'd> Evaluation<'c, 'd> {
 /// An iterator for the contexts of each node in a nodeset
 pub struct EvaluationNodesetIter<'c, 'd: 'c> {
     parent: Evaluation<'c, 'd>,
-    nodes: iter::Enumerate<nodeset::IntoIter<'d>>,
+    nodes: iter::Enumerate<::std::vec::IntoIter<Node<'d>>>,
     size: usize,
 }
 

src/expression.rs

@@ -9,7 +9,7 @@ use ::axis::{Axis, AxisLike};
 use ::context;
 use ::function;
 use ::node_test::NodeTest;
-use ::nodeset::Nodeset;
+use ::nodeset::{Nodeset, OrderedNodes};
 
 quick_error! {
     #[derive(Debug, Clone, PartialEq, Hash)]
@@ -45,6 +45,19 @@ fn value_into_nodeset(v: Value) -> Result<Nodeset, Error> {
     }
 }
 
+// In these cases, we use document order. From the spec:
+//
+// > `(preceding::foo)[1]` returns the first `foo` element in document
+// > order, because the axis that applies to the `[1]` predicate is
+// > the child axis
+//
+fn value_into_ordered_nodes(v: Value) -> Result<OrderedNodes, Error> {
+    match v {
+        Value::Nodeset(ns) => Ok(ns.document_order().into()),
+        _ => Err(Error::NotANodeset),
+    }
+}
+
 pub trait Expression: fmt::Debug {
     fn evaluate<'c, 'd>(&self, context: &context::Evaluation<'c, 'd>) -> Result<Value<'d>, Error>;
 }
@@ -324,9 +337,9 @@ impl Filter {
 impl Expression for Filter {
     fn evaluate<'c, 'd>(&self, context: &context::Evaluation<'c, 'd>) -> Result<Value<'d>, Error> {
         self.node_selector.evaluate(context)
-            .and_then(value_into_nodeset)
+            .and_then(value_into_ordered_nodes)
             .and_then(|nodes| self.predicate.select(context, nodes))
-            .and_then(|nodes| Ok(Value::Nodeset(nodes)))
+            .map(|nodes| Value::Nodeset(nodes.into()))
     }
 }
 
@@ -394,12 +407,12 @@ struct Predicate {
 }
 
 impl Predicate {
-    fn select<'c, 'd>(&self, context: &context::Evaluation<'c, 'd>, nodes: Nodeset<'d>)
-                      -> Result<Nodeset<'d>, Error>
+    fn select<'c, 'd>(&self, context: &context::Evaluation<'c, 'd>, nodes: OrderedNodes<'d>)
+                      -> Result<OrderedNodes<'d>, Error>
     {
         context.new_contexts_for(nodes).filter_map(|ctx| {
             match self.matches(&ctx) {
-                Ok(true) => Some(Ok(ctx)),
+                Ok(true) => Some(Ok(ctx.node)),
                 Ok(false) => None,
                 Err(e) => Some(Err(e)),
             }
@@ -443,13 +456,16 @@ impl<A> ParameterizedStep<A>
         // axis and node-test. We evaluate the predicates on the total
         // set of new nodes.
 
+        // This seems like a likely place where we could differ from
+        // the spec, so thorough testing would be ideal.
+
         self.apply_predicates(context, self.apply_axis(context, starting_nodes))
     }
 
     fn apply_axis<'c, 'd>(&self, context: &context::Evaluation<'c, 'd>, starting_nodes: Nodeset<'d>)
-                        -> Nodeset<'d>
+                        -> OrderedNodes<'d>
     {
-        let mut result = Nodeset::new();
+        let mut result = OrderedNodes::new();
 
         for node in starting_nodes.iter() {
             let child_context = context.new_context_for(node);
@@ -461,7 +477,7 @@ impl<A> ParameterizedStep<A>
 
     fn apply_predicates<'c, 'd>(&self,
                                 context: &context::Evaluation<'c, 'd>,
-                                nodes: Nodeset<'d>)
+                                nodes: OrderedNodes<'d>)
                                 -> Result<Nodeset<'d>, Error>
     {
         let mut nodes = nodes;
@@ -470,7 +486,7 @@ impl<A> ParameterizedStep<A>
             nodes = try!(predicate.select(context, nodes));
         }
 
-        Ok(nodes)
+        Ok(nodes.into())
     }
 }
 
@@ -540,7 +556,7 @@ mod test {
     use ::context::{self, Context};
     use ::function;
     use ::node_test::NodeTest;
-    use ::nodeset::Nodeset;
+    use ::nodeset::OrderedNodes;
 
     use super::*;
 
@@ -797,6 +813,12 @@ mod test {
 
         setup.context.set_variable("nodes", input_nodeset);
 
+        // We need to give these elements some kind of document order
+        let parent = setup.doc.create_element("parent");
+        setup.doc.root().append_child(parent);
+        parent.append_child(input_node_1);
+        parent.append_child(input_node_2);
+
         let selected_nodes = Box::new(Variable { name: "nodes".into() });
         let predicate = Box::new(Literal{value: Value::Number(1.0)});
 
@@ -877,7 +899,7 @@ mod test {
         fn select_nodes(&self,
                         _context:   &context::Evaluation,
                         _node_test: &NodeTest,
-                        _result:    &mut Nodeset)
+                        _result:    &mut OrderedNodes)
         {
             *self.calls.borrow_mut() += 1;
         }
@@ -886,8 +908,7 @@ mod test {
     #[derive(Debug)]
     struct DummyNodeTest;
     impl NodeTest for DummyNodeTest {
-        fn test(&self, _context: &context::Evaluation, _result: &mut Nodeset) {
-        }
+        fn test(&self, _context: &context::Evaluation, _result: &mut OrderedNodes) {}
     }
 
     #[test]

src/lib.rs

@@ -77,12 +77,14 @@
 //! [`Factory`]: struct.Factory.html
 //! [`Context`]: context/struct.Context.html
 //!
-//! ### Namespaces
+//! ### Programmatically-created XML
 //!
-//! The XPath specification assumes that the XML being processed has a
-//! prefix defined for every namespace present in the document. If you
-//! are processing XML that was parsed from text, this will be true by
-//! construction.
+//! The XPath specification assumes certain properties about the XML
+//! being processed. If you are processing XML that was parsed from
+//! text, this will be true by construction. If you have
+//! programmatically created XML, please note the following cases.
+//!
+//! #### Namespaces
 //!
 //! If you have programmatically created XML with namespaces but not
 //! defined prefixes, some XPath behavior may be confusing:
@@ -92,6 +94,17 @@
 //! 2. The `namespace` axis will not include namespaces without
 //! prefixes.
 //!
+//! #### Document order
+//!
+//! If you have programmatically created XML but not attached the
+//! nodes to the document, some XPath behavior may be confusing:
+//!
+//! 1. These nodes have no [*document order*]. If you create a
+//! variable containing these nodes and apply a predicate to them,
+//! these nodes will appear after any nodes that are present in the
+//! document, but the relative order of the nodes is undefined.
+//!
+//! [*document order*]: https://www.w3.org/TR/xpath/#dt-document-order
 
 #[macro_use]
 extern crate peresil;

src/macros.rs

@@ -9,3 +9,16 @@ macro_rules! nodeset(
     });
     ($($e:expr),+,) => (nodeset!($($e),+))
 );
+
+
+/// Convenience constructor for an OrderedNodes
+macro_rules! ordered_nodes {
+    ( $($val:expr,)* ) => {
+        $crate::nodeset::OrderedNodes::from(vec![
+            $( $crate::nodeset::Node::from($val), )*
+        ])
+    };
+    ( $($val:expr),* ) => {
+        ordered_nodes![$($val, )*]
+    };
+}