From 8f68f68378dc6d5acb70b26fbecbd812664362d5 Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Wed, 1 Feb 2017 12:03:34 +0100 Subject: [PATCH 1/5] Use idiomatic `map_or_else` calls to Option instead of double returns Signed-off-by: Victor Porof --- edn/src/types.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/edn/src/types.rs b/edn/src/types.rs index 4fb651ae..eb0af741 100644 --- a/edn/src/types.rs +++ b/edn/src/types.rs @@ -217,15 +217,13 @@ fn to_ord(value: &Value) -> i32 { pub struct Pair(Value, Value); pub fn to_symbol<'a, T: Into>>(namespace: T, name: &str) -> Value { - if let Some(ns) = namespace.into() { - return Value::NamespacedSymbol(symbols::NamespacedSymbol::new(ns, name)); - } - return Value::PlainSymbol(symbols::PlainSymbol::new(name)); + namespace.into().map_or_else( + || Value::PlainSymbol(symbols::PlainSymbol::new(name)), + |ns| Value::NamespacedSymbol(symbols::NamespacedSymbol::new(ns, name))) } pub fn to_keyword<'a, T: Into>>(namespace: T, name: &str) -> Value { - if let Some(ns) = namespace.into() { - return Value::NamespacedKeyword(symbols::NamespacedKeyword::new(ns, name)); - } - return Value::Keyword(symbols::Keyword::new(name)); -} + namespace.into().map_or_else( + || Value::Keyword(symbols::Keyword::new(name)), + |ns| Value::NamespacedKeyword(symbols::NamespacedKeyword::new(ns, name))) +} \ No newline at end of file From 25474980b162ce36fff4991b01a3df073be3b4fa Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Thu, 2 Feb 2017 10:51:42 +0100 Subject: [PATCH 2/5] Add rustdoc comments for `to_symbol` and `to_keyword` functions Signed-off-by: Victor Porof --- edn/src/types.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/edn/src/types.rs b/edn/src/types.rs index eb0af741..53342212 100644 --- a/edn/src/types.rs +++ b/edn/src/types.rs @@ -216,12 +216,42 @@ fn to_ord(value: &Value) -> i32 { pub struct Pair(Value, Value); +/// Converts `name` into a plain or namespaced value symbol, depending on +/// whether or not `namespace` is given. +/// +/// # Examples +/// +/// ``` +/// # use edn::types::to_symbol; +/// # use edn::types::Value; +/// # use edn::symbols; +/// let value = to_symbol("foo", "bar"); +/// assert_eq!(value, Value::NamespacedSymbol(symbols::NamespacedSymbol::new("foo", "bar"))); +/// +/// let value = to_symbol(None, "baz"); +/// assert_eq!(value, Value::PlainSymbol(symbols::PlainSymbol::new("baz"))); +/// ``` pub fn to_symbol<'a, T: Into>>(namespace: T, name: &str) -> Value { namespace.into().map_or_else( || Value::PlainSymbol(symbols::PlainSymbol::new(name)), |ns| Value::NamespacedSymbol(symbols::NamespacedSymbol::new(ns, name))) } +/// Converts `name` into a plain or namespaced value keyword, depending on +/// whether or not `namespace` is given. +/// +/// # Examples +/// +/// ``` +/// # use edn::types::to_keyword; +/// # use edn::types::Value; +/// # use edn::symbols; +/// let value = to_keyword("foo", "bar"); +/// assert_eq!(value, Value::NamespacedKeyword(symbols::NamespacedKeyword::new("foo", "bar"))); +/// +/// let value = to_keyword(None, "baz"); +/// assert_eq!(value, Value::Keyword(symbols::Keyword::new("baz"))); +/// ``` pub fn to_keyword<'a, T: Into>>(namespace: T, name: &str) -> Value { namespace.into().map_or_else( || Value::Keyword(symbols::Keyword::new(name)), From 17bc85fe2702612ad4d2bc745bc00ac8365b3c1c Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Wed, 1 Feb 2017 12:05:47 +0100 Subject: [PATCH 3/5] Remove return statements from edn parser tests Signed-off-by: Victor Porof --- edn/tests/tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/edn/tests/tests.rs b/edn/tests/tests.rs index 46a053c5..ba0255e6 100644 --- a/edn/tests/tests.rs +++ b/edn/tests/tests.rs @@ -25,22 +25,22 @@ use edn::utils; // Helper for making wrapped keywords with a namespace. fn k_ns(ns: &str, name: &str) -> Value { - return NamespacedKeyword(symbols::NamespacedKeyword::new(ns, name)); + NamespacedKeyword(symbols::NamespacedKeyword::new(ns, name)) } // Helper for making wrapped keywords without a namespace. fn k_plain(name: &str) -> Value { - return Keyword(symbols::Keyword::new(name)); + Keyword(symbols::Keyword::new(name)) } // Helper for making wrapped symbols with a namespace fn s_ns(ns: &str, name: &str) -> Value { - return NamespacedSymbol(symbols::NamespacedSymbol::new(ns, name)); + NamespacedSymbol(symbols::NamespacedSymbol::new(ns, name)) } // Helper for making wrapped symbols without a namespace fn s_plain(name: &str) -> Value { - return PlainSymbol(symbols::PlainSymbol::new(name)); + PlainSymbol(symbols::PlainSymbol::new(name)) } #[test] From 4b2c7870c06d124851b8b73d3d58af1f4a3b35f9 Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Wed, 1 Feb 2017 12:07:47 +0100 Subject: [PATCH 4/5] Wrap code indicated by the "_" in documentation as suggested by rustdoc best practices Signed-off-by: Victor Porof --- edn/tests/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/edn/tests/tests.rs b/edn/tests/tests.rs index ba0255e6..1d855079 100644 --- a/edn/tests/tests.rs +++ b/edn/tests/tests.rs @@ -374,10 +374,10 @@ fn test_map() { assert!(map("#{1 #{2 nil} \"hi\"").is_err()); } -/// The test_query_* functions contain the queries taken from the old Clojure implementation of Mentat. +/// The `test_query_*` functions contain the queries taken from the old Clojure implementation of Mentat. /// 2 changes have been applied, which should be checked and maybe fixed /// TODO: Decide if these queries should be placed in a vector wrapper. Is that implied? -/// Secondly, see note in test_query_starred_pages on the use of ' +/// Secondly, see note in `test_query_starred_pages` on the use of ' #[test] fn test_query_active_sessions() { let test = "[ From 4e9e8ed837688ab3b7b798f6b0751da15e8d4b4a Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Wed, 1 Feb 2017 12:15:15 +0100 Subject: [PATCH 5/5] Use idiomatic `enumerate` method on interators instead of iterating over indices Signed-off-by: Victor Porof --- edn/tests/tests.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/edn/tests/tests.rs b/edn/tests/tests.rs index 1d855079..4d42d21e 100644 --- a/edn/tests/tests.rs +++ b/edn/tests/tests.rs @@ -926,9 +926,7 @@ fn test_is_and_as_type_helper_functions() { ])), ]; - for i in 0..values.len() { - let value = &values[i]; - + for (i, value) in values.iter().enumerate() { let is_result = [ value.is_nil(), value.is_boolean(), @@ -946,12 +944,17 @@ fn test_is_and_as_type_helper_functions() { value.is_map(), ]; - for j in 0..values.len() { - assert_eq!(j == i, is_result[j]); + assert_eq!(values.len(), is_result.len()); + + for (j, result) in is_result.iter().enumerate() { + assert_eq!(j == i, *result); } - if i == 0 { assert_eq!(value.as_nil().unwrap(), ()) } - else { assert!(!value.as_nil().is_some()) } + if i == 0 { + assert_eq!(value.as_nil().unwrap(), ()) + } else { + assert!(!value.as_nil().is_some()) + } def_test_as_type!(value, as_boolean, i == 1, false); def_test_as_type!(value, as_integer, i == 2, 1i64);