Merge #203

203: mismatch type error r=metasim 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.
---

Fixes #201
~~Based on #202, so let's merge that first.~~ merged!


Co-authored-by: Michael Kirk <michael.code@endoftheworl.de>

CHANGES.md

@@ -10,6 +10,8 @@
   * <https://github.com/georust/geojson/pull/198>
 * Add `TryFrom<&geometry::Value>` for geo_type variants.
   * <https://github.com/georust/geojson/pull/202> 
+* Changed the format of the error produced when converting a geometry to an incompatible type - e.g. a LineString into a Point.
+  * <https://github.com/georust/geojson/pull/203>
 
 ## 0.23.0
 

src/conversion/mod.rs

@@ -63,12 +63,10 @@ macro_rules! assert_almost_eq {
     }};
 }
 
-
 macro_rules! try_from_owned_value {
     ($to:ty) => {
         #[cfg_attr(docsrs, doc(cfg(feature = "geo-types")))]
-        impl<T: CoordFloat> TryFrom<geometry::Value> for $to
-        {
+        impl<T: CoordFloat> TryFrom<geometry::Value> for $to {
             type Error = Error;
 
             fn try_from(value: geometry::Value) -> Result<Self> {
@@ -78,7 +76,6 @@ macro_rules! try_from_owned_value {
     };
 }
 
-
 pub(crate) mod from_geo_types;
 pub(crate) mod to_geo_types;
 

src/conversion/to_geo_types.rs

@@ -19,7 +19,7 @@ where
     fn try_from(value: &geometry::Value) -> Result<Self> {
         match value {
             geometry::Value::Point(point_type) => Ok(create_geo_point(&point_type)),
-            _ => Err(Error::InvalidGeometryConversion(value.clone())),
+            other => Err(mismatch_geom_err("Point", &other)),
         }
     }
 }
@@ -40,7 +40,7 @@ where
                     .map(|point_type| create_geo_point(&point_type))
                     .collect(),
             )),
-            _ => Err(Error::InvalidGeometryConversion(value.clone())),
+            other => Err(mismatch_geom_err("MultiPoint", &other)),
         }
     }
 }
@@ -58,7 +58,7 @@ where
             geometry::Value::LineString(multi_point_type) => {
                 Ok(create_geo_line_string(&multi_point_type))
             }
-            _ => Err(Error::InvalidGeometryConversion(value.clone())),
+            other => Err(mismatch_geom_err("LineString", &other)),
         }
     }
 }
@@ -76,7 +76,7 @@ where
             geometry::Value::MultiLineString(multi_line_string_type) => {
                 Ok(create_geo_multi_line_string(&multi_line_string_type))
             }
-            _ => Err(Error::InvalidGeometryConversion(value.clone())),
+            other => Err(mismatch_geom_err("MultiLineString", &other)),
         }
     }
 }
@@ -92,7 +92,7 @@ where
     fn try_from(value: &geometry::Value) -> Result<Self> {
         match value {
             geometry::Value::Polygon(polygon_type) => Ok(create_geo_polygon(&polygon_type)),
-            _ => Err(Error::InvalidGeometryConversion(value.clone())),
+            other => Err(mismatch_geom_err("Polygon", &other)),
         }
     }
 }
@@ -110,13 +110,12 @@ where
             geometry::Value::MultiPolygon(multi_polygon_type) => {
                 Ok(create_geo_multi_polygon(&multi_polygon_type))
             }
-            _ => Err(Error::InvalidGeometryConversion(value.clone())),
+            other => Err(mismatch_geom_err("MultiPolygon", other)),
         }
     }
 }
 try_from_owned_value!(geo_types::MultiPolygon<T>);
 
-
 #[cfg_attr(docsrs, doc(cfg(feature = "geo-types")))]
 impl<T> TryFrom<&geometry::Value> for geo_types::GeometryCollection<T>
 where
@@ -134,14 +133,13 @@ where
 
                 Ok(geo_types::GeometryCollection(geojson_geometries))
             }
-            _ => Err(Error::InvalidGeometryConversion(value.clone())),
+            other => Err(mismatch_geom_err("GeometryCollection", other)),
         }
     }
 }
 
 try_from_owned_value!(geo_types::GeometryCollection<T>);
 
-
 #[cfg_attr(docsrs, doc(cfg(feature = "geo-types")))]
 impl<T> TryFrom<&geometry::Value> for geo_types::Geometry<T>
 where
@@ -339,6 +337,13 @@ where
     )
 }
 
+fn mismatch_geom_err(expected_type: &'static str, found: &geometry::Value) -> Error {
+    Error::InvalidGeometryConversion {
+        expected_type,
+        found_type: found.type_name(),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::{Geometry, Value};
@@ -594,6 +599,19 @@ mod tests {
         assert_almost_eq!(geo_point.y(), coords[1], 1e-6);
     }
 
+    #[test]
+    fn geojson_mismatch_geometry_conversion_test() {
+        let coord1 = vec![100.0, 0.2];
+        let coord2 = vec![101.0, 1.0];
+        let geojson_line_string = Value::LineString(vec![coord1.clone(), coord2.clone()]);
+        use std::convert::TryFrom;
+        let error = geo_types::Point::<f64>::try_from(geojson_line_string).unwrap_err();
+        assert_eq!(
+            "Expected type: `Point`, but found `LineString`",
+            format!("{}", error)
+        )
+    }
+
     #[test]
     fn feature_collection_with_geom_collection() {
         let geojson_str = json!({
@@ -683,7 +701,7 @@ mod tests {
         let geojson_polygon = Value::Polygon(geojson_multi_line_string_type1);
         let _: geo_types::Polygon<f64> = (&geojson_polygon).try_into()?;
 
-       let geojson_line_string_type1 = vec![
+        let geojson_line_string_type1 = vec![
             coord1.clone(),
             coord2.clone(),
             coord3.clone(),
@@ -700,7 +718,7 @@ mod tests {
             vec![geojson_line_string_type1],
             vec![geojson_line_string_type2],
         ]);
-        let _: geo_types::MultiPolygon<f64> =  (&geojson_multi_polygon).try_into()?;
+        let _: geo_types::MultiPolygon<f64> = (&geojson_multi_polygon).try_into()?;
 
         let geojson_geometry_collection = Value::GeometryCollection(vec![
             Geometry::new(geojson_multi_point),

src/errors.rs

@@ -1,5 +1,4 @@
 //! Module for all GeoJSON-related errors
-use crate::geometry::Value as GValue;
 use crate::Feature;
 use serde_json::value::Value;
 use thiserror::Error;
@@ -21,8 +20,11 @@ pub enum Error {
     /// This was previously `GeoJsonUnknownType`, but has been split for clarity
     #[error("Expected a Feature mapping, but got a `{0}`")]
     NotAFeature(String),
-    #[error("Encountered a mismatch when converting to a Geo type: `{0}`")]
-    InvalidGeometryConversion(GValue),
+    #[error("Expected type: `{expected_type}`, but found `{found_type}`")]
+    InvalidGeometryConversion {
+        expected_type: &'static str,
+        found_type: &'static str,
+    },
     #[error(
         "Attempted to a convert a feature without a geometry into a geo_types::Geometry: `{0}`"
     )]

src/geometry.rs

@@ -83,21 +83,27 @@ pub enum Value {
     GeometryCollection(Vec<Geometry>),
 }
 
-impl<'a> From<&'a Value> for JsonObject {
-    fn from(value: &'a Value) -> JsonObject {
-        let mut map = JsonObject::new();
-        let ty = String::from(match value {
+impl Value {
+    pub fn type_name(&self) -> &'static str {
+        match self {
             Value::Point(..) => "Point",
             Value::MultiPoint(..) => "MultiPoint",
             Value::LineString(..) => "LineString",
             Value::MultiLineString(..) => "MultiLineString",
             Value::Polygon(..) => "Polygon",
             Value::MultiPolygon(..) => "MultiPolygon",
             Value::GeometryCollection(..) => "GeometryCollection",
-        });
-
-        map.insert(String::from("type"), ::serde_json::to_value(&ty).unwrap());
+        }
+    }
+}
 
+impl<'a> From<&'a Value> for JsonObject {
+    fn from(value: &'a Value) -> JsonObject {
+        let mut map = JsonObject::new();
+        map.insert(
+            String::from("type"),
+            ::serde_json::to_value(value.type_name()).unwrap(),
+        );
         map.insert(
             String::from(match value {
                 Value::GeometryCollection(..) => "geometries",