summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLars Wirzenius <liw@liw.fi>2022-01-21 09:34:47 +0200
committerLars Wirzenius <liw@liw.fi>2022-01-21 10:09:41 +0200
commitf49a7c9eaa14ce1aaf342d862ec292b84fa2e7bc (patch)
treee81eb68c3626715c541644e9055c2bd8c709fee9
parent82073da80b0f38d605204e96bf4b430c2b785813 (diff)
downloadroadmap-f49a7c9eaa14ce1aaf342d862ec292b84fa2e7bc.tar.gz
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
-rw-r--r--Cargo.lock15
-rw-r--r--Cargo.toml1
-rw-r--r--src/map.rs18
-rw-r--r--src/parser.rs101
-rw-r--r--src/status.rs11
-rw-r--r--src/step.rs15
6 files changed, 58 insertions, 103 deletions
diff --git a/Cargo.lock b/Cargo.lock
index 87e468d..be68abe 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -186,6 +186,7 @@ name = "roadmap"
version = "0.3.0"
dependencies = [
"anyhow",
+ "serde",
"serde_yaml",
"structopt",
"textwrap 0.14.2",
@@ -203,6 +204,20 @@ name = "serde"
version = "1.0.134"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "96b3c34c1690edf8174f5b289a336ab03f568a4460d8c6df75f2f3a692b3bc6a"
+dependencies = [
+ "serde_derive",
+]
+
+[[package]]
+name = "serde_derive"
+version = "1.0.134"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "784ed1fbfa13fe191077537b0d70ec8ad1e903cfe04831da608aa36457cb653d"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
[[package]]
name = "serde_yaml"
diff --git a/Cargo.toml b/Cargo.toml
index a5f1129..40ef824 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -11,6 +11,7 @@ repostiory = "https://gitlab.com/larswirzenius/roadmap"
[dependencies]
anyhow = "1"
+serde = { version = "1.0.134", features = ["derive"] }
serde_yaml = "0.8"
structopt = "0.3"
textwrap = "0.14"
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<String, Step>) -> 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<Roadmap> {
- let mut roadmap = Roadmap::new();
- let map: HashMap<String, serde_yaml::Value> = 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<String, Step> = 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<Step> {
- 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<Vec<&str>> {
- 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<Status> {
- 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<Status> {
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<String>,
}
@@ -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