diff --git a/crates/env_filter/CHANGELOG.md b/crates/env_filter/CHANGELOG.md index b9a276c..b8c47e8 100644 --- a/crates/env_filter/CHANGELOG.md +++ b/crates/env_filter/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] - ReleaseDate +### Features + +- Added `env_filter::Builder::try_parse(&self, &str)` method (failable version of `env_filter::Builder::parse()`) + ## [0.1.0] - 2024-01-19 diff --git a/crates/env_filter/src/filter.rs b/crates/env_filter/src/filter.rs index 5ae018d..de59e9b 100644 --- a/crates/env_filter/src/filter.rs +++ b/crates/env_filter/src/filter.rs @@ -6,6 +6,7 @@ use log::{LevelFilter, Metadata, Record}; use crate::enabled; use crate::parse_spec; +use crate::parser::{try_parse_spec, ParsingError}; use crate::Directive; use crate::FilterOp; @@ -107,6 +108,22 @@ impl Builder { self } + /// Parses the directive string, returning an error if the given directive string is invalid. + /// + /// See the [Enabling Logging] section for more details. + /// + /// [Enabling Logging]: ../index.html#enabling-logging + pub fn try_parse(&mut self, filters: &str) -> Result<&mut Self, ParsingError> { + let (directives, filter) = try_parse_spec(filters, false)?; + + self.filter = filter; + + for directive in directives { + self.insert_directive(directive); + } + Ok(self) + } + /// Build a log filter. pub fn build(&mut self) -> Filter { assert!(!self.built, "attempt to re-use consumed builder"); diff --git a/crates/env_filter/src/lib.rs b/crates/env_filter/src/lib.rs index 6a72204..83b4d54 100644 --- a/crates/env_filter/src/lib.rs +++ b/crates/env_filter/src/lib.rs @@ -56,3 +56,4 @@ use parser::parse_spec; pub use filter::Builder; pub use filter::Filter; pub use filtered_log::FilteredLog; +pub use parser::ParsingError; diff --git a/crates/env_filter/src/parser.rs b/crates/env_filter/src/parser.rs index 3cb01be..470e2a4 100644 --- a/crates/env_filter/src/parser.rs +++ b/crates/env_filter/src/parser.rs @@ -1,25 +1,60 @@ use log::LevelFilter; +use std::error::Error; +use std::fmt::{Debug, Display, Formatter}; use crate::Directive; use crate::FilterOp; +/// Error during logger directive parsing process. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct ParsingError { + /// Details of what exactly was wrong in the directive. + pub details: String, +} + +impl Display for ParsingError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "error parsing logger filter: {}", self.details) + } +} + +impl Error for ParsingError {} + /// Parse a logging specification string (e.g: `crate1,crate2::mod3,crate3::x=error/foo`) /// and return a vector with log directives. pub(crate) fn parse_spec(spec: &str) -> (Vec, Option) { - #![allow(clippy::print_stderr)] // compatibility + try_parse_spec(spec, true).unwrap_or_else(|_| { + // This never happens, but better have this branch than add .expect() + (vec![], None) + }) +} +/// Parse a logging specification string and return a vector with log directives. +/// +/// If `skip_errors` argument is set to true, the errors will be printed out into the `stderr` and +/// ignored, returning as the result correct directives from the given string. +/// +/// If `skip_errors` is false, the first encountered error in an invalid specification string will +/// be returned. +pub(crate) fn try_parse_spec( + spec: &str, + skip_errors: bool, +) -> Result<(Vec, Option), ParsingError> { let mut dirs = Vec::new(); let mut parts = spec.split('/'); let mods = parts.next(); let filter = parts.next(); if parts.next().is_some() { - eprintln!( - "warning: invalid logging spec '{}', \ + err_or_print( + skip_errors, + format!( + "warning: invalid logging spec '{}', \ ignoring it (too many '/'s)", - spec - ); - return (dirs, None); + spec + ), + )?; + return Ok((dirs, None)); } if let Some(m) = mods { for s in m.split(',').map(|ss| ss.trim()) { @@ -42,20 +77,26 @@ pub(crate) fn parse_spec(spec: &str) -> (Vec, Option) { if let Ok(num) = part1.parse() { (num, Some(part0)) } else { - eprintln!( - "warning: invalid logging spec '{}', \ + err_or_print( + skip_errors, + format!( + "warning: invalid logging spec '{}', \ ignoring it", - part1 - ); + part1 + ), + )?; continue; } } _ => { - eprintln!( - "warning: invalid logging spec '{}', \ + err_or_print( + skip_errors, + format!( + "warning: invalid logging spec '{}', \ ignoring it", - s - ); + s + ), + )?; continue; } }; @@ -66,22 +107,39 @@ pub(crate) fn parse_spec(spec: &str) -> (Vec, Option) { } } - let filter = filter.and_then(|filter| match FilterOp::new(filter) { - Ok(re) => Some(re), - Err(e) => { - eprintln!("warning: invalid regex filter - {}", e); - None - } - }); + let filter = filter + .map(|filter| match FilterOp::new(filter) { + Ok(re) => Ok(Some(re)), + Err(e) => err_or_print( + skip_errors, + format!("warning: invalid regex filter - {}", e), + ) + .map(|_| None), + }) + .transpose()? + .flatten(); + + Ok((dirs, filter)) +} + +fn err_or_print(skip_errors: bool, error_message: String) -> Result<(), ParsingError> { + #![allow(clippy::print_stderr)] // compatibility - (dirs, filter) + if skip_errors { + eprintln!("{error_message}"); + Ok(()) + } else { + Err(ParsingError { + details: error_message, + }) + } } #[cfg(test)] mod tests { use log::LevelFilter; - use super::parse_spec; + use super::{parse_spec, try_parse_spec}; #[test] fn parse_spec_valid() { @@ -108,6 +166,11 @@ mod tests { assert!(filter.is_none()); } + #[test] + fn try_parse_spec_invalid_crate() { + assert!(try_parse_spec("crate1::mod1=warn=info,crate2=debug", false).is_err()); + } + #[test] fn parse_spec_invalid_level() { // test parse_spec with 'noNumber' as log level @@ -118,6 +181,11 @@ mod tests { assert!(filter.is_none()); } + #[test] + fn try_parse_spec_invalid_level() { + assert!(try_parse_spec("crate1::mod1=noNumber,crate2=debug", false).is_err()); + } + #[test] fn parse_spec_string_level() { // test parse_spec with 'warn' as log level @@ -128,6 +196,11 @@ mod tests { assert!(filter.is_none()); } + #[test] + fn try_parse_spec_string_level() { + assert!(try_parse_spec("crate1::mod1=wrong,crate2=warn", false).is_err()); + } + #[test] fn parse_spec_empty_level() { // test parse_spec with '' as log level @@ -138,6 +211,11 @@ mod tests { assert!(filter.is_none()); } + #[test] + fn try_parse_spec_empty_level() { + assert!(try_parse_spec("crate1::mod1=wrong,crate2=", false).is_err()); + } + #[test] fn parse_spec_empty_level_isolated() { // test parse_spec with "" as log level (and the entire spec str) @@ -146,6 +224,13 @@ mod tests { assert!(filter.is_none()); } + #[test] + fn try_parse_spec_empty_level_isolated() { + let (dirs, filter) = try_parse_spec("", false).expect("parsing empty level returned Err"); + assert_eq!(dirs.len(), 0); + assert!(filter.is_none()); + } + #[test] fn parse_spec_blank_level_isolated() { // test parse_spec with a white-space-only string specified as the log @@ -155,6 +240,14 @@ mod tests { assert!(filter.is_none()); } + #[test] + fn try_parse_spec_blank_level_isolated() { + let (dirs, filter) = + try_parse_spec(" ", false).expect("parsing blank level returned Err"); + assert_eq!(dirs.len(), 0); + assert!(filter.is_none()); + } + #[test] fn parse_spec_blank_level_isolated_comma_only() { // The spec should contain zero or more comma-separated string slices, @@ -165,6 +258,14 @@ mod tests { assert!(filter.is_none()); } + #[test] + fn try_parse_spec_blank_level_isolated_comma_only() { + let (dirs, filter) = + try_parse_spec(",", false).expect("parsing isolated comma returned Err"); + assert_eq!(dirs.len(), 0); + assert!(filter.is_none()); + } + #[test] fn parse_spec_blank_level_isolated_comma_blank() { // The spec should contain zero or more comma-separated string slices, @@ -176,6 +277,14 @@ mod tests { assert!(filter.is_none()); } + #[test] + fn try_parse_spec_blank_level_isolated_comma_blank() { + let (dirs, filter) = try_parse_spec(", ", false) + .expect("parsing isolated comma with blank returned Err"); + assert_eq!(dirs.len(), 0); + assert!(filter.is_none()); + } + #[test] fn parse_spec_blank_level_isolated_blank_comma() { // The spec should contain zero or more comma-separated string slices, @@ -187,6 +296,14 @@ mod tests { assert!(filter.is_none()); } + #[test] + fn try_parse_spec_blank_level_isolated_blank_comma() { + let (dirs, filter) = try_parse_spec(" ,", false) + .expect("parsing blank with isolated comma returned Err"); + assert_eq!(dirs.len(), 0); + assert!(filter.is_none()); + } + #[test] fn parse_spec_global() { // test parse_spec with no crate @@ -261,4 +378,17 @@ mod tests { assert_eq!(dirs[0].level, LevelFilter::max()); assert!(filter.is_some() && filter.unwrap().to_string() == "a*c"); } + + #[test] + fn parse_spec_with_multiple_filters() { + // Having multiple / is invalid format and should be ignored + let (dirs, filter) = parse_spec("crate1/a.c/a*c"); + assert_eq!(dirs.len(), 0); + assert!(filter.is_none()); + } + + #[test] + fn try_parse_with_multiple_filters() { + assert!(try_parse_spec("crate1/a.c/a*c", false).is_err()); + } }