Merge pull request #146 from shepmaster/warnings

Address compiler / rustdoc / clippy warnings

Author: shepmaster

src/axis.rs

@@ -41,6 +41,7 @@ pub enum Axis {
     FollowingSibling,
     Preceding,
     Following,
+    #[allow(clippy::enum_variant_names)]
     SelfAxis,
 }
 

src/context.rs

@@ -47,26 +47,24 @@ type Namespaces = HashMap<String, String>;
 ///     }
 /// }
 ///
-/// fn main() {
-///     let package = parser::parse("<thing xmlns:ns0='net:brain' ns0:bonus='1' />")
-///         .expect("failed to parse XML");
-///     let document = package.as_document();
-///     let node = document.root().children()[0];
+/// let package = parser::parse("<thing xmlns:ns0='net:brain' ns0:bonus='1' />")
+///     .expect("failed to parse XML");
+/// let document = package.as_document();
+/// let node = document.root().children()[0];
 ///
-///     let mut context = Context::new();
-///     context.set_function("sigmoid", Sigmoid);
-///     context.set_variable("t", 2.0);
-///     context.set_namespace("neural", "net:brain");
+/// let mut context = Context::new();
+/// context.set_function("sigmoid", Sigmoid);
+/// context.set_variable("t", 2.0);
+/// context.set_namespace("neural", "net:brain");
 ///
-///     let xpath = "sigmoid(@neural:bonus + $t)";
+/// let xpath = "sigmoid(@neural:bonus + $t)";
 ///
-///     let factory = Factory::new();
-///     let xpath = factory.build(xpath).expect("Could not compile XPath");
+/// let factory = Factory::new();
+/// let xpath = factory.build(xpath).expect("Could not compile XPath");
 ///
-///     let value = xpath.evaluate(&context, node).expect("XPath evaluation failed");
+/// let value = xpath.evaluate(&context, node).expect("XPath evaluation failed");
 ///
-///     assert_eq!(0.952, (value.number() * 1000.0).trunc() / 1000.0);
-/// }
+/// assert_eq!(0.952, (value.number() * 1000.0).trunc() / 1000.0);
 /// ```
 ///
 /// Note that we are using a custom function (`sigmoid`), a variable

src/expression.rs

@@ -64,8 +64,9 @@ pub type SubExpression = Box<dyn Expression + 'static>;
 macro_rules! binary_constructor(
     ($t:ident) => (
         impl $t {
+            #[allow(clippy::new_ret_no_self)]
             pub fn new(left: SubExpression, right: SubExpression) -> SubExpression {
-                Box::new($t{left: left, right: right})
+                Box::new($t{ left, right })
             }
         }
     );
@@ -126,15 +127,15 @@ impl Equal {
         }
 
         let v = match (&left_val, &right_val) {
-            (&Value::Nodeset(ref left_nodes), &Value::Nodeset(ref right_nodes)) => {
+            (Value::Nodeset(left_nodes), Value::Nodeset(right_nodes)) => {
                 let left_strings = str_vals(left_nodes);
                 let right_strings = str_vals(right_nodes);
                 !left_strings.is_disjoint(&right_strings)
             }
             (&Value::Nodeset(ref nodes), &Number(val))
             | (&Number(val), &Value::Nodeset(ref nodes)) => {
                 let numbers = num_vals(nodes);
-                numbers.iter().any(|n| *n == val)
+                numbers.contains(&val)
             }
             (&Value::Nodeset(ref nodes), &Value::String(ref val))
             | (&Value::String(ref val), &Value::Nodeset(ref nodes)) => {
@@ -162,6 +163,7 @@ pub struct NotEqual {
 }
 
 impl NotEqual {
+    #[allow(clippy::new_ret_no_self)]
     pub fn new(left: SubExpression, right: SubExpression) -> SubExpression {
         Box::new(NotEqual {
             equal: Equal { left, right },
@@ -334,6 +336,7 @@ pub struct Path {
 }
 
 impl Path {
+    #[allow(clippy::new_ret_no_self)]
     pub fn new(start_point: SubExpression, steps: Vec<Step>) -> SubExpression {
         Box::new(Path { start_point, steps })
     }
@@ -359,6 +362,7 @@ pub struct Filter {
 }
 
 impl Filter {
+    #[allow(clippy::new_ret_no_self)]
     pub fn new(node_selector: SubExpression, predicate: SubExpression) -> SubExpression {
         let predicate = Predicate {
             expression: predicate,
@@ -728,7 +732,7 @@ mod test {
             name: "left".into(),
         });
         let right = Box::new(Literal {
-            value: Value::Number(6.28),
+            value: Value::Number(6.3),
         });
 
         let expr = Equal { left, right };

src/function.rs

@@ -353,13 +353,13 @@ impl Function for TwoStringPredicate {
 fn starts_with() -> TwoStringPredicate {
     fn imp(a: &str, b: &str) -> bool {
         str::starts_with(a, b)
-    };
+    }
     TwoStringPredicate(imp)
 }
 fn contains() -> TwoStringPredicate {
     fn imp(a: &str, b: &str) -> bool {
         str::contains(a, b)
-    };
+    }
     TwoStringPredicate(imp)
 }
 
@@ -501,7 +501,7 @@ impl Function for Translate {
 
         let s = s
             .chars()
-            .filter_map(|c| replacements.get(&c).cloned().unwrap_or_else(|| Some(c)))
+            .filter_map(|c| replacements.get(&c).cloned().unwrap_or(Some(c)))
             .collect();
 
         Ok(Value::String(s))
@@ -1031,7 +1031,7 @@ mod test {
     fn assert_number(expected: f64, actual: Result<Value<'_>, Error>) {
         match actual {
             Ok(Value::Number(n)) => assert_eq!(PedanticNumber(n), PedanticNumber(expected)),
-            _ => assert!(false, "{:?} did not evaluate correctly", actual),
+            _ => panic!("{:?} did not evaluate correctly", actual),
         }
     }
 

src/lib.rs

@@ -19,14 +19,12 @@
 //! use sxd_document::parser;
 //! use sxd_xpath::{evaluate_xpath, Value};
 //!
-//! fn main() {
-//!     let package = parser::parse("<root>hello</root>").expect("failed to parse XML");
-//!     let document = package.as_document();
+//! let package = parser::parse("<root>hello</root>").expect("failed to parse XML");
+//! let document = package.as_document();
 //!
-//!     let value = evaluate_xpath(&document, "/root").expect("XPath evaluation failed");
+//! let value = evaluate_xpath(&document, "/root").expect("XPath evaluation failed");
 //!
-//!     assert_eq!("hello", value.string());
-//! }
+//! assert_eq!("hello", value.string());
 //! ```
 //!
 //! Evaluating an XPath returns a [`Value`][], representing the
@@ -45,21 +43,19 @@
 //! use sxd_document::parser;
 //! use sxd_xpath::{Factory, Context, Value};
 //!
-//! fn main() {
-//!     let package = parser::parse("<root>hello</root>")
-//!         .expect("failed to parse XML");
-//!     let document = package.as_document();
+//! let package = parser::parse("<root>hello</root>")
+//!     .expect("failed to parse XML");
+//! let document = package.as_document();
 //!
-//!     let factory = Factory::new();
-//!     let xpath = factory.build("/root").expect("Could not compile XPath");
+//! let factory = Factory::new();
+//! let xpath = factory.build("/root").expect("Could not compile XPath");
 //!
-//!     let context = Context::new();
+//! let context = Context::new();
 //!
-//!     let value = xpath.evaluate(&context, document.root())
-//!         .expect("XPath evaluation failed");
+//! let value = xpath.evaluate(&context, document.root())
+//!     .expect("XPath evaluation failed");
 //!
-//!     assert_eq!("hello", value.string());
-//! }
+//! assert_eq!("hello", value.string());
 //! ```
 //!
 //! See [`Context`][] for details on how to customize the
@@ -83,22 +79,29 @@
 //! defined prefixes, some XPath behavior may be confusing:
 //!
 //! 1. The `name` method will not include a prefix, even if the
-//! element or attribute has a namespace.
+//!    element or attribute has a namespace.
 //! 2. The `namespace` axis will not include namespaces without
-//! prefixes.
+//!    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.
+//!    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
 
+// Ignoring these as our MSRV predates the suggestions
+#![allow(
+    clippy::legacy_numeric_constants,
+    clippy::match_like_matches_macro,
+    clippy::option_as_ref_deref
+)]
+
 use snafu::{ResultExt, Snafu};
 use std::borrow::ToOwned;
 use std::string;
@@ -350,7 +353,7 @@ impl XPath {
     ///
     /// The most common case is to pass in a reference to a [`Context`][]:
     ///
-    /// ```rust,no-run
+    /// ```rust,no_run
     /// use sxd_document::dom::Document;
     /// use sxd_xpath::{XPath, Context};
     ///
@@ -442,12 +445,10 @@ pub enum Error {
 /// use sxd_document::parser;
 /// use sxd_xpath::{evaluate_xpath, Value};
 ///
-/// fn main() {
-///     let package = parser::parse("<root><a>1</a><b>2</b></root>").expect("failed to parse the XML");
-///     let document = package.as_document();
+/// let package = parser::parse("<root><a>1</a><b>2</b></root>").expect("failed to parse the XML");
+/// let document = package.as_document();
 ///
-///     assert_eq!(Ok(Value::Number(3.0)), evaluate_xpath(&document, "/*/a + /*/b"));
-/// }
+/// assert_eq!(Ok(Value::Number(3.0)), evaluate_xpath(&document, "/*/a + /*/b"));
 /// ```
 pub fn evaluate_xpath<'d>(document: &'d Document<'d>, xpath: &str) -> Result<Value<'d>, Error> {
     let factory = Factory::new();

src/node_test.rs

@@ -181,7 +181,7 @@ mod test {
     }
 
     impl<'d> Setup<'d> {
-        fn new(package: &'d Package) -> Setup<'_> {
+        fn new(package: &'d Package) -> Setup<'d> {
             Setup {
                 doc: package.as_document(),
                 context: Context::without_core_functions(),

src/nodeset.rs

@@ -110,7 +110,7 @@ impl<'d> Node<'d> {
             } else {
                 name.local_part().to_owned()
             }
-        };
+        }
 
         match *self {
             Root(_) => None,
@@ -234,7 +234,7 @@ impl<'d> Node<'d> {
                     _ => {}
                 }
             }
-        };
+        }
 
         fn text_descendants_string_value(node: Node<'_>) -> String {
             let mut result = String::new();
@@ -279,21 +279,21 @@ conversion_trait!(Node, {
     dom::ProcessingInstruction => Node::ProcessingInstruction
 });
 
-impl<'d> Into<Node<'d>> for dom::ChildOfRoot<'d> {
-    fn into(self) -> Node<'d> {
+impl<'d> From<dom::ChildOfRoot<'d>> for Node<'d> {
+    fn from(other: dom::ChildOfRoot<'d>) -> Node<'d> {
         use self::Node::*;
-        match self {
+        match other {
             dom::ChildOfRoot::Element(n) => Element(n),
             dom::ChildOfRoot::Comment(n) => Comment(n),
             dom::ChildOfRoot::ProcessingInstruction(n) => ProcessingInstruction(n),
         }
     }
 }
 
-impl<'d> Into<Node<'d>> for dom::ChildOfElement<'d> {
-    fn into(self) -> Node<'d> {
+impl<'d> From<dom::ChildOfElement<'d>> for Node<'d> {
+    fn from(other: dom::ChildOfElement<'d>) -> Node<'d> {
         use self::Node::*;
-        match self {
+        match other {
             dom::ChildOfElement::Element(n) => Element(n),
             dom::ChildOfElement::Text(n) => Text(n),
             dom::ChildOfElement::Comment(n) => Comment(n),
@@ -302,10 +302,10 @@ impl<'d> Into<Node<'d>> for dom::ChildOfElement<'d> {
     }
 }
 
-impl<'d> Into<Node<'d>> for dom::ParentOfChild<'d> {
-    fn into(self) -> Node<'d> {
+impl<'d> From<dom::ParentOfChild<'d>> for Node<'d> {
+    fn from(other: dom::ParentOfChild<'d>) -> Self {
         use self::Node::*;
-        match self {
+        match other {
             dom::ParentOfChild::Root(n) => Root(n),
             dom::ParentOfChild::Element(n) => Element(n),
         }
@@ -351,10 +351,7 @@ impl<'d> Nodeset<'d> {
     ///
     /// [document order]: https://www.w3.org/TR/xpath/#dt-document-order
     pub fn document_order_first(&self) -> Option<Node<'d>> {
-        let node = match self.nodes.iter().next() {
-            Some(n) => n,
-            None => return None,
-        };
+        let node = self.nodes.iter().next()?;
 
         if self.nodes.len() == 1 {
             return Some(*node);
@@ -403,6 +400,7 @@ impl<'d> DocOrder<'d> {
     fn new(doc: dom::Document<'d>) -> Self {
         let mut idx = 0;
         let mut stack: Vec<Node<'_>> = vec![doc.root().into()];
+        #[allow(clippy::mutable_key_type)]
         let mut order = HashMap::new();
 
         while let Some(n) = stack.pop() {