From ab0d8cb9aa107c8d561f3c188e6cbf472a7df23b Mon Sep 17 00:00:00 2001 From: Ryan Leckey Date: Tue, 13 Jun 2017 02:21:46 -0700 Subject: :shirt: Fix clippy warnings --- Cargo.toml | 1 + src/config.rs | 2 +- src/error.rs | 26 +++++++++++++++++++++----- src/lib.rs | 20 ++++++++++++++++++++ src/path/mod.rs | 12 ++++++------ src/path/parser.rs | 11 ++++++----- src/value.rs | 12 ++++++++++-- tests/get_scalar.rs | 42 +++++++++++++++++++++--------------------- tests/get_struct.rs | 12 +++++++----- tests/merge.rs | 8 ++++---- 10 files changed, 97 insertions(+), 49 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c077840..4c57803 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,3 +24,4 @@ yaml-rust = { version = "^0.3.5", optional = true } [dev-dependencies] serde_derive = "^1.0.8" +float-cmp = "*" diff --git a/src/config.rs b/src/config.rs index ad5421b..730b261 100644 --- a/src/config.rs +++ b/src/config.rs @@ -105,7 +105,7 @@ impl Config { } pub fn deserialize<'de, T: Deserialize<'de>>(&self) -> Result { - return T::deserialize(self.cache.clone()); + T::deserialize(self.cache.clone()) } pub fn get<'de, T: Deserialize<'de>>(&self, key: &'de str) -> Result { diff --git a/src/error.rs b/src/error.rs index 92709c2..6b567a2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -43,13 +43,31 @@ pub enum ConfigError { PathParse(nom::ErrorKind), /// Configuration could not be parsed from file. - FileParse { uri: Option, cause: Box }, + FileParse { + /// The URI used to access the file (if not loaded from a string). + /// Example: `/path/to/config.json` + uri: Option, + + /// The captured error from attempting to parse the file in its desired format. + /// This is the actual error object from the library used for the parsing. + cause: Box + }, /// Value could not be converted into the requested type. Type { + /// The URI that references the source that the value came from. + /// Example: `/path/to/config.json` or `Environment` or `etcd://localhost` + // TODO: Why is this called Origin but FileParse has a uri field? origin: Option, + + /// What we found when parsing the value unexpected: Unexpected, + + /// What was expected when parsing the value expected: &'static str, + + /// The key in the configuration hash of this value (if available where the + /// error is generated). key: Option, }, @@ -153,8 +171,7 @@ impl Error for ConfigError { ConfigError::Frozen => "configuration is frozen", ConfigError::NotFound(_) => "configuration property not found", ConfigError::Type { .. } => "invalid type", - ConfigError::Foreign(ref cause) => cause.description(), - ConfigError::FileParse { ref cause, .. } => cause.description(), + ConfigError::Foreign(ref cause) | ConfigError::FileParse { ref cause, .. } => cause.description(), ConfigError::PathParse(ref kind) => kind.description(), _ => "configuration error", @@ -163,8 +180,7 @@ impl Error for ConfigError { fn cause(&self) -> Option<&Error> { match *self { - ConfigError::Foreign(ref cause) => Some(cause.as_ref()), - ConfigError::FileParse { ref cause, .. } => Some(cause.as_ref()), + ConfigError::Foreign(ref cause) | ConfigError::FileParse { ref cause, .. } => Some(cause.as_ref()), _ => None } diff --git a/src/lib.rs b/src/lib.rs index 212e621..5d06364 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,26 @@ +//! Config organizes hierarchical or layered configurations for Rust applications. +//! +//! Config lets you set a set of default parameters and then extend them via merging in +//! configuration from a variety of sources: +//! - Environment variables +//! - Another Config instance +//! - Remote configuration: etcd, Consul +//! - Files: JSON, YAML, TOML +//! - Manual, programmatic override (via a `.set` method on the Config instance) +//! +//! Additionally, Config supports: +//! - Live watching and re-reading of configuration files +//! - Deep access into the merged configuration via a path syntax +//! - Deserialization via `serde` of the configuration or any subset defined via a path +//! +//! See the [examples](https://github.com/mehcode/config-rs/tree/master/examples) for +//! general usage information. + #![allow(dead_code)] #![allow(unused_imports)] #![allow(unused_variables)] +#![allow(unknown_lints)] +// #![warn(missing_docs)] #[macro_use] extern crate serde; diff --git a/src/path/mod.rs b/src/path/mod.rs index d2df442..ff93287 100644 --- a/src/path/mod.rs +++ b/src/path/mod.rs @@ -17,12 +17,12 @@ impl FromStr for Expression { type Err = ConfigError; fn from_str(s: &str) -> Result { - parser::from_str(s).map_err(|kind| ConfigError::PathParse(kind)) + parser::from_str(s).map_err(ConfigError::PathParse) } } impl Expression { - pub fn get<'a>(self, root: &'a Value) -> Option<&'a Value> { + pub fn get(self, root: &Value) -> Option<&Value> { match self { Expression::Identifier(id) => { match root.kind { @@ -61,7 +61,7 @@ impl Expression { Expression::Identifier(ref id) => { match root.kind { ValueKind::Table(ref mut map) => { - Some(map.entry(id.clone()).or_insert(Value::new(None, ValueKind::Nil))) + Some(map.entry(id.clone()).or_insert_with(|| Value::new(None, ValueKind::Nil))) } _ => None, @@ -73,14 +73,14 @@ impl Expression { Some(value) => { match value.kind { ValueKind::Table(ref mut map) => { - Some(map.entry(key.clone()).or_insert(Value::new(None, ValueKind::Nil))) + Some(map.entry(key.clone()).or_insert_with(|| Value::new(None, ValueKind::Nil))) } _ => { *value = HashMap::::new().into(); if let ValueKind::Table(ref mut map) = value.kind { - Some(map.entry(key.clone()).or_insert(Value::new(None, ValueKind::Nil))) + Some(map.entry(key.clone()).or_insert_with(|| Value::new(None, ValueKind::Nil))) } else { println!("WHAT THE FUCK?"); @@ -116,7 +116,7 @@ impl Expression { ValueKind::Table(ref incoming_map) => { // Pull out another table let mut target = if let ValueKind::Table(ref mut map) = root.kind { - map.entry(id.clone()).or_insert(HashMap::::new().into()) + map.entry(id.clone()).or_insert_with(|| HashMap::::new().into()) } else { unreachable!(); }; diff --git a/src/path/parser.rs b/src/path/parser.rs index eea4343..ee3fb65 100644 --- a/src/path/parser.rs +++ b/src/path/parser.rs @@ -28,8 +28,9 @@ named!(integer , named!(ident, map!(ident_, Expression::Identifier)); +#[allow(cyclomatic_complexity)] fn postfix(expr: Expression) -> Box IResult<&[u8], Expression>> { - return Box::new(move |i: &[u8]| { + Box::new(move |i: &[u8]| { alt!(i, do_parse!( tag!(".") >> @@ -49,13 +50,13 @@ fn postfix(expr: Expression) -> Box IResult<&[u8], Expression>> { char!(']') ) ) - }); + }) } pub fn from_str(input: &str) -> Result { match ident(input.as_bytes()) { IResult::Done(mut rem, mut expr) => { - while rem.len() > 0 { + while !rem.is_empty() { match postfix(expr)(rem) { IResult::Done(rem_, expr_) => { rem = rem_; @@ -63,7 +64,7 @@ pub fn from_str(input: &str) -> Result { } // Forward Incomplete and Error - result @ _ => { + result => { return result.to_result(); } } @@ -73,7 +74,7 @@ pub fn from_str(input: &str) -> Result { } // Forward Incomplete and Error - result @ _ => result.to_result(), + result => result.to_result(), } } diff --git a/src/value.rs b/src/value.rs index 25d9709..dd48531 100644 --- a/src/value.rs +++ b/src/value.rs @@ -120,6 +120,7 @@ pub struct Value { } impl Value { + /// Create a new value instance that will remember its source uri. pub fn new(origin: Option<&String>, kind: V) -> Self where V: Into { @@ -129,11 +130,13 @@ impl Value { } } + /// Attempt to deserialize this value into the requested type. pub fn try_into<'de, T: Deserialize<'de>>(self) -> Result { - return T::deserialize(self); + T::deserialize(self) } /// Returns `self` as a bool, if possible. + // FIXME: Should this not be `try_into_*` ? pub fn into_bool(self) -> Result { match self.kind { ValueKind::Boolean(value) => Ok(value), @@ -146,7 +149,7 @@ impl Value { "0" | "false" | "off" | "no" => Ok(false), // Unexpected string value - s @ _ => { + s => { Err(ConfigError::invalid_type(self.origin.clone(), Unexpected::Str(s.into()), "a boolean")) @@ -168,6 +171,7 @@ impl Value { } /// Returns `self` into an i64, if possible. + // FIXME: Should this not be `try_into_*` ? pub fn into_int(self) -> Result { match self.kind { ValueKind::Integer(value) => Ok(value), @@ -204,6 +208,7 @@ impl Value { } /// Returns `self` into a f64, if possible. + // FIXME: Should this not be `try_into_*` ? pub fn into_float(self) -> Result { match self.kind { ValueKind::Float(value) => Ok(value), @@ -246,6 +251,7 @@ impl Value { } /// Returns `self` into a str, if possible. + // FIXME: Should this not be `try_into_*` ? pub fn into_str(self) -> Result { match self.kind { ValueKind::String(value) => Ok(value), @@ -268,6 +274,7 @@ impl Value { } /// Returns `self` into an array, if possible + // FIXME: Should this not be `try_into_*` ? pub fn into_array(self) -> Result> { match self.kind { ValueKind::Array(value) => Ok(value), @@ -295,6 +302,7 @@ impl Value { } /// If the `Value` is a Table, returns the associated Map. + // FIXME: Should this not be `try_into_*` ? pub fn into_table(self) -> Result> { match self.kind { ValueKind::Table(value) => Ok(value), diff --git a/tests/get_scalar.rs b/tests/get_scalar.rs index 08d8507..5d54e18 100644 --- a/tests/get_scalar.rs +++ b/tests/get_scalar.rs @@ -14,33 +14,33 @@ fn make() -> Config { fn test_scalar() { let c = make(); - assert!(c.get("debug").ok() == Some(true)); - assert!(c.get("production").ok() == Some(false)); + assert_eq!(c.get("debug").ok(), Some(true)); + assert_eq!(c.get("production").ok(), Some(false)); } #[test] fn test_scalar_type_loose() { let c = make(); - assert!(c.get("debug").ok() == Some(true)); - assert!(c.get("debug").ok() == Some("true".to_string())); - assert!(c.get("debug").ok() == Some(1)); - assert!(c.get("debug").ok() == Some(1.0)); - - assert!(c.get("debug_s").ok() == Some(true)); - assert!(c.get("debug_s").ok() == Some("true".to_string())); - assert!(c.get("debug_s").ok() == Some(1)); - assert!(c.get("debug_s").ok() == Some(1.0)); - - assert!(c.get("production").ok() == Some(false)); - assert!(c.get("production").ok() == Some("false".to_string())); - assert!(c.get("production").ok() == Some(0)); - assert!(c.get("production").ok() == Some(0.0)); - - assert!(c.get("production_s").ok() == Some(false)); - assert!(c.get("production_s").ok() == Some("false".to_string())); - assert!(c.get("production_s").ok() == Some(0)); - assert!(c.get("production_s").ok() == Some(0.0)); + assert_eq!(c.get("debug").ok(), Some(true)); + assert_eq!(c.get("debug").ok(), Some("true".to_string())); + assert_eq!(c.get("debug").ok(), Some(1)); + assert_eq!(c.get("debug").ok(), Some(1.0)); + + assert_eq!(c.get("debug_s").ok(), Some(true)); + assert_eq!(c.get("debug_s").ok(), Some("true".to_string())); + assert_eq!(c.get("debug_s").ok(), Some(1)); + assert_eq!(c.get("debug_s").ok(), Some(1.0)); + + assert_eq!(c.get("production").ok(), Some(false)); + assert_eq!(c.get("production").ok(), Some("false".to_string())); + assert_eq!(c.get("production").ok(), Some(0)); + assert_eq!(c.get("production").ok(), Some(0.0)); + + assert_eq!(c.get("production_s").ok(), Some(false)); + assert_eq!(c.get("production_s").ok(), Some("false".to_string())); + assert_eq!(c.get("production_s").ok(), Some(0)); + assert_eq!(c.get("production_s").ok(), Some(0.0)); } #[test] diff --git a/tests/get_struct.rs b/tests/get_struct.rs index 705c769..57e4bb2 100644 --- a/tests/get_struct.rs +++ b/tests/get_struct.rs @@ -1,10 +1,12 @@ extern crate config; extern crate serde; +extern crate float_cmp; #[macro_use] extern crate serde_derive; use config::*; +use float_cmp::ApproxEqUlps; #[derive(Debug, Deserialize)] struct Place { @@ -39,11 +41,11 @@ fn test_file_struct() { // Deserialize the entire file as single struct let s: Settings = c.deserialize().unwrap(); - assert_eq!(s.debug, 1.0); + assert!(s.debug.approx_eq_ulps(&1.0, 2)); assert_eq!(s.production, Some("false".to_string())); assert_eq!(s.place.name, "Torre di Pisa"); - assert_eq!(s.place.longitude, 43.7224985); - assert_eq!(s.place.latitude, 10.3970522); + assert!(s.place.longitude.approx_eq_ulps(&43.7224985, 2)); + assert!(s.place.latitude.approx_eq_ulps(&10.3970522, 2)); assert_eq!(s.place.favorite, false); assert_eq!(s.place.reviews, 3866); assert_eq!(s.place.rating, Some(4.5)); @@ -59,8 +61,8 @@ fn test_scalar_struct() { let p: Place = c.get("place").unwrap(); assert_eq!(p.name, "Torre di Pisa"); - assert_eq!(p.longitude, 43.7224985); - assert_eq!(p.latitude, 10.3970522); + assert!(p.longitude.approx_eq_ulps(&43.7224985, 2)); + assert!(p.latitude.approx_eq_ulps(&10.3970522, 2)); assert_eq!(p.favorite, false); assert_eq!(p.reviews, 3866); assert_eq!(p.rating, Some(4.5)); diff --git a/tests/merge.rs b/tests/merge.rs index df4ace8..4a42d6a 100644 --- a/tests/merge.rs +++ b/tests/merge.rs @@ -17,8 +17,8 @@ fn make() -> Config { fn test_merge() { let c = make(); - assert!(c.get("debug").ok() == Some(false)); - assert!(c.get("production").ok() == Some(true)); - assert!(c.get("place.creator.name").ok() == Some("Somebody New".to_string())); - assert!(c.get("place.rating").ok() == Some(4.9)); + assert_eq!(c.get("debug").ok(), Some(false)); + assert_eq!(c.get("production").ok(), Some(true)); + assert_eq!(c.get("place.creator.name").ok(), Some("Somebody New".to_string())); + assert_eq!(c.get("place.rating").ok(), Some(4.9)); } -- cgit v1.2.3