Merge #72

72: Store a single geometry inside Wkt r=lnicola a=lnicola - [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md). - [ ] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users. --- r? `@urschrei` Co-authored-by: Laurențiu Nicola <lnicola@dend.ro>
georust · Jan 20, 2022 · 8e22c63 · 8e22c63
2 parents 6b5603f + cf69d2a
commit 8e22c63
Show file tree

Hide file tree

Showing 13 changed files with 66 additions and 147 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -2,8 +2,12 @@
 
 ## Unreleased
 ### Changed
-* Now accepts `MULTIPOINT`s with less parentheses, as output by `ST_AsText` in postgis:
+* Now accepts `MULTIPOINT`s with fewer parentheses, as output by `ST_AsText` in postgis:
   `MULTIPOINT(0 1, 2 3)` in addition to `MULTIPOINT((0 1), (2 3))`
+* BREAKING: Replace `Wkt::items` with `Wkt::item` and remove `Wkt::add_item()`.
+  * <https://github.com/georust/wkt/pull/72>
+* BREAKING: Reject empty strings instead of parsing them into an empty `Wkt`.
+  * <https://github.com/georust/wkt/pull/72>
 
 ## 0.9.2 - 2020-04-30
 ### Added

diff --git a/benches/parse.rs b/benches/parse.rs
@@ -2,8 +2,6 @@
 extern crate criterion;
 extern crate wkt;
 
-use criterion::Criterion;
-
 fn criterion_benchmark(c: &mut criterion::Criterion) {
     c.bench_function("parse small", |bencher| {
         let s = include_str!("./small.wkt");

diff --git a/src/conversion.rs b/src/conversion.rs
@@ -42,41 +42,31 @@ where
 {
     type Error = Error;
 
-    fn try_from(mut wkt: Wkt<T>) -> Result<Self, Self::Error> {
-        if wkt.items.len() == 1 {
-            Self::try_from(wkt.items.pop().unwrap())
-        } else {
-            Geometry::GeometryCollection(GeometryCollection(wkt.items)).try_into()
-        }
+    fn try_from(wkt: Wkt<T>) -> Result<Self, Self::Error> {
+        Self::try_from(wkt.item)
     }
 }
 
-#[macro_use]
 macro_rules! try_from_wkt_impl {
     ($($type: ident),+) => {
         $(
             /// Convert a Wkt enum into a specific geo-type
             impl<T: CoordFloat> TryFrom<Wkt<T>> for geo_types::$type<T> {
                 type Error = Error;
 
-                fn try_from(mut wkt: Wkt<T>) -> Result<Self, Self::Error> {
-                    match wkt.items.len() {
-                        1 => {
-                            let item = wkt.items.pop().unwrap();
-                            let geometry = geo_types::Geometry::try_from(item)?;
-                            Self::try_from(geometry).map_err(|e| {
-                                match e {
-                                    geo_types::Error::MismatchedGeometry { expected, found } => {
-                                        Error::MismatchedGeometry { expected, found }
-                                    }
-                                    // currently only one error type in geo-types error enum, but that seems likely to change
-                                    #[allow(unreachable_patterns)]
-                                    other => Error::External(Box::new(other)),
-                                }
-                            })
+                fn try_from(wkt: Wkt<T>) -> Result<Self, Self::Error> {
+                    let item = wkt.item;
+                    let geometry = geo_types::Geometry::try_from(item)?;
+                    Self::try_from(geometry).map_err(|e| {
+                        match e {
+                            geo_types::Error::MismatchedGeometry { expected, found } => {
+                                Error::MismatchedGeometry { expected, found }
+                            }
+                            // currently only one error type in geo-types error enum, but that seems likely to change
+                            #[allow(unreachable_patterns)]
+                            other => Error::External(Box::new(other)),
                         }
-                        other => Err(Error::WrongNumberOfGeometries(other)),
-                    }
+                    })
                 }
             }
         )+
@@ -321,48 +311,14 @@ mod tests {
             m: None,
         }))
         .as_item();
-        let mut wkt = Wkt::new();
-        wkt.add_item(w_point);
+        let wkt = Wkt { item: w_point };
 
         let converted = geo_types::Geometry::try_from(wkt).unwrap();
         let g_point: geo_types::Point<f64> = geo_types::Point::new(1.0, 2.0);
 
         assert_eq!(converted, geo_types::Geometry::Point(g_point));
     }
 
-    #[test]
-    fn convert_collection_wkt() {
-        let w_point_1 = Point(Some(Coord {
-            x: 1.0,
-            y: 2.0,
-            z: None,
-            m: None,
-        }))
-        .as_item();
-        let w_point_2 = Point(Some(Coord {
-            x: 3.0,
-            y: 4.0,
-            z: None,
-            m: None,
-        }))
-        .as_item();
-        let mut wkt = Wkt::new();
-        wkt.add_item(w_point_1);
-        wkt.add_item(w_point_2);
-
-        let converted = geo_types::Geometry::try_from(wkt).unwrap();
-        let geo_collection: geo_types::GeometryCollection<f64> =
-            geo_types::GeometryCollection(vec![
-                geo_types::Point::new(1.0, 2.0).into(),
-                geo_types::Point::new(3.0, 4.0).into(),
-            ]);
-
-        assert_eq!(
-            converted,
-            geo_types::Geometry::GeometryCollection(geo_collection)
-        );
-    }
-
     #[test]
     fn convert_empty_point() {
         let point = Point(None);

diff --git a/src/deserialize.rs b/src/deserialize.rs
@@ -71,14 +71,8 @@ where
     where
         E: Error,
     {
-        let mut wkt = Wkt::from_str(s).map_err(|e| serde::de::Error::custom(e))?;
-        if wkt.items.len() == 1 {
-            Ok(wkt.items.remove(0))
-        } else {
-            Err(serde::de::Error::custom(
-                "WKT should have only 1 Geometry item",
-            ))
-        }
+        let wkt = Wkt::from_str(s).map_err(|e| serde::de::Error::custom(e))?;
+        Ok(wkt.item)
     }
 }
 
@@ -197,7 +191,7 @@ mod tests {
                 .deserialize_any(WktVisitor::<f64>::default())
                 .unwrap();
             assert!(matches!(
-                wkt.items[0],
+                wkt.item,
                 Geometry::Point(Point(Some(Coord {
                     x: _, // floating-point types cannot be used in patterns
                     y: _, // floating-point types cannot be used in patterns

diff --git a/src/lib.rs b/src/lib.rs
@@ -131,28 +131,19 @@ pub struct Wkt<T>
 where
     T: WktFloat,
 {
-    pub items: Vec<Geometry<T>>,
+    pub item: Geometry<T>,
 }
 
 impl<T> Wkt<T>
 where
     T: WktFloat + FromStr + Default,
 {
-    pub fn new() -> Self {
-        Wkt { items: vec![] }
-    }
-
-    pub fn add_item(&mut self, item: Geometry<T>) {
-        self.items.push(item);
-    }
-
     pub fn from_str(wkt_str: &str) -> Result<Self, &'static str> {
         let tokens = Tokens::from_str(wkt_str);
         Wkt::from_tokens(tokens)
     }
 
     fn from_tokens(tokens: Tokens<T>) -> Result<Self, &'static str> {
-        let mut wkt = Wkt::new();
         let mut tokens = tokens.peekable();
         let word = match tokens.next() {
             Some(Token::Word(word)) => {
@@ -161,14 +152,12 @@ where
                 }
                 word
             }
-            None => return Ok(wkt),
             _ => return Err("Invalid WKT format"),
         };
         match Geometry::from_word_and_tokens(&word, &mut tokens) {
-            Ok(item) => wkt.add_item(item),
+            Ok(item) => Ok(Wkt { item }),
             Err(s) => return Err(s),
         }
-        Ok(wkt)
     }
 }
 
@@ -230,32 +219,29 @@ mod tests {
 
     #[test]
     fn empty_string() {
-        let wkt: Wkt<f64> = Wkt::from_str("").ok().unwrap();
-        assert_eq!(0, wkt.items.len());
+        let res: Result<Wkt<f64>, _> = Wkt::from_str("");
+        assert!(res.is_err());
     }
 
     #[test]
     fn empty_items() {
-        let mut wkt: Wkt<f64> = Wkt::from_str("POINT EMPTY").ok().unwrap();
-        assert_eq!(1, wkt.items.len());
-        match wkt.items.pop().unwrap() {
+        let wkt: Wkt<f64> = Wkt::from_str("POINT EMPTY").ok().unwrap();
+        match wkt.item {
             Geometry::Point(Point(None)) => (),
             _ => unreachable!(),
         };
 
-        let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOLYGON EMPTY").ok().unwrap();
-        assert_eq!(1, wkt.items.len());
-        match wkt.items.pop().unwrap() {
+        let wkt: Wkt<f64> = Wkt::from_str("MULTIPOLYGON EMPTY").ok().unwrap();
+        match wkt.item {
             Geometry::MultiPolygon(MultiPolygon(polygons)) => assert_eq!(polygons.len(), 0),
             _ => unreachable!(),
         };
     }
 
     #[test]
     fn lowercase_point() {
-        let mut wkt: Wkt<f64> = Wkt::from_str("point EMPTY").ok().unwrap();
-        assert_eq!(1, wkt.items.len());
-        match wkt.items.pop().unwrap() {
+        let wkt: Wkt<f64> = Wkt::from_str("point EMPTY").ok().unwrap();
+        match wkt.item {
             Geometry::Point(Point(None)) => (),
             _ => unreachable!(),
         };
@@ -272,10 +258,8 @@ mod tests {
 
     #[test]
     fn support_jts_linearring() {
-        let mut wkt: Wkt<f64> = Wkt::from_str("linearring (10 20, 30 40)").ok().unwrap();
-        assert_eq!(1, wkt.items.len());
-
-        match wkt.items.pop().unwrap() {
+        let wkt: Wkt<f64> = Wkt::from_str("linearring (10 20, 30 40)").ok().unwrap();
+        match wkt.item {
             Geometry::LineString(_ls) => (),
             _ => panic!("expected to be parsed as a LINESTRING"),
         };

diff --git a/src/towkt.rs b/src/towkt.rs
@@ -219,8 +219,6 @@ where
 {
     fn to_wkt(&self) -> Wkt<T> {
         let w_geom = g_geom_to_w_geom(&self);
-        Wkt {
-            items: vec![w_geom],
-        }
+        Wkt { item: w_geom }
     }
 }
diff --git a/src/types/geometrycollection.rs b/src/types/geometrycollection.rs
@@ -88,11 +88,10 @@ mod tests {
 
     #[test]
     fn basic_geometrycollection() {
-        let mut wkt: Wkt<f64> = Wkt::from_str("GEOMETRYCOLLECTION (POINT (8 4)))")
+        let wkt: Wkt<f64> = Wkt::from_str("GEOMETRYCOLLECTION (POINT (8 4)))")
             .ok()
             .unwrap();
-        assert_eq!(1, wkt.items.len());
-        let items = match wkt.items.pop().unwrap() {
+        let items = match wkt.item {
             Geometry::GeometryCollection(GeometryCollection(items)) => items,
             _ => unreachable!(),
         };
@@ -101,12 +100,10 @@ mod tests {
 
     #[test]
     fn complex_geometrycollection() {
-        let mut wkt: Wkt<f64> =
-            Wkt::from_str("GEOMETRYCOLLECTION (POINT (8 4),LINESTRING(4 6,7 10)))")
-                .ok()
-                .unwrap();
-        assert_eq!(1, wkt.items.len());
-        let items = match wkt.items.pop().unwrap() {
+        let wkt: Wkt<f64> = Wkt::from_str("GEOMETRYCOLLECTION (POINT (8 4),LINESTRING(4 6,7 10)))")
+            .ok()
+            .unwrap();
+        let items = match wkt.item {
             Geometry::GeometryCollection(GeometryCollection(items)) => items,
             _ => unreachable!(),
         };

diff --git a/src/types/linestring.rs b/src/types/linestring.rs
@@ -67,9 +67,8 @@ mod tests {
 
     #[test]
     fn basic_linestring() {
-        let mut wkt = Wkt::from_str("LINESTRING (10 -20, -0 -0.5)").ok().unwrap();
-        assert_eq!(1, wkt.items.len());
-        let coords = match wkt.items.pop().unwrap() {
+        let wkt = Wkt::from_str("LINESTRING (10 -20, -0 -0.5)").ok().unwrap();
+        let coords = match wkt.item {
             Geometry::LineString(LineString(coords)) => coords,
             _ => unreachable!(),
         };

diff --git a/src/types/multilinestring.rs b/src/types/multilinestring.rs
@@ -76,11 +76,10 @@ mod tests {
 
     #[test]
     fn basic_multilinestring() {
-        let mut wkt: Wkt<f64> = Wkt::from_str("MULTILINESTRING ((8 4, -3 0), (4 0, 6 -10))")
+        let wkt: Wkt<f64> = Wkt::from_str("MULTILINESTRING ((8 4, -3 0), (4 0, 6 -10))")
             .ok()
             .unwrap();
-        assert_eq!(1, wkt.items.len());
-        let lines = match wkt.items.pop().unwrap() {
+        let lines = match wkt.item {
             Geometry::MultiLineString(MultiLineString(lines)) => lines,
             _ => unreachable!(),
         };

diff --git a/src/types/multipoint.rs b/src/types/multipoint.rs
@@ -72,9 +72,8 @@ mod tests {
 
     #[test]
     fn basic_multipoint() {
-        let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT ((8 4), (4 0))").ok().unwrap();
-        assert_eq!(1, wkt.items.len());
-        let points = match wkt.items.pop().unwrap() {
+        let wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT ((8 4), (4 0))").ok().unwrap();
+        let points = match wkt.item {
             Geometry::MultiPoint(MultiPoint(points)) => points,
             _ => unreachable!(),
         };
@@ -83,9 +82,8 @@ mod tests {
 
     #[test]
     fn postgis_style_multipoint() {
-        let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT (8 4, 4 0)").unwrap();
-        assert_eq!(1, wkt.items.len());
-        let points = match wkt.items.pop().unwrap() {
+        let wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT (8 4, 4 0)").unwrap();
+        let points = match wkt.item {
             Geometry::MultiPoint(MultiPoint(points)) => points,
             _ => unreachable!(),
         };
@@ -94,9 +92,8 @@ mod tests {
 
     #[test]
     fn mixed_parens_multipoint() {
-        let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT (8 4, (4 0))").unwrap();
-        assert_eq!(1, wkt.items.len());
-        let points = match wkt.items.pop().unwrap() {
+        let wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT (8 4, (4 0))").unwrap();
+        let points = match wkt.item {
             Geometry::MultiPoint(MultiPoint(points)) => points,
             _ => unreachable!(),
         };
@@ -105,9 +102,8 @@ mod tests {
 
     #[test]
     fn empty_multipoint() {
-        let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT EMPTY").unwrap();
-        assert_eq!(1, wkt.items.len());
-        let points = match wkt.items.pop().unwrap() {
+        let wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT EMPTY").unwrap();
+        let points = match wkt.item {
             Geometry::MultiPoint(MultiPoint(points)) => points,
             _ => unreachable!(),
         };

diff --git a/src/types/multipolygon.rs b/src/types/multipolygon.rs
@@ -81,11 +81,10 @@ mod tests {
 
     #[test]
     fn basic_multipolygon() {
-        let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOLYGON (((8 4)), ((4 0)))")
+        let wkt: Wkt<f64> = Wkt::from_str("MULTIPOLYGON (((8 4)), ((4 0)))")
             .ok()
             .unwrap();
-        assert_eq!(1, wkt.items.len());
-        let polygons = match wkt.items.pop().unwrap() {
+        let polygons = match wkt.item {
             Geometry::MultiPolygon(MultiPolygon(polygons)) => polygons,
             _ => unreachable!(),
         };