Skip to content

Rust idoms and clippy warnings #138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -161,8 +161,7 @@ impl Config {
match value {
Some(value) => {
// Deserialize the received value into the requested type
T::deserialize(value)
.map_err(|e| e.extend_with_key(key))
T::deserialize(value).map_err(|e| e.extend_with_key(key))
}

None => Err(ConfigError::NotFound(key.into())),
Expand Down Expand Up @@ -212,7 +211,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())
}

Expand Down
46 changes: 26 additions & 20 deletions src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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! {
Expand Down Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -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))
}
}

Expand All @@ -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> {
Expand Down Expand Up @@ -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!(),
}
}
Expand Down Expand Up @@ -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! {
Expand Down
2 changes: 1 addition & 1 deletion src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand Down
48 changes: 16 additions & 32 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -88,9 +88,9 @@ impl ConfigError {
expected: &'static str,
) -> Self {
ConfigError::Type {
origin: origin,
unexpected: unexpected,
expected: expected,
origin,
unexpected,
expected,
key: None,
}
}
Expand All @@ -105,9 +105,9 @@ impl ConfigError {
expected,
..
} => ConfigError::Type {
origin: origin,
unexpected: unexpected,
expected: expected,
origin,
unexpected,
expected,
key: Some(key.into()),
},

Expand All @@ -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,
}
Expand Down Expand Up @@ -166,7 +164,7 @@ 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 | ConfigError::PathParse(_) => write!(f, "{}", self.to_string()),

ConfigError::Message(ref s) => write!(f, "{}", s),

Expand Down Expand Up @@ -209,21 +207,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())
Expand Down
2 changes: 1 addition & 1 deletion src/file/format/hjson.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?);
Expand Down
17 changes: 4 additions & 13 deletions src/file/format/ini.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,21 @@ 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() {
match *sec {
Some(ref sec) => {
let mut sec_map: HashMap<String, Value> = HashMap::new();
for (k, v) in prop.iter() {
sec_map.insert(
k.clone(),
Value::new(uri, ValueKind::String(v.clone())),
);
sec_map.insert(k.clone(), Value::new(uri, ValueKind::String(v.clone())));
}
map.insert(
sec.clone(),
Value::new(uri, ValueKind::Table(sec_map)),
);
map.insert(sec.clone(), Value::new(uri, ValueKind::Table(sec_map)));
}
None => {
for (k, v) in prop.iter() {
map.insert(
k.clone(),
Value::new(uri, ValueKind::String(v.clone())),
);
map.insert(k.clone(), Value::new(uri, ValueKind::String(v.clone())));
}
}
}
Expand Down
18 changes: 10 additions & 8 deletions src/file/format/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?);
Expand All @@ -22,13 +22,15 @@ fn from_json_value(uri: Option<&String>, value: &serde_json::Value) -> Value {
match *value {
serde_json::Value::String(ref value) => Value::new(uri, ValueKind::String(value.clone())),

serde_json::Value::Number(ref value) => if let Some(value) = value.as_i64() {
Value::new(uri, ValueKind::Integer(value))
} else if let Some(value) = value.as_f64() {
Value::new(uri, ValueKind::Float(value))
} else {
unreachable!();
},
serde_json::Value::Number(ref value) => {
if let Some(value) = value.as_i64() {
Value::new(uri, ValueKind::Integer(value))
} else if let Some(value) = value.as_f64() {
Value::new(uri, ValueKind::Float(value))
} else {
unreachable!();
}
}

serde_json::Value::Bool(value) => Value::new(uri, ValueKind::Boolean(value)),

Expand Down
10 changes: 5 additions & 5 deletions src/file/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,22 @@ lazy_static! {
impl FileFormat {
// TODO: pub(crate)
#[doc(hidden)]
pub fn extensions(&self) -> &'static Vec<&'static str> {
pub fn extensions(self) -> &'static Vec<&'static str> {
// It should not be possible for this to fail
// A FileFormat would need to be declared without being added to the
// ALL_EXTENSIONS map.
ALL_EXTENSIONS.get(self).unwrap()
ALL_EXTENSIONS.get(&self).unwrap()
}

// TODO: pub(crate)
#[doc(hidden)]
#[allow(unused_variables)]
pub fn parse(
&self,
self,
uri: Option<&String>,
text: &str,
) -> Result<HashMap<String, Value>, Box<Error + Send + Sync>> {
match *self {
) -> Result<HashMap<String, Value>, Box<dyn Error + Send + Sync>> {
match self {
#[cfg(feature = "toml")]
FileFormat::Toml => toml::parse(uri, text),

Expand Down
2 changes: 1 addition & 1 deletion src/file/format/toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?);
Expand Down
2 changes: 1 addition & 1 deletion src/file/format/yaml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
9 changes: 3 additions & 6 deletions src/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use value::Value;
pub use self::format::FileFormat;
use self::source::FileSource;

pub use self::source::string::FileSourceString;
pub use self::source::file::FileSourceFile;
pub use self::source::string::FileSourceString;

#[derive(Clone, Debug)]
pub struct File<T>
Expand Down Expand Up @@ -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())
}

Expand All @@ -119,9 +119,6 @@ where
// Parse the string using the given format
format
.parse(uri.as_ref(), &contents)
.map_err(|cause| ConfigError::FileParse {
uri: uri,
cause: cause,
})
.map_err(|cause| ConfigError::FileParse { uri, cause })
}
}
Loading