Ensure unique directive names when building filters

Use a HashMap to ensure that filter directives have unique names. This
allows later calls to filter::Builder methods to override earlier calls,
as specified in the documentation.

Author: BramBonne

src/filter/mod.rs

@@ -59,6 +59,7 @@
 //! [`Filter::matches`]: struct.Filter.html#method.matches
 
 use log::{Level, LevelFilter, Metadata, Record};
+use std::collections::HashMap;
 use std::env;
 use std::fmt;
 use std::mem;
@@ -107,7 +108,7 @@ pub struct Filter {
 ///
 /// [`Filter`]: struct.Filter.html
 pub struct Builder {
-    directives: Vec<Directive>,
+    directives: HashMap<Option<String>, LevelFilter>,
     filter: Option<inner::Filter>,
     built: bool,
 }
@@ -171,7 +172,7 @@ impl Builder {
     /// Initializes the filter builder with defaults.
     pub fn new() -> Builder {
         Builder {
-            directives: Vec::new(),
+            directives: HashMap::new(),
             filter: None,
             built: false,
         }
@@ -203,10 +204,7 @@ impl Builder {
     /// The given module (if any) will log at most the specified level provided.
     /// If no module is provided then the filter will apply to all log messages.
     pub fn filter(&mut self, module: Option<&str>, level: LevelFilter) -> &mut Self {
-        self.directives.push(Directive {
-            name: module.map(|s| s.to_string()),
-            level,
-        });
+        self.directives.insert(module.map(|s| s.to_string()), level);
         self
     }
 
@@ -221,7 +219,7 @@ impl Builder {
         self.filter = filter;
 
         for directive in directives {
-            self.directives.push(directive);
+            self.directives.insert(directive.name, directive.level);
         }
         self
     }
@@ -231,24 +229,31 @@ impl Builder {
         assert!(!self.built, "attempt to re-use consumed builder");
         self.built = true;
 
+        let mut directives = Vec::new();
         if self.directives.is_empty() {
             // Adds the default filter if none exist
-            self.directives.push(Directive {
+            directives.push(Directive {
                 name: None,
                 level: LevelFilter::Error,
             });
         } else {
+            // Consume map of directives.
+            let directives_map = mem::replace(&mut self.directives, HashMap::new());
+            directives = directives_map
+                .into_iter()
+                .map(|(name, level)| Directive { name, level })
+                .collect();
             // Sort the directives by length of their name, this allows a
             // little more efficient lookup at runtime.
-            self.directives.sort_by(|a, b| {
+            directives.sort_by(|a, b| {
                 let alen = a.name.as_ref().map(|a| a.len()).unwrap_or(0);
                 let blen = b.name.as_ref().map(|b| b.len()).unwrap_or(0);
                 alen.cmp(&blen)
             });
         }
 
         Filter {
-            directives: mem::replace(&mut self.directives, Vec::new()),
+            directives: mem::replace(&mut directives, Vec::new()),
             filter: mem::replace(&mut self.filter, None),
         }
     }

src/lib.rs

@@ -1280,4 +1280,20 @@ mod tests {
 
         assert_eq!(Some("from default".to_owned()), env.get_write_style());
     }
+
+    #[test]
+    fn builder_parse_env_overrides_existing_filters() {
+        env::set_var(
+            "builder_parse_default_env_overrides_existing_filters",
+            "debug",
+        );
+        let env = Env::new().filter("builder_parse_default_env_overrides_existing_filters");
+
+        let mut builder = Builder::new();
+        builder.filter_level(LevelFilter::Trace);
+        // Overrides global level to debug
+        builder.parse_env(env);
+
+        assert_eq!(builder.filter.build().filter(), LevelFilter::Debug);
+    }
 }