From d1982ad8af36a0e7f444f21f424aa66ca8252714 Mon Sep 17 00:00:00 2001 From: Dmitriy Pleshevskiy Date: Thu, 15 Apr 2021 23:35:57 +0300 Subject: [PATCH 1/4] feat: add possibility to get optional env --- itconfig-macro/src/expand.rs | 31 ++++++++ itconfig-tests/tests/config_macro.rs | 19 +++++ itconfig-tests/tests/get_env.rs | 74 ----------------- itconfig/src/getenv.rs | 114 +++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 74 deletions(-) delete mode 100644 itconfig-tests/tests/get_env.rs diff --git a/itconfig-macro/src/expand.rs b/itconfig-macro/src/expand.rs index 3789ab3..99db9dc 100644 --- a/itconfig-macro/src/expand.rs +++ b/itconfig-macro/src/expand.rs @@ -1,6 +1,8 @@ use crate::ast::*; use proc_macro2::TokenStream as TokenStream2; use quote::{quote, ToTokens, TokenStreamExt}; +use syn::Path; +use syn::Type; fn vec_to_token_stream_2(input: &Vec) -> Vec where @@ -145,6 +147,8 @@ impl ToTokens for Variable { } else if self.initial.is_some() { let initial = self.initial.as_ref().unwrap(); quote!(::itconfig::get_env_or_set_default(#env_name, #initial)) + } else if is_option_type(&self.ty) { + quote!(::itconfig::maybe_get_env(#env_name)) } else { quote!(::itconfig::get_env_or_panic(#env_name)) }; @@ -170,3 +174,30 @@ impl ToTokens for Variable { } } } + +fn path_ident(path: &Path) -> String { + path.segments + .iter() + .into_iter() + .fold(String::with_capacity(250), |mut acc, v| { + acc.push_str(&v.ident.to_string()); + acc.push('|'); + acc + }) +} + +fn is_option_path_ident(path_ident: String) -> bool { + vec!["Option|", "std|option|Option|", "core|option|Option|"] + .into_iter() + .find(|s| &path_ident == *s) + .is_some() +} + +fn is_option_type(ty: &Type) -> bool { + match ty { + Type::Path(ty_path) => { + ty_path.qself.is_none() && is_option_path_ident(path_ident(&ty_path.path)) + } + _ => false, + } +} diff --git a/itconfig-tests/tests/config_macro.rs b/itconfig-tests/tests/config_macro.rs index b93ae6b..e1ad3ac 100644 --- a/itconfig-tests/tests/config_macro.rs +++ b/itconfig-tests/tests/config_macro.rs @@ -531,3 +531,22 @@ mod test_case_22 { assert_eq!(config::STATIC_CONCAT_VARIABLE(), "static part".to_string()) } } + +mod test_case_23 { + use std::env; + + itconfig::config! { + SOMETHING: Option<&'static str>, + NOTHING: Option<&'static str>, + } + + #[test] + fn optional_variables() { + config::init(); + + env::set_var("SOMETHING", "hello world"); + + assert_eq!(config::SOMETHING(), Some("hello world")); + assert_eq!(config::NOTHING(), None); + } +} diff --git a/itconfig-tests/tests/get_env.rs b/itconfig-tests/tests/get_env.rs deleted file mode 100644 index bf99979..0000000 --- a/itconfig-tests/tests/get_env.rs +++ /dev/null @@ -1,74 +0,0 @@ -use itconfig::EnvError::*; -use itconfig::*; -use std::env; - -#[test] -#[should_panic(expected = "Environment variable \"TEST_CASE_1\" is missing")] -fn get_missing_env() { - get_env_or_panic::("TEST_CASE_1"); -} - -#[test] -#[should_panic(expected = "Failed to parse environment variable \"TEST_CASE_2\"")] -fn get_env_with_invalid_value() { - let env_name = "TEST_CASE_2"; - env::set_var(&env_name, "30r"); - get_env_or_panic::(env_name); -} - -#[test] -fn get_result_of_missing_env() { - let env_name = String::from("TEST_CASE_3"); - let env_val = get_env::(&env_name); - assert_eq!(env_val, Err(MissingVariable(env_name))) -} - -#[test] -fn get_result_of_env_with_invalid_value() { - let env_name = String::from("TEST_CASE_4"); - env::set_var(&env_name, "30r"); - let env_val = get_env::(&env_name); - assert_eq!(env_val, Err(FailedToParse(env_name))) -} - -#[test] -fn get_result_of_env_successfully() { - env::set_var("TEST_CASE_5", "30"); - let env_var = get_env("TEST_CASE_5"); - assert_eq!(env_var, Ok(30)); -} - -#[test] -fn get_missing_env_with_default_value() { - let flag: bool = get_env_or_default("TEST_CASE_6", "true"); - assert_eq!(flag, true); -} - -#[test] -#[should_panic(expected = "Failed to parse environment variable \"TEST_CASE_7\"")] -fn get_invalid_env_with_default_value() { - env::set_var("TEST_CASE_7", "30r"); - get_env_or_default::("TEST_CASE_7", 30); -} - -#[test] -#[should_panic(expected = "Failed to parse environment variable \"TEST_CASE_8\"")] -fn get_env_with_invalid_default_value() { - get_env_or_default::("TEST_CASE_8", "30r"); -} - -#[test] -fn get_env_with_default_successfully() { - env::set_var("TEST_CASE_9", "10"); - let env_val: u32 = get_env_or_default("TEST_CASE_9", 30); - assert_eq!(env_val, 10) -} - -#[test] -fn get_missing_env_with_set_default_value() { - let flag: bool = get_env_or_set_default("TEST_CASE_10", "true"); - assert_eq!(flag, true); - - let env_var = env::var("TEST_CASE_10"); - assert_eq!(env_var, Ok(String::from("true"))) -} diff --git a/itconfig/src/getenv.rs b/itconfig/src/getenv.rs index a3240e5..a396604 100644 --- a/itconfig/src/getenv.rs +++ b/itconfig/src/getenv.rs @@ -1,6 +1,34 @@ use crate::prelude::*; use std::env; +/// Same as get_env but returns Option enum instead Result +/// +/// Example +/// ------- +/// +/// ```rust +/// # extern crate itconfig; +/// # use itconfig::maybe_get_env; +/// use std::env; +/// +/// fn main () { +/// env::set_var("HOST", "https://example.com"); +/// +/// let host: Option<&'static str> = maybe_get_env("HOST"); +/// let not_existence_host: Option<&'static str> = maybe_get_env("NOT_EXISTENCE_HOST"); +/// +/// assert_eq!(host, Some("https://example.com")); +/// assert_eq!(not_existence_host, None); +/// } +/// ``` +/// +pub fn maybe_get_env(env_name: &str) -> Option +where + T: FromEnvString, +{ + get_env(env_name).ok() +} + /// This function is similar as `get_env`, but it unwraps result with panic on error. /// /// Panics @@ -137,3 +165,89 @@ where fn make_panic(e: EnvError) -> T { panic!("{}", e) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + #[should_panic(expected = "Environment variable \"TEST_CASE_1\" is missing")] + fn get_missing_env() { + get_env_or_panic::("TEST_CASE_1"); + } + + #[test] + #[should_panic(expected = "Failed to parse environment variable \"TEST_CASE_2\"")] + fn get_env_with_invalid_value() { + let env_name = "TEST_CASE_2"; + env::set_var(&env_name, "30r"); + get_env_or_panic::(env_name); + } + + #[test] + fn get_result_of_missing_env() { + let env_name = String::from("TEST_CASE_3"); + let env_val = get_env::(&env_name); + assert_eq!(env_val, Err(EnvError::MissingVariable(env_name))) + } + + #[test] + fn get_result_of_env_with_invalid_value() { + let env_name = String::from("TEST_CASE_4"); + env::set_var(&env_name, "30r"); + let env_val = get_env::(&env_name); + assert_eq!(env_val, Err(EnvError::FailedToParse(env_name))) + } + + #[test] + fn get_result_of_env_successfully() { + env::set_var("TEST_CASE_5", "30"); + let env_var = get_env("TEST_CASE_5"); + assert_eq!(env_var, Ok(30)); + } + + #[test] + fn get_missing_env_with_default_value() { + let flag: bool = get_env_or_default("TEST_CASE_6", "true"); + assert_eq!(flag, true); + } + + #[test] + #[should_panic(expected = "Failed to parse environment variable \"TEST_CASE_7\"")] + fn get_invalid_env_with_default_value() { + env::set_var("TEST_CASE_7", "30r"); + get_env_or_default::("TEST_CASE_7", 30); + } + + #[test] + #[should_panic(expected = "Failed to parse environment variable \"TEST_CASE_8\"")] + fn get_env_with_invalid_default_value() { + get_env_or_default::("TEST_CASE_8", "30r"); + } + + #[test] + fn get_env_with_default_successfully() { + env::set_var("TEST_CASE_9", "10"); + let env_val: u32 = get_env_or_default("TEST_CASE_9", 30); + assert_eq!(env_val, 10) + } + + #[test] + fn get_missing_env_with_set_default_value() { + let flag: bool = get_env_or_set_default("TEST_CASE_10", "true"); + assert_eq!(flag, true); + + let env_var = env::var("TEST_CASE_10"); + assert_eq!(env_var, Ok(String::from("true"))) + } + + #[test] + fn get_optional_env() { + env::set_var("TEST_CASE_11", "something"); + let something: Option<&'static str> = maybe_get_env("TEST_CASE_11"); + assert_eq!(something, Some("something")); + + let nothing: Option<&'static str> = maybe_get_env("TEST_CASE_11_NONE"); + assert_eq!(nothing, None); + } +} From 3da357788ba9a1a5d3c3320c92d5ddd6e767f434 Mon Sep 17 00:00:00 2001 From: Dmitriy Pleshevskiy Date: Thu, 15 Apr 2021 23:45:57 +0300 Subject: [PATCH 2/4] refac: move helper functions to utils --- itconfig-macro/src/expand.rs | 37 +----------------------------------- itconfig-macro/src/lib.rs | 2 ++ itconfig-macro/src/utils.rs | 37 ++++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 36 deletions(-) create mode 100644 itconfig-macro/src/utils.rs diff --git a/itconfig-macro/src/expand.rs b/itconfig-macro/src/expand.rs index 99db9dc..50196c3 100644 --- a/itconfig-macro/src/expand.rs +++ b/itconfig-macro/src/expand.rs @@ -1,15 +1,7 @@ use crate::ast::*; +use crate::utils::{is_option_type, vec_to_token_stream_2}; use proc_macro2::TokenStream as TokenStream2; use quote::{quote, ToTokens, TokenStreamExt}; -use syn::Path; -use syn::Type; - -fn vec_to_token_stream_2(input: &Vec) -> Vec -where - T: ToTokens, -{ - input.iter().map(|ns| ns.into_token_stream()).collect() -} impl ToTokens for RootNamespace { fn to_tokens(&self, tokens: &mut TokenStream2) { @@ -174,30 +166,3 @@ impl ToTokens for Variable { } } } - -fn path_ident(path: &Path) -> String { - path.segments - .iter() - .into_iter() - .fold(String::with_capacity(250), |mut acc, v| { - acc.push_str(&v.ident.to_string()); - acc.push('|'); - acc - }) -} - -fn is_option_path_ident(path_ident: String) -> bool { - vec!["Option|", "std|option|Option|", "core|option|Option|"] - .into_iter() - .find(|s| &path_ident == *s) - .is_some() -} - -fn is_option_type(ty: &Type) -> bool { - match ty { - Type::Path(ty_path) => { - ty_path.qself.is_none() && is_option_path_ident(path_ident(&ty_path.path)) - } - _ => false, - } -} diff --git a/itconfig-macro/src/lib.rs b/itconfig-macro/src/lib.rs index 09040da..8c4aa63 100644 --- a/itconfig-macro/src/lib.rs +++ b/itconfig-macro/src/lib.rs @@ -4,9 +4,11 @@ mod ast; mod expand; mod parse; +mod utils; extern crate proc_macro; extern crate proc_macro2; + use self::proc_macro::TokenStream; use ast::RootNamespace; use quote::ToTokens; diff --git a/itconfig-macro/src/utils.rs b/itconfig-macro/src/utils.rs new file mode 100644 index 0000000..f909fbc --- /dev/null +++ b/itconfig-macro/src/utils.rs @@ -0,0 +1,37 @@ +use proc_macro2::TokenStream as TokenStream2; +use quote::ToTokens; +use syn::{Path, Type}; + +pub fn vec_to_token_stream_2(input: &Vec) -> Vec +where + T: ToTokens, +{ + input.iter().map(|ns| ns.into_token_stream()).collect() +} + +fn path_ident(path: &Path) -> String { + path.segments + .iter() + .into_iter() + .fold(String::with_capacity(250), |mut acc, v| { + acc.push_str(&v.ident.to_string()); + acc.push('|'); + acc + }) +} + +fn is_option_path_ident(path_ident: String) -> bool { + vec!["Option|", "std|option|Option|", "core|option|Option|"] + .into_iter() + .find(|s| &path_ident == *s) + .is_some() +} + +pub fn is_option_type(ty: &Type) -> bool { + match ty { + Type::Path(ty_path) => { + ty_path.qself.is_none() && is_option_path_ident(path_ident(&ty_path.path)) + } + _ => false, + } +} From 151e830157cf12ef882ba502e9344350d999ed4d Mon Sep 17 00:00:00 2001 From: Dmitriy Pleshevskiy Date: Thu, 15 Apr 2021 23:46:10 +0300 Subject: [PATCH 3/4] chore: add more tests --- itconfig-tests/tests/config_macro.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/itconfig-tests/tests/config_macro.rs b/itconfig-tests/tests/config_macro.rs index e1ad3ac..8cf4b75 100644 --- a/itconfig-tests/tests/config_macro.rs +++ b/itconfig-tests/tests/config_macro.rs @@ -537,6 +537,11 @@ mod test_case_23 { itconfig::config! { SOMETHING: Option<&'static str>, + #[env_name = "SOMETHING"] + STD_SOMETHING: std::option::Option<&'static str>, + #[env_name = "SOMETHING"] + CORE_SOMETHING: core::option::Option<&'static str>, + NOTHING: Option<&'static str>, } @@ -547,6 +552,8 @@ mod test_case_23 { env::set_var("SOMETHING", "hello world"); assert_eq!(config::SOMETHING(), Some("hello world")); + assert_eq!(config::STD_SOMETHING(), Some("hello world")); + assert_eq!(config::CORE_SOMETHING(), Some("hello world")); assert_eq!(config::NOTHING(), None); } } From 7ed17d8951fe083829f249e9ba9fa389b4e08298 Mon Sep 17 00:00:00 2001 From: Dmitriy Pleshevskiy Date: Thu, 15 Apr 2021 23:54:28 +0300 Subject: [PATCH 4/4] chore: deny and fix all clippy rules --- itconfig-macro/src/expand.rs | 2 +- itconfig-macro/src/lib.rs | 1 + itconfig-macro/src/parse.rs | 21 +++++++++++++-------- itconfig-macro/src/utils.rs | 9 ++++----- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/itconfig-macro/src/expand.rs b/itconfig-macro/src/expand.rs index 50196c3..7008088 100644 --- a/itconfig-macro/src/expand.rs +++ b/itconfig-macro/src/expand.rs @@ -125,7 +125,7 @@ impl ToTokens for Variable { let env_name = &self .env_name .clone() - .unwrap_or(name.to_string().to_uppercase()); + .unwrap_or_else(|| name.to_string().to_uppercase()); let meta = vec_to_token_stream_2(&self.meta); let get_variable: TokenStream2 = if self.concat_parts.is_some() { diff --git a/itconfig-macro/src/lib.rs b/itconfig-macro/src/lib.rs index 8c4aa63..038de09 100644 --- a/itconfig-macro/src/lib.rs +++ b/itconfig-macro/src/lib.rs @@ -1,4 +1,5 @@ #![recursion_limit = "256"] +#![deny(clippy::all)] #![forbid(unsafe_code)] mod ast; diff --git a/itconfig-macro/src/parse.rs b/itconfig-macro/src/parse.rs index 3cbfc17..9a1f923 100644 --- a/itconfig-macro/src/parse.rs +++ b/itconfig-macro/src/parse.rs @@ -117,7 +117,7 @@ impl Parse for RootNamespace { match attr.parse_meta()? { Meta::List(MetaList { nested, .. }) => { let message = - format!("expected #[config(name = \"...\")] or #[config(unwrap)]"); + "expected #[config(name = \"...\")] or #[config(unwrap)]".to_string(); match nested.first().unwrap() { NestedMeta::Meta(Meta::NameValue(MetaNameValue { path, @@ -127,7 +127,7 @@ impl Parse for RootNamespace { if path.is_ident("name") { name = Some(Ident::new(&lit_str.value(), Span::call_site())); } else { - Err(Error::new_spanned(attr, message))?; + return Err(Error::new_spanned(attr, message)); } } NestedMeta::Meta(Meta::Path(path)) => { @@ -135,17 +135,17 @@ impl Parse for RootNamespace { name = None; with_module = false; } else { - Err(Error::new_spanned(attr, message))?; + return Err(Error::new_spanned(attr, message)); } } _ => { - Err(Error::new_spanned(attr, message))?; + return Err(Error::new_spanned(attr, message)); } } } _ => { - let message = format!("expected #[config(...)]"); - Err(Error::new_spanned(attr, message))?; + let message = "expected #[config(...)]".to_string(); + return Err(Error::new_spanned(attr, message)); } } } else { @@ -166,7 +166,7 @@ impl Parse for RootNamespace { let prefix = String::new(); let namespaces = namespaces .into_iter() - .map(fill_env_prefix(prefix.clone())) + .map(fill_env_prefix(prefix)) .collect(); Ok(RootNamespace { @@ -231,7 +231,10 @@ impl Parse for Variable { if content.peek(Ident::peek_any) { let concat_var: Variable = content.parse()?; let name = &concat_var.name; - let env_name = &concat_var.env_name.clone().unwrap_or(name.to_string()); + let env_name = &concat_var + .env_name + .clone() + .unwrap_or_else(|| name.to_string()); let get_variable = if concat_var.initial.is_some() { let initial = concat_var.initial.as_ref().unwrap(); @@ -245,8 +248,10 @@ impl Parse for Variable { let part: Lit = content.parse()?; tmp_vec.push(quote!(#part.to_string())); } + content.parse::().ok(); } + concat_parts = Some(tmp_vec); } else { initial = input diff --git a/itconfig-macro/src/utils.rs b/itconfig-macro/src/utils.rs index f909fbc..3d03212 100644 --- a/itconfig-macro/src/utils.rs +++ b/itconfig-macro/src/utils.rs @@ -2,7 +2,9 @@ use proc_macro2::TokenStream as TokenStream2; use quote::ToTokens; use syn::{Path, Type}; -pub fn vec_to_token_stream_2(input: &Vec) -> Vec +const OPTION_PATH_IDENTS: &[&str] = &["Option|", "std|option|Option|", "core|option|Option|"]; + +pub fn vec_to_token_stream_2(input: &[T]) -> Vec where T: ToTokens, { @@ -21,10 +23,7 @@ fn path_ident(path: &Path) -> String { } fn is_option_path_ident(path_ident: String) -> bool { - vec!["Option|", "std|option|Option|", "core|option|Option|"] - .into_iter() - .find(|s| &path_ident == *s) - .is_some() + OPTION_PATH_IDENTS.iter().any(|s| path_ident == *s) } pub fn is_option_type(ty: &Type) -> bool {