Merge #96: Cleanup the crate

4a4d29a Re-order code (Tobin C. Harding)
e8ed68e Clean up import statements (Tobin C. Harding)
9a7db48 Do trivial error cleanup (Tobin C. Harding)
1fe7659 Do trivial docs cleanups (Tobin C. Harding)
845d2c5 Move HashableValue to client module (Tobin C. Harding)
67c87ed Put attributes below rustdoc (Tobin C. Harding)
1b4e4bd Put the use statements together (Tobin C. Harding)
6ce09a5 Remove stale comment (Tobin C. Harding)

Pull request description:

  Do a bunch of cleanups, not all that important, this is just basic tidying up.

  The last patch may be seen as code churn, I do this error code move all the time but it can be dropped if it annoys you apoelstra.

ACKs for top commit:
  apoelstra:
    ACK 4a4d29a

Tree-SHA512: 0e8149fdfa6b2c067bd245c2db52f20bb4218c1edff67023141897a5fd3b76cd3cc69e3513dc176c671f8c48686e7a0d8c1b4ded878ad4280b5c6180202d9a56

src/client.rs

@@ -4,35 +4,32 @@
 //!
 //! Support for connecting to JSONRPC servers over HTTP, sending requests,
 //! and parsing responses
-//!
 
 use std::borrow::Cow;
 use std::collections::HashMap;
 use std::fmt;
+use std::hash::{Hash, Hasher};
 use std::sync::atomic;
 
-use serde;
-use serde_json;
 use serde_json::value::RawValue;
+use serde_json::Value;
 
-use super::{Request, Response};
 use crate::error::Error;
-use crate::util::HashableValue;
+use crate::{Request, Response};
 
 /// An interface for a transport over which to use the JSONRPC protocol.
 pub trait Transport: Send + Sync + 'static {
-    /// Send an RPC request over the transport.
+    /// Sends an RPC request over the transport.
     fn send_request(&self, _: Request) -> Result<Response, Error>;
-    /// Send a batch of RPC requests over the transport.
+    /// Sends a batch of RPC requests over the transport.
     fn send_batch(&self, _: &[Request]) -> Result<Vec<Response>, Error>;
-    /// Format the target of this transport.
-    /// I.e. the URL/socket/...
+    /// Formats the target of this transport. I.e. the URL/socket/...
     fn fmt_target(&self, f: &mut fmt::Formatter) -> fmt::Result;
 }
 
 /// A JSON-RPC client.
 ///
-/// Create a new Client using one of the transport-specific constructors e.g.,
+/// Creates a new Client using one of the transport-specific constructors e.g.,
 /// [`Client::simple_http`] for a bare-minimum HTTP transport.
 pub struct Client {
     pub(crate) transport: Box<dyn Transport>,
@@ -111,7 +108,7 @@ impl Client {
         Ok(results)
     }
 
-    /// Make a request and deserialize the response.
+    /// Makes a request and deserializes the response.
     ///
     /// To construct the arguments, one can use one of the shorthand methods
     /// [`crate::arg`] or [`crate::try_arg`].
@@ -149,9 +146,65 @@ impl<T: Transport> From<T> for Client {
     }
 }
 
+/// Newtype around `Value` which allows hashing for use as hashmap keys,
+/// this is needed for batch requests.
+///
+/// The reason `Value` does not support `Hash` or `Eq` by itself
+/// is that it supports `f64` values; but for batch requests we
+/// will only be hashing the "id" field of the request/response
+/// pair, which should never need decimal precision and therefore
+/// never use `f64`.
+#[derive(Clone, PartialEq, Debug)]
+struct HashableValue<'a>(pub Cow<'a, Value>);
+
+impl<'a> Eq for HashableValue<'a> {}
+
+impl<'a> Hash for HashableValue<'a> {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        match *self.0.as_ref() {
+            Value::Null => "null".hash(state),
+            Value::Bool(false) => "false".hash(state),
+            Value::Bool(true) => "true".hash(state),
+            Value::Number(ref n) => {
+                "number".hash(state);
+                if let Some(n) = n.as_i64() {
+                    n.hash(state);
+                } else if let Some(n) = n.as_u64() {
+                    n.hash(state);
+                } else {
+                    n.to_string().hash(state);
+                }
+            }
+            Value::String(ref s) => {
+                "string".hash(state);
+                s.hash(state);
+            }
+            Value::Array(ref v) => {
+                "array".hash(state);
+                v.len().hash(state);
+                for obj in v {
+                    HashableValue(Cow::Borrowed(obj)).hash(state);
+                }
+            }
+            Value::Object(ref m) => {
+                "object".hash(state);
+                m.len().hash(state);
+                for (key, val) in m {
+                    key.hash(state);
+                    HashableValue(Cow::Borrowed(val)).hash(state);
+                }
+            }
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
+
+    use std::borrow::Cow;
+    use std::collections::HashSet;
+    use std::str::FromStr;
     use std::sync;
 
     struct DummyTransport;
@@ -177,4 +230,38 @@ mod tests {
         assert_eq!(client.nonce.load(sync::atomic::Ordering::Relaxed), 3);
         assert!(req1.id != req2.id);
     }
+
+    #[test]
+    fn hash_value() {
+        let val = HashableValue(Cow::Owned(Value::from_str("null").unwrap()));
+        let t = HashableValue(Cow::Owned(Value::from_str("true").unwrap()));
+        let f = HashableValue(Cow::Owned(Value::from_str("false").unwrap()));
+        let ns =
+            HashableValue(Cow::Owned(Value::from_str("[0, -0, 123.4567, -100000000]").unwrap()));
+        let m =
+            HashableValue(Cow::Owned(Value::from_str("{ \"field\": 0, \"field\": -0 }").unwrap()));
+
+        let mut coll = HashSet::new();
+
+        assert!(!coll.contains(&val));
+        coll.insert(val.clone());
+        assert!(coll.contains(&val));
+
+        assert!(!coll.contains(&t));
+        assert!(!coll.contains(&f));
+        coll.insert(t.clone());
+        assert!(coll.contains(&t));
+        assert!(!coll.contains(&f));
+        coll.insert(f.clone());
+        assert!(coll.contains(&t));
+        assert!(coll.contains(&f));
+
+        assert!(!coll.contains(&ns));
+        coll.insert(ns.clone());
+        assert!(coll.contains(&ns));
+
+        assert!(!coll.contains(&m));
+        coll.insert(m.clone());
+        assert!(coll.contains(&m));
+    }
 }

src/error.rs

@@ -2,13 +2,11 @@
 
 //! # Error handling
 //!
-//! Some useful methods for creating Error objects
-//!
+//! Some useful methods for creating Error objects.
 
 use std::{error, fmt};
 
 use serde::{Deserialize, Serialize};
-use serde_json;
 
 use crate::Response;
 
@@ -50,18 +48,18 @@ impl From<RpcError> for Error {
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use Error::*;
+
         match *self {
-            Error::Transport(ref e) => write!(f, "transport error: {}", e),
-            Error::Json(ref e) => write!(f, "JSON decode error: {}", e),
-            Error::Rpc(ref r) => write!(f, "RPC error response: {:?}", r),
-            Error::BatchDuplicateResponseId(ref v) => {
-                write!(f, "duplicate RPC batch response ID: {}", v)
-            }
-            Error::WrongBatchResponseId(ref v) => write!(f, "wrong RPC batch response ID: {}", v),
-            Error::NonceMismatch => write!(f, "Nonce of response did not match nonce of request"),
-            Error::VersionMismatch => write!(f, "`jsonrpc` field set to non-\"2.0\""),
-            Error::EmptyBatch => write!(f, "batches can't be empty"),
-            Error::WrongBatchResponseSize => write!(f, "too many responses returned in batch"),
+            Transport(ref e) => write!(f, "transport error: {}", e),
+            Json(ref e) => write!(f, "JSON decode error: {}", e),
+            Rpc(ref r) => write!(f, "RPC error response: {:?}", r),
+            BatchDuplicateResponseId(ref v) => write!(f, "duplicate RPC batch response ID: {}", v),
+            WrongBatchResponseId(ref v) => write!(f, "wrong RPC batch response ID: {}", v),
+            NonceMismatch => write!(f, "nonce of response did not match nonce of request"),
+            VersionMismatch => write!(f, "`jsonrpc` field set to non-\"2.0\""),
+            EmptyBatch => write!(f, "batches can't be empty"),
+            WrongBatchResponseSize => write!(f, "too many responses returned in batch"),
         }
     }
 }
@@ -121,8 +119,8 @@ pub enum StandardError {
     InternalError,
 }
 
-#[derive(Clone, Debug, Deserialize, Serialize)]
 /// A JSONRPC error object
+#[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct RpcError {
     /// The integer identifier of the error
     pub code: i32,

src/lib.rs

@@ -3,14 +3,11 @@
 //! # Rust JSON-RPC Library
 //!
 //! Rust support for the JSON-RPC 2.0 protocol.
-//!
 
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 // Coding conventions
 #![warn(missing_docs)]
 
-use serde::{Deserialize, Serialize};
-
 /// Re-export `serde` crate.
 pub extern crate serde;
 /// Re-export `serde_json` crate.
@@ -22,7 +19,6 @@ pub extern crate base64;
 
 pub mod client;
 pub mod error;
-mod util;
 
 #[cfg(feature = "simple_http")]
 pub mod simple_http;
@@ -33,12 +29,12 @@ pub mod simple_tcp;
 #[cfg(all(feature = "simple_uds", not(windows)))]
 pub mod simple_uds;
 
-// Re-export error type
+use serde::{Deserialize, Serialize};
+use serde_json::value::RawValue;
+
 pub use crate::client::{Client, Transport};
 pub use crate::error::Error;
 
-use serde_json::value::RawValue;
-
 /// Shorthand method to convert an argument into a boxed [`serde_json::value::RawValue`].
 ///
 /// Since serializers rarely fail, it's probably easier to use [`arg`] instead.
@@ -60,27 +56,27 @@ pub fn arg<T: serde::Serialize>(arg: T) -> Box<RawValue> {
     }
 }
 
-#[derive(Debug, Clone, Serialize)]
 /// A JSONRPC request object.
+#[derive(Debug, Clone, Serialize)]
 pub struct Request<'a> {
     /// The name of the RPC call.
     pub method: &'a str,
     /// Parameters to the RPC call.
     pub params: &'a [Box<RawValue>],
-    /// Identifier for this Request, which should appear in the response.
+    /// Identifier for this request, which should appear in the response.
     pub id: serde_json::Value,
     /// jsonrpc field, MUST be "2.0".
     pub jsonrpc: Option<&'a str>,
 }
 
-#[derive(Debug, Clone, Deserialize, Serialize)]
 /// A JSONRPC response object.
+#[derive(Debug, Clone, Deserialize, Serialize)]
 pub struct Response {
     /// A result if there is one, or [`None`].
     pub result: Option<Box<RawValue>>,
     /// An error if there is one, or [`None`].
     pub error: Option<error::RpcError>,
-    /// Identifier for this Request, which should match that of the request.
+    /// Identifier for this response, which should match that of the request.
     pub id: serde_json::Value,
     /// jsonrpc field, MUST be "2.0".
     pub jsonrpc: Option<String>,