From e15d7829afa488f86af87018e61604ede33bbae3 Mon Sep 17 00:00:00 2001
From: Lars Wirzenius <liw@liw.fi>
Date: Fri, 4 Oct 2019 10:25:02 +0300
Subject: Change: make format_as_dot validate the roadmap

---
 src/lib.rs    |  1 +
 src/map.rs    | 46 ++++++++++++++++++++++++++++++++++++++++------
 src/parser.rs | 40 +++++-----------------------------------
 3 files changed, 46 insertions(+), 41 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index cf5500d..d6745a8 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -39,6 +39,7 @@ pub use step::Step;
 
 mod map;
 pub use map::Roadmap;
+pub use map::RoadmapError;
 
 mod parser;
 pub use parser::from_yaml;
diff --git a/src/map.rs b/src/map.rs
index 4ed4c7f..ab27164 100644
--- a/src/map.rs
+++ b/src/map.rs
@@ -4,6 +4,11 @@ pub use crate::from_yaml;
 pub use crate::Status;
 pub use crate::Step;
 
+
+/// Error in Roadmap, from parsing or otherwise.
+pub type RoadmapError<T> = Result<T, String>;
+
+
 /// Represent a full project roadmap.
 ///
 /// This stores all the steps needed to reach the end goal. See the
@@ -116,10 +121,41 @@ impl Roadmap {
         self.steps.iter().all(|other| !other.depends_on(step.name()))
     }
 
+
+    // Validate that the parsed, constructed roadmap is valid.
+    pub fn validate(&self) -> RoadmapError<()> {
+        // Is there exactly one goal?
+        match self.count_goals() {
+            0 => return Err(format!("the roadmap doesn't have a goal")),
+            1 => (),
+            _ => return Err(format!("must have exactly one goal for roadmap")),
+        }
+
+        // Does every dependency exist?
+        for step in self.iter() {
+            for depname in step.dependencies() {
+                match self.get_step(depname) {
+                    None => {
+                        return Err(format!(
+                            "step {} depends on missing {}",
+                            step.name(),
+                            depname
+                        ))
+                    }
+                    Some(_) => (),
+                }
+            }
+        }
+
+        Ok(())
+    }
+
     /// Get a Graphviz dot language representation of a roadmap. This
     /// is the textual representation, and the caller needs to use the
     /// Graphviz dot(1) tool to create an image from it.
     pub fn format_as_dot(&self, label_width: usize) -> Result<String, Box<dyn std::error::Error>> {
+        self.validate()?;
+
         let labels = self.steps.iter().map(|step| {
             format!(
                 "{} [label=\"{}\" style=filled fillcolor=\"{}\" shape=\"{}\"];\n",
@@ -237,12 +273,10 @@ blocked:
     #[test]
     fn empty_dot() {
         let roadmap = Roadmap::new();
-        assert_eq!(
-            roadmap.format_as_dot(999).unwrap(),
-            "digraph \"roadmap\" {
-}
-"
-        );
+        match roadmap.format_as_dot(999) {
+            Err(_) => (),
+            _ => panic!("expected error for empty roadmap"),
+        }
     }
 
     #[test]
diff --git a/src/parser.rs b/src/parser.rs
index cb1c5ce..60bbf2a 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -4,12 +4,10 @@ use std::collections::HashMap;
 use std::error::Error;
 
 pub use crate::Roadmap;
+pub use crate::RoadmapError;
 pub use crate::Status;
 pub use crate::Step;
 
-/// Result type for roadmap parsing.
-type ParseResult<T> = Result<T, String>;
-
 /// Create a new roadmap from a textual YAML representation.
 pub fn from_yaml(yaml: &str) -> Result<Roadmap, Box<dyn Error>> {
     let mut roadmap = Roadmap::new();
@@ -20,12 +18,12 @@ pub fn from_yaml(yaml: &str) -> Result<Roadmap, Box<dyn Error>> {
         roadmap.add_step(step);
     }
 
-    validate(&roadmap)?;
+    roadmap.validate()?;
     Ok(roadmap)
 }
 
 // Convert a Value into a Step, if possible.
-fn step_from_value(name: &str, value: &Value) -> ParseResult<Step> {
+fn step_from_value(name: &str, value: &Value) -> RoadmapError<Step> {
     match value {
         Value::Mapping(_) => {
             let label = parse_label(&value);
@@ -45,7 +43,7 @@ fn step_from_value(name: &str, value: &Value) -> ParseResult<Step> {
 }
 
 // Get a sequence of depenencies.
-fn parse_depends(map: &Value) -> ParseResult<Vec<&str>> {
+fn parse_depends(map: &Value) -> RoadmapError<Vec<&str>> {
     let key_name = "depends";
     let key = Value::String(key_name.to_string());
     let mut depends: Vec<&str> = vec![];
@@ -73,7 +71,7 @@ fn parse_label(map: &Value) -> &str {
 }
 
 // Get status string from a Mapping element. Default to unknown status.
-fn parse_status<'a>(map: &'a Value) -> ParseResult<Status> {
+fn parse_status<'a>(map: &'a Value) -> RoadmapError<Status> {
     let text = parse_string("status", map);
     match Status::from_text(text) {
         Some(status) => Ok(status),
@@ -91,34 +89,6 @@ fn parse_string<'a>(key_name: &str, map: &'a Value) -> &'a str {
     }
 }
 
-// Validate that the parsed, constructed roadmap is valid.
-fn validate(roadmap: &Roadmap) -> ParseResult<()> {
-    // Is there exactly one goal?
-    match roadmap.count_goals() {
-        0 => return Err(format!("the roadmap doesn't have a goal")),
-        1 => (),
-        _ => return Err(format!("must have exactly one goal for roadmap")),
-    }
-
-    // Does every dependency exist?
-    for step in roadmap.iter() {
-        for depname in step.dependencies() {
-            match roadmap.get_step(depname) {
-                None => {
-                    return Err(format!(
-                        "step {} depends on missing {}",
-                        step.name(),
-                        depname
-                    ))
-                }
-                Some(_) => (),
-            }
-        }
-    }
-
-    Ok(())
-}
-
 #[cfg(test)]
 mod tests {
     use super::from_yaml;
-- 
cgit v1.2.1