Handle empty git-config file for parser

src/config.rs

@@ -2,14 +2,22 @@ use std::collections::HashMap;
 
 use crate::parser::{parse_from_str, Event, Parser, ParserError};
 
-#[derive(PartialEq, Eq, Hash, Copy, Clone, PartialOrd, Ord)]
+#[derive(PartialEq, Eq, Hash, Copy, Clone, PartialOrd, Ord, Debug)]
 struct SectionId(usize);
 
+#[derive(Debug, PartialEq, Eq)]
 enum LookupTreeNode<'a> {
     Terminal(Vec<SectionId>),
     NonTerminal(HashMap<&'a str, Vec<SectionId>>),
 }
 
+#[derive(Debug, PartialEq, Eq)]
+pub enum GitConfigError<'a> {
+    SectionDoesNotExist(&'a str),
+    SubSectionDoesNotExist(Option<&'a str>),
+    KeyDoesNotExist(&'a str),
+}
+
 /// High level `git-config` reader and writer.
 pub struct GitConfig<'a> {
     front_matter_events: Vec<Event<'a>>,
@@ -27,14 +35,13 @@ impl<'a> GitConfig<'a> {
     }
 
     pub fn from_parser(parser: Parser<'a>) -> Self {
-        // Monotonically increasing
+        let mut new_self = Self {
-        let mut section_id_counter: usize = 0;
+            front_matter_events: vec![],
-
+            sections: HashMap::new(),
-        // Fields for the struct
+            section_lookup_tree: HashMap::new(),
-        let mut front_matter_events: Vec<Event<'a>> = vec![];
+            section_header_separators: HashMap::new(),
-        let mut sections: HashMap<SectionId, Vec<Event<'a>>> = HashMap::new();
+            section_id_counter: 0,
-        let mut section_lookup_tree: HashMap<&str, Vec<LookupTreeNode>> = HashMap::new();
+        };
-        let mut section_header_separators = HashMap::new();
 
         // Current section that we're building
         let mut current_section_name: Option<&str> = None;
@@ -45,49 +52,14 @@ impl<'a> GitConfig<'a> {
             match event {
                 e @ Event::Comment(_) => match maybe_section {
                     Some(ref mut section) => section.push(e),
-                    None => front_matter_events.push(e),
+                    None => new_self.front_matter_events.push(e),
                 },
                 Event::SectionHeader(header) => {
-                    // Push current section to struct
+                    new_self.push_section(
-                    let new_section_id = SectionId(section_id_counter);
+                        &mut current_section_name,
-                    if let Some(section) = maybe_section.take() {
+                        &mut current_subsection_name,
-                        sections.insert(new_section_id, section);
+                        &mut maybe_section,
-                        let lookup = section_lookup_tree
+                    );
-                            .entry(current_section_name.unwrap())
-                            .or_default();
-
-                        let mut found_node = false;
-                        if let Some(subsection_name) = current_subsection_name {
-                            for node in lookup.iter_mut() {
-                                if let LookupTreeNode::NonTerminal(subsection) = node {
-                                    found_node = true;
-                                    subsection
-                                        .entry(subsection_name)
-                                        .or_default()
-                                        .push(new_section_id);
-                                    break;
-                                }
-                            }
-                            if !found_node {
-                                let mut map = HashMap::new();
-                                map.insert(subsection_name, vec![new_section_id]);
-                                lookup.push(LookupTreeNode::NonTerminal(map));
-                            }
-                        } else {
-                            for node in lookup.iter_mut() {
-                                if let LookupTreeNode::Terminal(vec) = node {
-                                    found_node = true;
-                                    vec.push(new_section_id);
-                                    break;
-                                }
-                            }
-                            if !found_node {
-                                lookup.push(LookupTreeNode::Terminal(vec![new_section_id]))
-                            }
-                        }
-
-                        section_id_counter += 1;
-                    }
 
                     // Initialize new section
                     let (name, subname) = (header.name, header.subsection_name);
@@ -97,8 +69,9 @@ impl<'a> GitConfig<'a> {
                     // We need to store the new, current id counter, so don't
                     // use new_section_id here and use the already incremented
                     // section id value.
-                    section_header_separators
+                    new_self
-                        .insert(SectionId(section_id_counter), header.separator);
+                        .section_header_separators
+                        .insert(SectionId(new_self.section_id_counter), header.separator);
                 }
                 e @ Event::Key(_) => maybe_section
                     .as_mut()
@@ -110,7 +83,7 @@ impl<'a> GitConfig<'a> {
                     .push(e),
                 e @ Event::Newline(_) => match maybe_section {
                     Some(ref mut section) => section.push(e),
-                    None => front_matter_events.push(e),
+                    None => new_self.front_matter_events.push(e),
                 },
                 e @ Event::ValueNotDone(_) => maybe_section
                     .as_mut()
@@ -122,20 +95,93 @@ impl<'a> GitConfig<'a> {
                     .push(e),
                 e @ Event::Whitespace(_) => match maybe_section {
                     Some(ref mut section) => section.push(e),
-                    None => front_matter_events.push(e),
+                    None => new_self.front_matter_events.push(e),
                 },
             }
         }
 
-        Self {
+        new_self.push_section(
-            front_matter_events,
+            &mut current_section_name,
-            section_lookup_tree,
+            &mut current_subsection_name,
-            sections,
+            &mut maybe_section,
-            section_header_separators,
+        );
-            section_id_counter,
+
+        new_self
+    }
+
+    fn push_section(
+        &mut self,
+        current_section_name: &mut Option<&'a str>,
+        current_subsection_name: &mut Option<&'a str>,
+        maybe_section: &mut Option<Vec<Event<'a>>>,
+    ) {
+        let new_section_id = SectionId(self.section_id_counter);
+        if let Some(section) = maybe_section.take() {
+            self.sections.insert(new_section_id, section);
+            let lookup = self
+                .section_lookup_tree
+                .entry(current_section_name.unwrap())
+                .or_default();
+
+            let mut found_node = false;
+            if let Some(subsection_name) = current_subsection_name {
+                for node in lookup.iter_mut() {
+                    if let LookupTreeNode::NonTerminal(subsection) = node {
+                        found_node = true;
+                        subsection
+                            .entry(subsection_name)
+                            .or_default()
+                            .push(new_section_id);
+                        break;
+                    }
+                }
+                if !found_node {
+                    let mut map = HashMap::new();
+                    map.insert(*subsection_name, vec![new_section_id]);
+                    lookup.push(LookupTreeNode::NonTerminal(map));
+                }
+            } else {
+                for node in lookup.iter_mut() {
+                    if let LookupTreeNode::Terminal(vec) = node {
+                        found_node = true;
+                        vec.push(new_section_id);
+                        break;
+                    }
+                }
+                if !found_node {
+                    lookup.push(LookupTreeNode::Terminal(vec![new_section_id]))
+                }
+            }
+            self.section_id_counter += 1;
         }
     }
 
+    /// Returns an uninterpreted value given a section and optional subsection
+    /// and key.
+    ///
+    /// Note that `git-config` follows a "last-one-wins" rule for single values.
+    /// If multiple sections contain the same key, then the last section's last
+    /// key's value will be returned.
+    ///
+    /// Concretely, if you have the following config:
+    ///
+    /// ```text
+    /// [core]
+    ///     a = b
+    /// [core]
+    ///     a = c
+    ///     a = d
+    /// ```
+    ///
+    /// Then this function will return `d`:
+    ///
+    /// ```
+    /// # use serde_git_config::config::GitConfig;
+    /// # let git_config = GitConfig::from_str("[core]a=b\n[core]\na=c\na=d").unwrap();
+    /// assert_eq!(git_config.get_raw_single_value("core", None, "a"), Ok("d"));
+    /// ```
+    ///
+    /// The the resolution is as follows
     pub fn get_raw_single_value<'b>(
         &self,
         section_name: &'b str,
@@ -153,15 +199,20 @@ impl<'a> GitConfig<'a> {
         // violated invariant.
         let events = self.sections.get(&section_id).unwrap();
         let mut found_key = false;
+        let mut latest_value = None;
+        // logic needs fixing for last one wins rule
         for event in events {
             match event {
                 Event::Key(event_key) if *event_key == key => found_key = true,
-                Event::Value(v) if found_key => return Ok(v),
+                Event::Value(v) if found_key => {
+                    found_key = false;
+                    latest_value = Some(*v);
+                }
                 _ => (),
             }
         }
 
-        Err(GitConfigError::KeyDoesNotExist(key))
+        latest_value.ok_or(GitConfigError::KeyDoesNotExist(key))
     }
 
     fn get_section_id_by_name_and_subname<'b>(
@@ -236,8 +287,61 @@ impl<'a> GitConfig<'a> {
     }
 }
 
-pub enum GitConfigError<'a> {
+#[cfg(test)]
-    SectionDoesNotExist(&'a str),
+mod git_config {
-    SubSectionDoesNotExist(Option<&'a str>),
+    mod from_parser {
-    KeyDoesNotExist(&'a str),
+        use super::super::*;
+
+        #[test]
+        fn parse_empty() {
+            let config = GitConfig::from_str("").unwrap();
+            assert!(config.section_header_separators.is_empty());
+            assert_eq!(config.section_id_counter, 0);
+            assert!(config.section_lookup_tree.is_empty());
+            assert!(config.sections.is_empty());
+        }
+
+        #[test]
+        fn parse_single_section() {
+            let config = GitConfig::from_str("[core]\na=b\nc=d").unwrap();
+            let expected_separators = {
+                let mut map = HashMap::new();
+                map.insert(SectionId(0), None);
+                map
+            };
+            assert_eq!(config.section_header_separators, expected_separators);
+            assert_eq!(config.section_id_counter, 1);
+            let expected_lookup_tree = {
+                let mut tree = HashMap::new();
+                tree.insert("core", vec![LookupTreeNode::Terminal(vec![SectionId(0)])]);
+                tree
+            };
+            assert_eq!(config.section_lookup_tree, expected_lookup_tree);
+            let expected_sections = {
+                let mut sections = HashMap::new();
+                sections.insert(
+                    SectionId(0),
+                    vec![
+                        Event::Newline("\n"),
+                        Event::Key("a"),
+                        Event::Value("b"),
+                        Event::Newline("\n"),
+                        Event::Key("c"),
+                        Event::Value("d"),
+                    ],
+                );
+                sections
+            };
+            assert_eq!(config.sections, expected_sections);
+        }
+
+        #[test]
+        fn parse_single_subsection() {}
+
+        #[test]
+        fn parse_multiple_sections() {}
+
+        #[test]
+        fn parse_multiple_duplicate_sections() {}
+    }
 }

src/parser.rs

@@ -12,7 +12,6 @@ use nom::character::complete::{char, none_of, one_of};
 use nom::character::{is_newline, is_space};
 use nom::combinator::{map, opt};
 use nom::error::{Error as NomError, ErrorKind};
-use nom::multi::many1;
 use nom::sequence::delimited;
 use nom::IResult;
 use nom::{branch::alt, multi::many0};
@@ -360,7 +359,7 @@ impl<'a> Parser<'a> {
 /// data succeeding valid `git-config` data.
 pub fn parse_from_str(input: &str) -> Result<Parser<'_>, ParserError> {
     let (i, comments) = many0(comment)(input)?;
-    let (i, sections) = many1(section)(i)?;
+    let (i, sections) = many0(section)(i)?;
 
     if !i.is_empty() {
         return Err(ParserError::ConfigHasExtraData(i));

tests/parser_integration_tests.rs

@@ -62,8 +62,7 @@ fn personal_config() {
     assert_eq!(
         parse_from_str(config)
             .unwrap()
-            .into_iter()
+            .into_vec(),
-            .collect::<Vec<_>>(),
         vec![
             gen_section_header("user", None),
             newline(),
@@ -163,3 +162,8 @@ fn personal_config() {
         ]
     );
 }
+
+#[test]
+fn parse_empty() {
+    assert_eq!(parse_from_str("").unwrap().into_vec(), vec![]);
+}