From a2d2026f6e152b5d481864e2a68d64c52f8f1cdc Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sat, 4 Nov 2023 01:20:08 -0700 Subject: [PATCH 1/2] Expose parse errors via `Dotenv::iter`. Refactor `dotenv::dotenv::Iter` to expose parse errors for those interested. The parse errors are structured as opaque strings to hide the parsing implementation details (currently nom and nom's errors are the internal currency). This allows a consumer that cares about propagating errors in `.env` files to use something like: ``` let env = dotenv::from_filename(".env")?; let mut iter = env.iter(); while let Some((key, value)) = iter.try_next()? { if std::env::var(key).is_err() { std::env::set_var(key, value); } } ``` Fixes #4 --- src/dotenv.rs | 36 +++++++++++++++------------ src/errors.rs | 10 ++++++++ tests/fixtures/sample-basic.env | 1 - tests/test-dotenv-try-next.rs | 19 ++++++++++++++ tests/test-sample-bad.rs | 44 +++++++++++++++++++++++++++++++++ tests/test-sample-basic.rs | 7 +++--- tests/test-sample-expand.rs | 7 +++--- tests/test-sample-multiline.rs | 7 +++--- 8 files changed, 105 insertions(+), 26 deletions(-) create mode 100644 tests/test-dotenv-try-next.rs create mode 100644 tests/test-sample-bad.rs diff --git a/src/dotenv.rs b/src/dotenv.rs index 675c2db..27fb4b4 100644 --- a/src/dotenv.rs +++ b/src/dotenv.rs @@ -59,28 +59,32 @@ impl<'a> Iter<'a> { Value::List(list) => Some(list.into_iter().flat_map(|it| self.resolve(it)).collect()), } } + + pub fn try_next(&mut self) -> crate::Result> { + while !self.input.is_empty() { + match parse(self.input) { + Ok((rest, maybe)) => { + self.input = rest; // set next input + + if let Some((key, value)) = maybe { + if let Some(value) = self.resolve(value) { + self.resolved.insert(key, value.clone()); + return Ok(Some((key, value))); + } + } + } + Err(err) => return Err(crate::Error::Parse(format!("{err}"))), + } + } + Ok(None) + } } impl<'a> Iterator for Iter<'a> { type Item = (&'a str, String); fn next(&mut self) -> Option { - while let Ok((rest, maybe)) = parse(self.input) { - self.input = rest; // set next input - - if let Some((key, value)) = maybe { - if let Some(value) = self.resolve(value) { - self.resolved.insert(key, value.clone()); - return Some((key, value)); - } - } - - if rest.is_empty() { - break; - } - } - - None + self.try_next().unwrap_or_default() } } diff --git a/src/errors.rs b/src/errors.rs index 16a6f3d..c257ce6 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,6 +1,7 @@ use std::env; use std::error; use std::fmt; +use std::fmt::Display; use std::io; #[derive(Debug)] @@ -8,6 +9,7 @@ use std::io; pub enum Error { Io(io::Error), Env(env::VarError), + Parse(String), } impl Error { @@ -29,6 +31,7 @@ impl fmt::Display for Error { match self { Error::Io(err) => err.fmt(fmt), Error::Env(err) => err.fmt(fmt), + Error::Parse(err) => err.fmt(fmt), } } } @@ -38,6 +41,7 @@ impl error::Error for Error { match self { Error::Io(err) => Some(err), Error::Env(err) => Some(err), + Error::Parse(_) => None, } } } @@ -53,3 +57,9 @@ impl From for Error { Error::Env(err) } } + +impl From> for Error { + fn from(err: nom::error::Error) -> Self { + Error::Parse(format!("{err}")) + } +} diff --git a/tests/fixtures/sample-basic.env b/tests/fixtures/sample-basic.env index 243b3f4..91a5638 100644 --- a/tests/fixtures/sample-basic.env +++ b/tests/fixtures/sample-basic.env @@ -24,7 +24,6 @@ DOUBLE_AND_SINGLE_QUOTES_INSIDE_BACKTICKS=`double "quotes" and single 'quotes' w EXPAND_NEWLINES="expand\nnew\nlines" DONT_EXPAND_UNQUOTED=dontexpand\nnewlines DONT_EXPAND_SQUOTED='dontexpand\nnewlines' -DONT_EXPAND_SQUOTED='dontexpand\nnewlines' # COMMENTS=work INLINE_COMMENTS=inline comments # work #very #well INLINE_COMMENTS_SINGLE_QUOTES='inline comments outside of #singlequotes' # work diff --git a/tests/test-dotenv-try-next.rs b/tests/test-dotenv-try-next.rs new file mode 100644 index 0000000..5a99a85 --- /dev/null +++ b/tests/test-dotenv-try-next.rs @@ -0,0 +1,19 @@ +mod fixtures; +use fixtures::*; + +#[test] +fn test_propagate_env_parse_errors() -> anyhow::Result<()> { + let (_t, mut exps) = with_basic_dotenv()?; + + // This is an example of how a consumer that cares about invalid `.env` files can handle them using the + // `Iter::try_next` API (see: https://github.com/arniu/dotenvs-rs/issues/4) + let env = dotenv::from_filename(".env")?; + let mut iter = env.iter(); + while let Some((key, value)) = iter.try_next()? { + let expected = exps.remove(key).unwrap(); + assert_eq!(expected, value, "check {}", key); + } + assert!(exps.is_empty()); + + Ok(()) +} diff --git a/tests/test-sample-bad.rs b/tests/test-sample-bad.rs new file mode 100644 index 0000000..7eec8e3 --- /dev/null +++ b/tests/test-sample-bad.rs @@ -0,0 +1,44 @@ +use dotenv::Error; +use std::collections::HashMap; +use std::iter::{IntoIterator, Iterator}; + +const BAD_ENV: &str = r#" +A=foo bar +B="notenough +C='toomany'' +D=valid +export NOT_SET +E=valid +"#; + +#[test] +fn test_bad_env() -> anyhow::Result<()> { + let env = dotenv::from_read(BAD_ENV.as_bytes())?; + + assert_eq!( + vec![ + ("A", "foo bar".into()), + ("B", "\"notenough".into()), + ("C", "toomany".into()) + ] + .into_iter() + .collect::>(), + env.iter().collect::>() + ); + + let mut iter = env.iter(); + assert_eq!(Some(("A", "foo bar".into())), iter.try_next()?); + assert_eq!(Some(("B", "\"notenough".into())), iter.try_next()?); + assert_eq!(Some(("C", "toomany".into())), iter.try_next()?); + + // TODO: Use assert_matches! when it stabilizes: https://github.com/rust-lang/rust/issues/82775 + match iter.try_next().unwrap_err() { + Error::Parse(err) => assert_eq!( + "Parsing Error: Error { input: \"'\\nD=valid\\nexport NOT_SET\\nE=valid\\n\", code: Tag }", + err + ), + err => panic!("Unexpected error variant: {err:?}", err = err), + } + + Ok(()) +} diff --git a/tests/test-sample-basic.rs b/tests/test-sample-basic.rs index 3aae913..f29a833 100644 --- a/tests/test-sample-basic.rs +++ b/tests/test-sample-basic.rs @@ -3,11 +3,12 @@ use fixtures::*; #[test] fn test_sample() -> anyhow::Result<()> { - let (_t, exps) = with_basic_dotenv()?; + let (_t, mut exps) = with_basic_dotenv()?; for (key, value) in dotenv::from_filename(".env")?.iter() { - let expected = exps.get(key).unwrap(); - assert_eq!(expected, &value, "check {}", key); + let expected = exps.remove(key).unwrap(); + assert_eq!(expected, value, "check {}", key); } + assert!(exps.is_empty()); Ok(()) } diff --git a/tests/test-sample-expand.rs b/tests/test-sample-expand.rs index c206fca..9d8c9d4 100644 --- a/tests/test-sample-expand.rs +++ b/tests/test-sample-expand.rs @@ -3,11 +3,12 @@ use fixtures::*; #[test] fn test_sample() -> anyhow::Result<()> { - let (_t, exps) = with_expand_dotenv()?; + let (_t, mut exps) = with_expand_dotenv()?; for (key, value) in dotenv::from_filename(".env")?.iter() { - let expected = exps.get(key).unwrap(); - assert_eq!(expected, &value, "check {}", key); + let expected = exps.remove(key).unwrap(); + assert_eq!(expected, value, "check {}", key); } + assert!(exps.is_empty()); Ok(()) } diff --git a/tests/test-sample-multiline.rs b/tests/test-sample-multiline.rs index 1dccf32..9dfee6d 100644 --- a/tests/test-sample-multiline.rs +++ b/tests/test-sample-multiline.rs @@ -3,11 +3,12 @@ use fixtures::*; #[test] fn test_sample() -> anyhow::Result<()> { - let (_t, exps) = with_multiline_dotenv()?; + let (_t, mut exps) = with_multiline_dotenv()?; for (key, value) in dotenv::from_filename(".env")?.iter() { - let expected = exps.get(key).unwrap(); - assert_eq!(expected, &value, "check {}", key); + let expected = exps.remove(key).unwrap(); + assert_eq!(expected, value, "check {}", key); } + assert!(exps.is_empty()); Ok(()) } From b2276ef3fd039ed8565b4c1cbedb7a5aeeca734e Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sun, 5 Nov 2023 04:09:57 -0800 Subject: [PATCH 2/2] Cleanups. --- src/dotenv.rs | 2 +- src/errors.rs | 11 +++++------ tests/test-sample-bad.rs | 19 ++++++------------- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/src/dotenv.rs b/src/dotenv.rs index 27fb4b4..2d8a862 100644 --- a/src/dotenv.rs +++ b/src/dotenv.rs @@ -73,7 +73,7 @@ impl<'a> Iter<'a> { } } } - Err(err) => return Err(crate::Error::Parse(format!("{err}"))), + Err(err) => return Err(err.into()), } } Ok(None) diff --git a/src/errors.rs b/src/errors.rs index c257ce6..c6cd69b 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,7 +1,6 @@ use std::env; use std::error; use std::fmt; -use std::fmt::Display; use std::io; #[derive(Debug)] @@ -29,9 +28,9 @@ impl Error { impl fmt::Display for Error { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { match self { - Error::Io(err) => err.fmt(fmt), - Error::Env(err) => err.fmt(fmt), - Error::Parse(err) => err.fmt(fmt), + Error::Io(err) => fmt::Display::fmt(&err, fmt), + Error::Env(err) => fmt::Display::fmt(&err, fmt), + Error::Parse(err) => fmt::Display::fmt(&err, fmt), } } } @@ -58,8 +57,8 @@ impl From for Error { } } -impl From> for Error { - fn from(err: nom::error::Error) -> Self { +impl From> for Error { + fn from(err: nom::Err) -> Self { Error::Parse(format!("{err}")) } } diff --git a/tests/test-sample-bad.rs b/tests/test-sample-bad.rs index 7eec8e3..d286748 100644 --- a/tests/test-sample-bad.rs +++ b/tests/test-sample-bad.rs @@ -1,6 +1,4 @@ use dotenv::Error; -use std::collections::HashMap; -use std::iter::{IntoIterator, Iterator}; const BAD_ENV: &str = r#" A=foo bar @@ -20,10 +18,8 @@ fn test_bad_env() -> anyhow::Result<()> { ("A", "foo bar".into()), ("B", "\"notenough".into()), ("C", "toomany".into()) - ] - .into_iter() - .collect::>(), - env.iter().collect::>() + ], + env.iter().collect::>() ); let mut iter = env.iter(); @@ -32,13 +28,10 @@ fn test_bad_env() -> anyhow::Result<()> { assert_eq!(Some(("C", "toomany".into())), iter.try_next()?); // TODO: Use assert_matches! when it stabilizes: https://github.com/rust-lang/rust/issues/82775 - match iter.try_next().unwrap_err() { - Error::Parse(err) => assert_eq!( - "Parsing Error: Error { input: \"'\\nD=valid\\nexport NOT_SET\\nE=valid\\n\", code: Tag }", - err - ), - err => panic!("Unexpected error variant: {err:?}", err = err), - } + assert!(matches!( + iter.try_next().unwrap_err(), + Error::Parse(err) if err == "Parsing Error: Error { input: \"'\\nD=valid\\nexport NOT_SET\\nE=valid\\n\", code: Tag }" + )); Ok(()) }