From f49a7c9eaa14ce1aaf342d862ec292b84fa2e7bc Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 21 Jan 2022 09:34:47 +0200 Subject: refactor: parse with serde directly to data structure Use serde to parse directly to a hashmap of steps, instead of generic YAML values. This gives us better error messages. Sponsored-by: author --- src/map.rs | 18 +++++++---- src/parser.rs | 101 +++++----------------------------------------------------- src/status.rs | 11 ++++++- src/step.rs | 15 +++++++-- 4 files changed, 42 insertions(+), 103 deletions(-) (limited to 'src') diff --git a/src/map.rs b/src/map.rs index e6a24a9..b592fbd 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use textwrap::fill; pub use crate::from_yaml; @@ -21,8 +23,10 @@ impl Roadmap { /// Create a new, empty roadmap. /// /// You probably want the `from_yaml` function instead. - pub fn new() -> Self { - Self::default() + pub fn new(map: HashMap) -> Self { + Self { + steps: map.values().cloned().collect(), + } } // Find steps that nothing depends on. @@ -220,13 +224,13 @@ mod tests { #[test] fn new_roadmap() { - let roadmap = Roadmap::new(); + let roadmap = Roadmap::default(); assert_eq!(roadmap.step_names().count(), 0); } #[test] fn add_step_to_roadmap() { - let mut roadmap = Roadmap::new(); + let mut roadmap = Roadmap::default(); let first = Step::new("first", "the first step"); roadmap.add_step(first); let names: Vec<&str> = roadmap.step_names().collect(); @@ -235,7 +239,7 @@ mod tests { #[test] fn get_step_from_roadmap() { - let mut roadmap = Roadmap::new(); + let mut roadmap = Roadmap::default(); let first = Step::new("first", "the first step"); roadmap.add_step(first); let gotit = roadmap.get_step("first").unwrap(); @@ -279,7 +283,7 @@ blocked: #[test] fn empty_dot() { - let roadmap = Roadmap::new(); + let roadmap = Roadmap::default(); match roadmap.format_as_dot(999) { Err(_) => (), _ => panic!("expected error for empty roadmap"), @@ -288,7 +292,7 @@ blocked: #[test] fn simple_dot() { - let mut roadmap = Roadmap::new(); + let mut roadmap = Roadmap::default(); let mut first = Step::new("first", ""); first.set_status(Status::Ready); let mut second = Step::new("second", ""); diff --git a/src/parser.rs b/src/parser.rs index 6cc3d25..8c7a9c1 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1,4 +1,3 @@ -use serde_yaml::Value; use std::collections::HashMap; pub use crate::Roadmap; @@ -9,121 +8,37 @@ pub use crate::Step; /// Create a new roadmap from a textual YAML representation. pub fn from_yaml(yaml: &str) -> RoadmapResult { - let mut roadmap = Roadmap::new(); - let map: HashMap = serde_yaml::from_str(yaml)?; - - for (name, value) in map { - let step = step_from_value(&name, &value)?; - roadmap.add_step(step); + let mut roadmap: HashMap = serde_yaml::from_str(yaml)?; + for (name, step) in roadmap.iter_mut() { + step.set_name(name); } - + let roadmap = Roadmap::new(roadmap); roadmap.validate()?; Ok(roadmap) } -// Convert a Value into a Step, if possible. -fn step_from_value(name: &str, value: &Value) -> RoadmapResult { - match value { - Value::Mapping(_) => { - let label = parse_label(value); - let status = parse_status(value)?; - - let mut step = Step::new(name, label); - step.set_status(status); - - for depname in parse_depends(value)?.iter() { - step.add_dependency(depname); - } - - Ok(step) - } - _ => Err(RoadmapError::StepNotMapping), - } -} - -// Get a sequence of depenencies. -fn parse_depends(map: &Value) -> RoadmapResult> { - let key_name = "depends"; - let key = Value::String(key_name.to_string()); - let mut depends: Vec<&str> = vec![]; - - match map.get(&key) { - None => (), - Some(Value::Sequence(deps)) => { - for depname in deps.iter() { - match depname { - Value::String(depname) => depends.push(depname), - _ => return Err(RoadmapError::DependsNotNames), - } - } - } - _ => return Err(RoadmapError::DependsNotNames), - } - - Ok(depends) -} - -// Get label string from a Mapping element, or empty string. -fn parse_label(map: &Value) -> &str { - parse_string("label", map) -} - -// Get status string from a Mapping element. Default to unknown status. -fn parse_status(map: &Value) -> RoadmapResult { - let text = parse_string("status", map); - match Status::from_text(text) { - Some(status) => Ok(status), - _ => Err(RoadmapError::UnknownStatus(text.into())), - } -} - -// Get string value from a Mapping field, or empty string if field -// isn't there. -fn parse_string<'a>(key_name: &str, map: &'a Value) -> &'a str { - let key = Value::String(key_name.to_string()); - match map.get(&key) { - Some(Value::String(s)) => s, - _ => "", - } -} - #[cfg(test)] mod tests { use super::from_yaml; #[test] fn yaml_is_empty() { - if from_yaml("").is_ok() { - panic!("expected a parse error"); - } + assert!(from_yaml("").is_err()); } #[test] fn yaml_is_list() { - if from_yaml("[]").is_ok() { - panic!("expected a parse error"); - } - } - - #[test] - fn yaml_map_entries_not_maps() { - if from_yaml("foo: []").is_ok() { - panic!("expected a parse error"); - } + assert!(from_yaml("[]").is_err()); } #[test] fn yaml_unknown_dep() { - if from_yaml("foo: {depends: [bar]}").is_ok() { - panic!("expected a parse error"); - } + assert!(from_yaml("foo: {depends: [bar]}").is_err()); } #[test] fn yaml_unknown_status() { - if from_yaml(r#"foo: {status: "bar"}"#).is_ok() { - panic!("expected a parse error"); - } + assert!(from_yaml(r#"foo: {status: "bar"}"#).is_err()); } #[test] diff --git a/src/status.rs b/src/status.rs index 9075377..811d1ca 100644 --- a/src/status.rs +++ b/src/status.rs @@ -1,3 +1,5 @@ +use serde::Deserialize; + /// Represent the status of a step in a roadmap. /// /// The unknown status allows the user to not specify the status, and @@ -5,7 +7,8 @@ /// example, a step is inferred to be blocked if any of it /// dependencies are not finished. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize)] +#[serde(rename_all = "lowercase")] pub enum Status { Unknown, Goal, @@ -15,6 +18,12 @@ pub enum Status { Blocked, } +impl Default for Status { + fn default() -> Self { + Self::Unknown + } +} + impl Status { pub fn from_text(text: &str) -> Option { match text { diff --git a/src/step.rs b/src/step.rs index 92b8eec..c37994a 100644 --- a/src/step.rs +++ b/src/step.rs @@ -1,4 +1,5 @@ use super::Status; +use serde::Deserialize; use std::fmt; /// A roadmap step. @@ -6,11 +7,16 @@ use std::fmt; /// See the crate documentation for an example. You /// probably don't want to create steps manually, but via the roadmap /// YAML parsing function. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, Deserialize)] +#[serde(deny_unknown_fields)] pub struct Step { + #[serde(skip)] name: String, - status: Status, + #[serde(default)] label: String, + #[serde(default)] + status: Status, + #[serde(default)] depends: Vec, } @@ -25,6 +31,11 @@ impl Step { } } + /// Set the name of a step. + pub fn set_name(&mut self, name: &str) { + self.name = name.to_string(); + } + /// Return the name of a step. pub fn name(&self) -> &str { &self.name -- cgit v1.2.1