Merge #614

614: Specify the details of conversion failures in an exported error enum r=michaelkirk a=michaelkirk - [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md). - [x] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users. --- I came across this while trying to improve ergonomics of the WKT crate, which relies on this conversion logic. (see georust/wkt#57) I *think* this is not a breaking change (see comment inline), but would appreciate confirmation. If I'm wrong, and it indeed is a breaking change, I'd prefer to hold off on merging it for now. Co-authored-by: Michael Kirk <michael.code@endoftheworl.de>
georust · Feb 6, 2021 · 7e265bd · 7e265bd
2 parents 6a74467 + 9c2d7b5
commit 7e265bd
Show file tree

Hide file tree

Showing 5 changed files with 106 additions and 88 deletions.
diff --git a/geo-types/CHANGES.md b/geo-types/CHANGES.md
@@ -4,6 +4,8 @@
 
 * Implement `Default` on `Coordinate` and `Point` structs (defaults to `(x: 0, y: 0)`)
   * <https://github.com/georust/geo/pull/616>
+* Add specific details about conversion failures in the newly public `geo_types::Error`
+  * <https://github.com/georust/geo/pull/614>
 
 ## 0.7.0
 

diff --git a/geo-types/src/error.rs b/geo-types/src/error.rs
@@ -0,0 +1,44 @@
+use std::fmt;
+
+#[derive(Debug)]
+pub enum Error {
+    MismatchedGeometry {
+        expected: &'static str,
+        found: &'static str,
+    },
+}
+
+impl std::error::Error for Error {}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Error::MismatchedGeometry { expected, found } => {
+                write!(f, "Expected a {}, but found a {}", expected, found)
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use crate::{Geometry, Point, Rect};
+    use std::convert::TryFrom;
+
+    #[test]
+    fn error_output() {
+        let point = Point::new(1.0, 2.0);
+        let point_geometry = Geometry::from(point);
+
+        let rect = Rect::new(Point::new(1.0, 2.0), Point::new(3.0, 4.0));
+        let rect_geometry = Geometry::from(rect);
+
+        Point::try_from(point_geometry).expect("failed to unwrap inner enum Point");
+
+        let failure = Point::try_from(rect_geometry).unwrap_err();
+        assert_eq!(
+            format!("{}", failure),
+            "Expected a geo_types::point::Point<f64>, but found a geo_types::rect::Rect<f64>"
+        );
+    }
+}
diff --git a/geo-types/src/geometry.rs b/geo-types/src/geometry.rs
@@ -1,10 +1,9 @@
 use crate::{
-    CoordNum, GeometryCollection, Line, LineString, MultiLineString, MultiPoint, MultiPolygon,
-    Point, Polygon, Rect, Triangle,
+    CoordNum, Error, GeometryCollection, Line, LineString, MultiLineString, MultiPoint,
+    MultiPolygon, Point, Polygon, Rect, Triangle,
 };
+use core::any::type_name;
 use std::convert::TryFrom;
-use std::error::Error;
-use std::fmt;
 
 /// An enum representing any possible geometry type.
 ///
@@ -182,94 +181,56 @@ impl<T: CoordNum> Geometry<T> {
     }
 }
 
-#[derive(Debug)]
-pub struct FailedToConvertError;
-
-impl fmt::Display for FailedToConvertError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "Could not convert from enum member to concrete type")
-    }
-}
-
-impl Error for FailedToConvertError {
-    fn description(&self) -> &str {
-        "Could not convert from enum member to concrete type"
-    }
-}
-
-impl<T: CoordNum> TryFrom<Geometry<T>> for Point<T> {
-    type Error = FailedToConvertError;
-
-    fn try_from(geom: Geometry<T>) -> Result<Point<T>, Self::Error> {
-        match geom {
-            Geometry::Point(p) => Ok(p),
-            _ => Err(FailedToConvertError),
+#[macro_use]
+macro_rules! try_from_geometry_impl {
+    ($($type: ident),+) => {
+        $(
+        /// Convert a Geometry enum into its inner type.
+        ///
+        /// Fails if the enum case does not match the type you are trying to convert it to.
+        impl <T: CoordNum> TryFrom<Geometry<T>> for $type<T> {
+            type Error = Error;
+
+            fn try_from(geom: Geometry<T>) -> Result<Self, Self::Error> {
+                match geom {
+                    Geometry::$type(g) => Ok(g),
+                    other => Err(Error::MismatchedGeometry {
+                        expected: type_name::<$type<T>>(),
+                        found: inner_type_name(other)
+                    })
+                }
+            }
         }
+        )+
     }
 }
 
-impl<T: CoordNum> TryFrom<Geometry<T>> for Line<T> {
-    type Error = FailedToConvertError;
+try_from_geometry_impl!(
+    Point,
+    Line,
+    LineString,
+    Polygon,
+    MultiPoint,
+    MultiLineString,
+    MultiPolygon,
+    Rect,
+    Triangle
+);
 
-    fn try_from(geom: Geometry<T>) -> Result<Line<T>, Self::Error> {
-        match geom {
-            Geometry::Line(l) => Ok(l),
-            _ => Err(FailedToConvertError),
-        }
-    }
-}
-
-impl<T: CoordNum> TryFrom<Geometry<T>> for LineString<T> {
-    type Error = FailedToConvertError;
-
-    fn try_from(geom: Geometry<T>) -> Result<LineString<T>, Self::Error> {
-        match geom {
-            Geometry::LineString(ls) => Ok(ls),
-            _ => Err(FailedToConvertError),
-        }
-    }
-}
-
-impl<T: CoordNum> TryFrom<Geometry<T>> for Polygon<T> {
-    type Error = FailedToConvertError;
-
-    fn try_from(geom: Geometry<T>) -> Result<Polygon<T>, Self::Error> {
-        match geom {
-            Geometry::Polygon(ls) => Ok(ls),
-            _ => Err(FailedToConvertError),
-        }
-    }
-}
-
-impl<T: CoordNum> TryFrom<Geometry<T>> for MultiPoint<T> {
-    type Error = FailedToConvertError;
-
-    fn try_from(geom: Geometry<T>) -> Result<MultiPoint<T>, Self::Error> {
-        match geom {
-            Geometry::MultiPoint(mp) => Ok(mp),
-            _ => Err(FailedToConvertError),
-        }
-    }
-}
-
-impl<T: CoordNum> TryFrom<Geometry<T>> for MultiLineString<T> {
-    type Error = FailedToConvertError;
-
-    fn try_from(geom: Geometry<T>) -> Result<MultiLineString<T>, Self::Error> {
-        match geom {
-            Geometry::MultiLineString(mls) => Ok(mls),
-            _ => Err(FailedToConvertError),
-        }
-    }
-}
-
-impl<T: CoordNum> TryFrom<Geometry<T>> for MultiPolygon<T> {
-    type Error = FailedToConvertError;
-
-    fn try_from(geom: Geometry<T>) -> Result<MultiPolygon<T>, Self::Error> {
-        match geom {
-            Geometry::MultiPolygon(mp) => Ok(mp),
-            _ => Err(FailedToConvertError),
-        }
+fn inner_type_name<T>(geometry: Geometry<T>) -> &'static str
+where
+    T: CoordNum,
+{
+    match geometry {
+        Geometry::Point(_) => type_name::<Point<T>>(),
+        Geometry::Line(_) => type_name::<Line<T>>(),
+        Geometry::LineString(_) => type_name::<LineString<T>>(),
+        Geometry::Polygon(_) => type_name::<Polygon<T>>(),
+        Geometry::MultiPoint(_) => type_name::<MultiPoint<T>>(),
+        Geometry::MultiLineString(_) => type_name::<MultiLineString<T>>(),
+        Geometry::MultiPolygon(_) => type_name::<MultiPolygon<T>>(),
+        Geometry::GeometryCollection(_) => type_name::<GeometryCollection<T>>(),
+        Geometry::Rect(_) => type_name::<Rect<T>>(),
+        Geometry::Triangle(_) => type_name::<Triangle<T>>(),
     }
 }
diff --git a/geo-types/src/lib.rs b/geo-types/src/lib.rs
@@ -114,8 +114,12 @@ mod triangle;
 pub use crate::triangle::Triangle;
 
 mod rect;
+#[allow(deprecated)]
 pub use crate::rect::{InvalidRectCoordinatesError, Rect};
 
+mod error;
+pub use error::Error;
+
 #[macro_use]
 mod macros;
 

diff --git a/geo-types/src/rect.rs b/geo-types/src/rect.rs
@@ -85,6 +85,7 @@ impl<T: CoordNum> Rect<T> {
         since = "0.6.2",
         note = "Use `Rect::new` instead, since `Rect::try_new` will never Error"
     )]
+    #[allow(deprecated)]
     pub fn try_new<C>(c1: C, c2: C) -> Result<Rect<T>, InvalidRectCoordinatesError>
     where
         C: Into<Coordinate<T>>,
@@ -264,11 +265,17 @@ impl<T: CoordFloat> Rect<T> {
 
 static RECT_INVALID_BOUNDS_ERROR: &str = "Failed to create Rect: 'min' coordinate's x/y value must be smaller or equal to the 'max' x/y value";
 
+#[deprecated(
+    since = "0.6.2",
+    note = "Use `Rect::new` instead, since `Rect::try_new` will never Error"
+)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct InvalidRectCoordinatesError;
 
+#[allow(deprecated)]
 impl std::error::Error for InvalidRectCoordinatesError {}
 
+#[allow(deprecated)]
 impl std::fmt::Display for InvalidRectCoordinatesError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", RECT_INVALID_BOUNDS_ERROR)