Modernize codebase, fix clippy warnings

.travis.yml

@@ -2,13 +2,14 @@ sudo: true
 language: rust
 cache: cargo
 rust:
-  - nightly
-  - beta
-  - stable
+    - nightly
+    - beta
+    - stable
 
 matrix:
     allow_failures:
         - rust: nightly
 
 script:
-  - cargo test
+    - cargo clippy --tests -- -D warnings
+    - cargo test

src/config.rs

@@ -18,7 +18,7 @@ enum ConfigKind {
     Mutable {
         defaults: HashMap<path::Expression, Value>,
         overrides: HashMap<path::Expression, Value>,
-        sources: Vec<Box<Source + Send + Sync>>,
+        sources: Vec<Box<dyn Source + Send + Sync>>,
     },
 
     // A frozen configuration.
@@ -212,7 +212,7 @@ impl Config {
 }
 
 impl Source for Config {
-    fn clone_into_box(&self) -> Box<Source + Send + Sync> {
+    fn clone_into_box(&self) -> Box<dyn Source + Send + Sync> {
         Box::new((*self).clone())
     }
 

src/de.rs

@@ -3,7 +3,7 @@ use error::*;
 use serde::de;
 use std::collections::{HashMap, VecDeque};
 use std::iter::Enumerate;
-use value::{Value, ValueKind, Table};
+use value::{Table, Value, ValueKind};
 
 impl<'de> de::Deserializer<'de> for Value {
     type Error = ConfigError;
@@ -125,7 +125,11 @@ impl<'de> de::Deserializer<'de> for Value {
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_enum(EnumAccess{ value: self, name: name, variants: variants })
+        visitor.visit_enum(EnumAccess {
+            value: self,
+            name,
+            variants,
+        })
     }
 
     forward_to_deserialize_any! {
@@ -178,11 +182,10 @@ impl<'de> de::SeqAccess<'de> for SeqAccess {
         T: de::DeserializeSeed<'de>,
     {
         match self.elements.next() {
-            Some((idx, value)) => {
-                seed.deserialize(value)
-                    .map(Some)
-                    .map_err(|e| e.prepend_index(idx))
-            }
+            Some((idx, value)) => seed
+                .deserialize(value)
+                .map(Some)
+                .map_err(|e| e.prepend_index(idx)),
             None => Ok(None),
         }
     }
@@ -229,8 +232,7 @@ impl<'de> de::MapAccess<'de> for MapAccess {
         V: de::DeserializeSeed<'de>,
     {
         let (key, value) = self.elements.pop_front().unwrap();
-        de::DeserializeSeed::deserialize(seed, value)
-            .map_err(|e| e.prepend_key(key))
+        de::DeserializeSeed::deserialize(seed, value).map_err(|e| e.prepend_key(key))
     }
 }
 
@@ -241,12 +243,12 @@ struct EnumAccess {
 }
 
 impl EnumAccess {
-    fn variant_deserializer(&self, name: &String) -> Result<StrDeserializer> {
+    fn variant_deserializer(&self, name: &str) -> Result<StrDeserializer> {
         self.variants
             .iter()
-            .find(|&s| s == name)
+            .find(|&s| *s == name)
             .map(|&s| StrDeserializer(s))
-            .ok_or(self.no_constructor_error(name))
+            .ok_or_else(|| self.no_constructor_error(name))
     }
 
     fn table_deserializer(&self, table: &Table) -> Result<StrDeserializer> {
@@ -315,21 +317,21 @@ impl<'de> de::VariantAccess<'de> for EnumAccess {
         V: de::Visitor<'de>,
     {
         match self.value.kind {
-            ValueKind::Table(t) => de::Deserializer::deserialize_seq(t.into_iter().next().unwrap().1, visitor),
+            ValueKind::Table(t) => {
+                de::Deserializer::deserialize_seq(t.into_iter().next().unwrap().1, visitor)
+            }
             _ => unreachable!(),
         }
     }
 
-    fn struct_variant<V>(
-        self,
-        _fields: &'static [&'static str],
-        visitor: V,
-    ) -> Result<V::Value>
+    fn struct_variant<V>(self, _fields: &'static [&'static str], visitor: V) -> Result<V::Value>
     where
         V: de::Visitor<'de>,
     {
         match self.value.kind {
-            ValueKind::Table(t) => de::Deserializer::deserialize_map(t.into_iter().next().unwrap().1, visitor),
+            ValueKind::Table(t) => {
+                de::Deserializer::deserialize_map(t.into_iter().next().unwrap().1, visitor)
+            }
             _ => unreachable!(),
         }
     }
@@ -448,7 +450,11 @@ impl<'de> de::Deserializer<'de> for Config {
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_enum(EnumAccess{ value: self.cache, name: name, variants: variants })
+        visitor.visit_enum(EnumAccess {
+            value: self.cache,
+            name,
+            variants,
+        })
     }
 
     forward_to_deserialize_any! {

src/env.rs

@@ -63,7 +63,7 @@ impl Default for Environment {
 }
 
 impl Source for Environment {
-    fn clone_into_box(&self) -> Box<Source + Send + Sync> {
+    fn clone_into_box(&self) -> Box<dyn Source + Send + Sync> {
         Box::new((*self).clone())
     }
 

src/error.rs

@@ -51,7 +51,7 @@ pub enum ConfigError {
 
         /// 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<Error + Send + Sync>,
+        cause: Box<dyn Error + Send + Sync>,
     },
 
     /// Value could not be converted into the requested type.
@@ -76,7 +76,7 @@ pub enum ConfigError {
     Message(String),
 
     /// Unadorned error from a foreign origin.
-    Foreign(Box<Error + Send + Sync>),
+    Foreign(Box<dyn Error + Send + Sync>),
 }
 
 impl ConfigError {
@@ -88,9 +88,9 @@ impl ConfigError {
         expected: &'static str,
     ) -> Self {
         ConfigError::Type {
-            origin: origin,
-            unexpected: unexpected,
-            expected: expected,
+            origin,
+            unexpected,
+            expected,
             key: None,
         }
     }
@@ -105,9 +105,9 @@ impl ConfigError {
                 expected,
                 ..
             } => ConfigError::Type {
-                origin: origin,
-                unexpected: unexpected,
-                expected: expected,
+                origin,
+                unexpected,
+                expected,
                 key: Some(key.into()),
             },
 
@@ -131,14 +131,12 @@ impl ConfigError {
                 unexpected,
                 expected,
                 key,
-            } => {
-                ConfigError::Type {
-                    origin,
-                    unexpected,
-                    expected,
-                    key: Some(concat(key)),
-                }
-            }
+            } => ConfigError::Type {
+                origin,
+                unexpected,
+                expected,
+                key: Some(concat(key)),
+            },
             ConfigError::NotFound(key) => ConfigError::NotFound(concat(Some(key))),
             _ => self,
         }
@@ -166,7 +164,8 @@ impl fmt::Debug for ConfigError {
 impl fmt::Display for ConfigError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
-            ConfigError::Frozen | ConfigError::PathParse(_) => write!(f, "{}", self.description()),
+            ConfigError::Frozen => write!(f, "configuration is frozen"),
+            ConfigError::PathParse(kind) => writeln!(f, "{}", kind.description()),
 
             ConfigError::Message(ref s) => write!(f, "{}", s),
 
@@ -209,21 +208,7 @@ impl fmt::Display for ConfigError {
 }
 
 impl Error for ConfigError {
-    fn description(&self) -> &str {
-        match *self {
-            ConfigError::Frozen => "configuration is frozen",
-            ConfigError::NotFound(_) => "configuration property not found",
-            ConfigError::Type { .. } => "invalid type",
-            ConfigError::Foreign(ref cause) | ConfigError::FileParse { ref cause, .. } => {
-                cause.description()
-            }
-            ConfigError::PathParse(ref kind) => kind.description(),
-
-            _ => "configuration error",
-        }
-    }
-
-    fn cause(&self) -> Option<&Error> {
+    fn cause(&self) -> Option<&dyn Error> {
         match *self {
             ConfigError::Foreign(ref cause) | ConfigError::FileParse { ref cause, .. } => {
                 Some(cause.as_ref())

src/file/format/hjson.rs

@@ -7,7 +7,7 @@ use value::{Value, ValueKind};
 pub fn parse(
     uri: Option<&String>,
     text: &str,
-) -> Result<HashMap<String, Value>, Box<Error + Send + Sync>> {
+) -> Result<HashMap<String, Value>, Box<dyn Error + Send + Sync>> {
     // Parse a JSON object value from the text
     // TODO: Have a proper error fire if the root of a file is ever not a Table
     let value = from_hjson_value(uri, &serde_hjson::from_str(text)?);

src/file/format/ini.rs

@@ -7,7 +7,7 @@ use value::{Value, ValueKind};
 pub fn parse(
     uri: Option<&String>,
     text: &str,
-) -> Result<HashMap<String, Value>, Box<Error + Send + Sync>> {
+) -> Result<HashMap<String, Value>, Box<dyn Error + Send + Sync>> {
     let mut map: HashMap<String, Value> = HashMap::new();
     let i = Ini::load_from_str(text)?;
     for (sec, prop) in i.iter() {

src/file/format/json.rs

@@ -7,7 +7,7 @@ use value::{Value, ValueKind};
 pub fn parse(
     uri: Option<&String>,
     text: &str,
-) -> Result<HashMap<String, Value>, Box<Error + Send + Sync>> {
+) -> Result<HashMap<String, Value>, Box<dyn Error + Send + Sync>> {
     // Parse a JSON object value from the text
     // TODO: Have a proper error fire if the root of a file is ever not a Table
     let value = from_json_value(uri, &serde_json::from_str(text)?);

src/file/format/mod.rs

@@ -86,7 +86,7 @@ impl FileFormat {
         &self,
         uri: Option<&String>,
         text: &str,
-    ) -> Result<HashMap<String, Value>, Box<Error + Send + Sync>> {
+    ) -> Result<HashMap<String, Value>, Box<dyn Error + Send + Sync>> {
         match *self {
             #[cfg(feature = "toml")]
             FileFormat::Toml => toml::parse(uri, text),

src/file/format/toml.rs

@@ -7,7 +7,7 @@ use value::{Value, ValueKind};
 pub fn parse(
     uri: Option<&String>,
     text: &str,
-) -> Result<HashMap<String, Value>, Box<Error + Send + Sync>> {
+) -> Result<HashMap<String, Value>, Box<dyn Error + Send + Sync>> {
     // Parse a TOML value from the provided text
     // TODO: Have a proper error fire if the root of a file is ever not a Table
     let value = from_toml_value(uri, &toml::from_str(text)?);

src/file/format/yaml.rs

@@ -9,7 +9,7 @@ use yaml_rust as yaml;
 pub fn parse(
     uri: Option<&String>,
     text: &str,
-) -> Result<HashMap<String, Value>, Box<Error + Send + Sync>> {
+) -> Result<HashMap<String, Value>, Box<dyn Error + Send + Sync>> {
     // Parse a YAML object from file
     let mut docs = yaml::YamlLoader::load_from_str(text)?;
     let root = match docs.len() {

src/file/mod.rs

@@ -94,7 +94,7 @@ where
     T: 'static,
     T: Sync + Send,
 {
-    fn clone_into_box(&self) -> Box<Source + Send + Sync> {
+    fn clone_into_box(&self) -> Box<dyn Source + Send + Sync> {
         Box::new((*self).clone())
     }
 
@@ -120,8 +120,8 @@ where
         format
             .parse(uri.as_ref(), &contents)
             .map_err(|cause| ConfigError::FileParse {
-                uri: uri,
-                cause: cause,
+                uri,
+                cause,
             })
     }
 }

src/file/source/file.rs

@@ -21,13 +21,13 @@ pub struct FileSourceFile {
 
 impl FileSourceFile {
     pub fn new(name: PathBuf) -> FileSourceFile {
-        FileSourceFile { name: name }
+        FileSourceFile { name }
     }
 
     fn find_file(
         &self,
         format_hint: Option<FileFormat>,
-    ) -> Result<(PathBuf, FileFormat), Box<Error + Send + Sync>> {
+    ) -> Result<(PathBuf, FileFormat), Box<dyn Error + Send + Sync>> {
         // First check for an _exact_ match
         let mut filename = env::current_dir()?.as_path().join(self.name.clone());
         if filename.is_file() {
@@ -91,7 +91,7 @@ impl FileSource for FileSourceFile {
     fn resolve(
         &self,
         format_hint: Option<FileFormat>,
-    ) -> Result<(Option<String>, String, FileFormat), Box<Error + Send + Sync>> {
+    ) -> Result<(Option<String>, String, FileFormat), Box<dyn Error + Send + Sync>> {
         // Find file
         let (filename, format) = self.find_file(format_hint)?;
 
@@ -103,7 +103,7 @@ impl FileSource for FileSourceFile {
         };
 
         // Read contents from file
-        let mut file = fs::File::open(filename.clone())?;
+        let mut file = fs::File::open(filename)?;
         let mut text = String::new();
         file.read_to_string(&mut text)?;
 

src/file/source/mod.rs

@@ -12,5 +12,5 @@ pub trait FileSource: Debug + Clone {
     fn resolve(
         &self,
         format_hint: Option<FileFormat>,
-    ) -> Result<(Option<String>, String, FileFormat), Box<Error + Send + Sync>>;
+    ) -> Result<(Option<String>, String, FileFormat), Box<dyn Error + Send + Sync>>;
 }

src/file/source/string.rs

@@ -19,7 +19,7 @@ impl FileSource for FileSourceString {
     fn resolve(
         &self,
         format_hint: Option<FileFormat>,
-    ) -> Result<(Option<String>, String, FileFormat), Box<Error + Send + Sync>> {
+    ) -> Result<(Option<String>, String, FileFormat), Box<dyn Error + Send + Sync>> {
         Ok((
             None,
             self.0.clone(),