diff --git a/osm2streets/src/lanes/placement.rs b/osm2streets/src/lanes/placement.rs index f67355f6..61d322ba 100644 --- a/osm2streets/src/lanes/placement.rs +++ b/osm2streets/src/lanes/placement.rs @@ -1,18 +1,17 @@ use abstutil::Tags; +use anyhow::Result; use super::{LtrLaneNum, Placement, RoadPosition}; use Placement::*; use RoadPosition::*; -impl TryFrom<&str> for RoadPosition { - type Error = (); - +impl RoadPosition { /// Tries to parse a road position from an osm tag value as per the `placement` scheme. /// See https://wiki.openstreetmap.org/wiki/Proposed_features/placement#Tagging /// /// The direction is treated as forward, use `reverse()` on the result if the context is backward. - fn try_from(value: &str) -> Result { + pub fn parse(value: &str) -> Result { match value { "" => Ok(Center), "separation" => Ok(Separation), @@ -23,66 +22,60 @@ impl TryFrom<&str> for RoadPosition { "left_of" => Ok(LeftOf(LtrLaneNum::Forward(lane))), "middle_of" => Ok(MiddleOf(LtrLaneNum::Forward(lane))), "right_of" => Ok(RightOf(LtrLaneNum::Forward(lane))), - _ => Err(()), + _ => Err(anyhow!("unknown lane position specifier: {kind}")), } } else { - Err(()) + Err(anyhow!("bad lane number: {lane_str}")) } } else { - Err(()) + Err(anyhow!("unknown placement value: {value}")) } } } } } -impl TryFrom<&Tags> for Placement { - type Error = (); - +impl Placement { /// Tries to parse a placement from a set of OSM tags according to the `placement` scheme. /// See https://wiki.openstreetmap.org/wiki/Proposed_features/placement#Tagging /// /// Limitations: /// - Doesn't validate tag combos, just returns the first interpretation it finds. /// - Doesn't allow `:end` and `:start` to mix `:forward` and `:back`. Maybe it should? - fn try_from(tags: &Tags) -> Result { + pub fn parse(tags: &Tags) -> Result { if let Some(transition_or_pos) = tags.get("placement") { if transition_or_pos == "transition" { Ok(Transition) } else { - Ok(Consistent(RoadPosition::try_from( - transition_or_pos.as_str(), - )?)) + Ok(Consistent(RoadPosition::parse(transition_or_pos.as_str())?)) } } else if tags.has_any(vec!["placement:start", "placement:end"]) { Ok(Varying( - RoadPosition::try_from(tags.get("placement:start").map_or("", |s| s.as_str()))?, - RoadPosition::try_from(tags.get("placement:end").map_or("", |s| s.as_str()))?, + RoadPosition::parse(tags.get("placement:start").map_or("", |s| s.as_str()))?, + RoadPosition::parse(tags.get("placement:end").map_or("", |s| s.as_str()))?, )) } else if let Some(pos) = tags.get("placement:forward") { - Ok(Consistent(RoadPosition::try_from(pos.as_str())?)) + Ok(Consistent(RoadPosition::parse(pos.as_str())?)) } else if tags.has_any(vec!["placement:forward:start", "placement:forward:end"]) { Ok(Varying( - RoadPosition::try_from( + RoadPosition::parse( tags.get("placement:forward:start") .map_or("", |s| s.as_str()), )?, - RoadPosition::try_from( - tags.get("placement:forward:end").map_or("", |s| s.as_str()), - )?, + RoadPosition::parse(tags.get("placement:forward:end").map_or("", |s| s.as_str()))?, )) } else if let Some(backwards_pos) = tags.get("placement:backward") { Ok(Consistent( - RoadPosition::try_from(backwards_pos.as_str())?.reverse(), + RoadPosition::parse(backwards_pos.as_str())?.reverse(), )) } else if tags.has_any(vec!["placement:backward:start", "placement:backward:end"]) { Ok(Varying( - RoadPosition::try_from( + RoadPosition::parse( tags.get("placement:backward:start") .map_or("", |s| s.as_str()), )? .reverse(), - RoadPosition::try_from( + RoadPosition::parse( tags.get("placement:backward:end") .map_or("", |s| s.as_str()), )? @@ -99,72 +92,79 @@ mod tests { use super::*; use std::collections::BTreeMap; use LtrLaneNum::*; - use Placement::*; - use RoadPosition::*; #[test] fn road_position_parses() { - assert_eq!(RoadPosition::try_from(""), Ok(Center)); - assert_eq!(RoadPosition::try_from("separation"), Ok(Separation)); - assert_eq!(RoadPosition::try_from("left_of:1"), Ok(LeftOf(Forward(1)))); + assert_eq!(RoadPosition::parse("").unwrap(), Center); + assert_eq!(RoadPosition::parse("separation").unwrap(), Separation); + assert_eq!( + RoadPosition::parse("left_of:1").unwrap(), + LeftOf(Forward(1)) + ); assert_eq!( - RoadPosition::try_from("middle_of:1"), - Ok(MiddleOf(Forward(1))) + RoadPosition::parse("middle_of:1").unwrap(), + MiddleOf(Forward(1)) ); assert_eq!( - RoadPosition::try_from("right_of:1"), - Ok(RightOf(Forward(1))) + RoadPosition::parse("right_of:1").unwrap(), + RightOf(Forward(1)) ); } #[test] fn placement_parses() { assert_eq!( - Placement::try_from(&Tags::new(BTreeMap::from([( + Placement::parse(&Tags::new(BTreeMap::from([( "placement".into(), "transition".into() - )]))), - Ok(Transition) + )]))) + .unwrap(), + Transition ); assert_eq!( - Placement::try_from(&Tags::new(BTreeMap::from([( + Placement::parse(&Tags::new(BTreeMap::from([( "placement".into(), "right_of:1".into() - )]))), - Ok(Consistent(RightOf(Forward(1)))) + )]))) + .unwrap(), + Consistent(RightOf(Forward(1))) ); assert_eq!( - Placement::try_from(&Tags::new(BTreeMap::from([( + Placement::parse(&Tags::new(BTreeMap::from([( "placement:forward".into(), "right_of:1".into() - )]))), - Ok(Consistent(RightOf(Forward(1)))) + )]))) + .unwrap(), + Consistent(RightOf(Forward(1))) ); assert_eq!( - Placement::try_from(&Tags::new(BTreeMap::from([( + Placement::parse(&Tags::new(BTreeMap::from([( "placement:backward".into(), "right_of:1".into() - )]))), - Ok(Consistent(RightOf(Backward(1)))) + )]))) + .unwrap(), + Consistent(RightOf(Backward(1))) ); assert_eq!( - Placement::try_from(&Tags::new(BTreeMap::from([ + Placement::parse(&Tags::new(BTreeMap::from([ ("placement:start".into(), "right_of:1".into()), ("placement:end".into(), "right_of:2".into()) - ]))), - Ok(Varying(RightOf(Forward(1)), RightOf(Forward(2)))) + ]))) + .unwrap(), + Varying(RightOf(Forward(1)), RightOf(Forward(2))) ); assert_eq!( - Placement::try_from(&Tags::new(BTreeMap::from([ + Placement::parse(&Tags::new(BTreeMap::from([ ("placement:backward:start".into(), "right_of:1".into()), ("placement:backward:end".into(), "right_of:2".into()) - ]))), - Ok(Varying(RightOf(Backward(1)), RightOf(Backward(2)))) + ]))) + .unwrap(), + Varying(RightOf(Backward(1)), RightOf(Backward(2))) ); } } diff --git a/osm2streets/src/road.rs b/osm2streets/src/road.rs index 5eb1e637..d1a1049d 100644 --- a/osm2streets/src/road.rs +++ b/osm2streets/src/road.rs @@ -89,8 +89,10 @@ impl Road { }; // Ignoring errors for now. - let placement = - Placement::try_from(&osm_tags).unwrap_or(Placement::Consistent(RoadPosition::Center)); + let placement = Placement::parse(&osm_tags).unwrap_or_else(|e| { + warn!("bad placement value (using default): {e}"); + Placement::Consistent(RoadPosition::Center) + }); let mut result = Self { id,