change the boolops trait to return result

geo/CHANGES.md

@@ -1,7 +1,8 @@
 # Changes
 
 ## Unreleased
-
+* Change BooleanOps trait to return an ErrorType instead of crashing when given bad input
+  * <https://github.com/georust/geo/pull/1233>
 * Implement getter methods on `AffineTransform` to access internal elements.
   * <https://github.com/georust/geo/pull/1159>
 * Fix issue in Debug impl for AffineTransform where yoff is shown instead of xoff

geo/src/algorithm/bool_ops/mod.rs

@@ -1,7 +1,23 @@
+use std::{error::Error, fmt::Display};
+
 use geo_types::{MultiLineString, MultiPolygon};
 
 use crate::{CoordsIter, GeoFloat, GeoNum, Polygon};
 
+#[derive(Debug)]
+pub enum BooleanError {
+    Crash,
+}
+
+impl Display for BooleanError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str("Crash")
+    }
+}
+impl Error for BooleanError {}
+
+pub type BooleanResult<T> = Result<T, BooleanError>;
+
 /// Boolean Operations on geometry.
 ///
 /// Boolean operations are set operations on geometries considered as a subset
@@ -25,17 +41,17 @@ use crate::{CoordsIter, GeoFloat, GeoNum, Polygon};
 pub trait BooleanOps: Sized {
     type Scalar: GeoNum;
 
-    fn boolean_op(&self, other: &Self, op: OpType) -> MultiPolygon<Self::Scalar>;
-    fn intersection(&self, other: &Self) -> MultiPolygon<Self::Scalar> {
+    fn boolean_op(&self, other: &Self, op: OpType) -> BooleanResult<MultiPolygon<Self::Scalar>>;
+    fn intersection(&self, other: &Self) -> BooleanResult<MultiPolygon<Self::Scalar>> {
         self.boolean_op(other, OpType::Intersection)
     }
-    fn union(&self, other: &Self) -> MultiPolygon<Self::Scalar> {
+    fn union(&self, other: &Self) -> BooleanResult<MultiPolygon<Self::Scalar>> {
         self.boolean_op(other, OpType::Union)
     }
-    fn xor(&self, other: &Self) -> MultiPolygon<Self::Scalar> {
+    fn xor(&self, other: &Self) -> BooleanResult<MultiPolygon<Self::Scalar>> {
         self.boolean_op(other, OpType::Xor)
     }
-    fn difference(&self, other: &Self) -> MultiPolygon<Self::Scalar> {
+    fn difference(&self, other: &Self) -> BooleanResult<MultiPolygon<Self::Scalar>> {
         self.boolean_op(other, OpType::Difference)
     }
 
@@ -47,7 +63,7 @@ pub trait BooleanOps: Sized {
         &self,
         ls: &MultiLineString<Self::Scalar>,
         invert: bool,
-    ) -> MultiLineString<Self::Scalar>;
+    ) -> BooleanResult<MultiLineString<Self::Scalar>>;
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
@@ -61,51 +77,51 @@ pub enum OpType {
 impl<T: GeoFloat> BooleanOps for Polygon<T> {
     type Scalar = T;
 
-    fn boolean_op(&self, other: &Self, op: OpType) -> MultiPolygon<Self::Scalar> {
+    fn boolean_op(&self, other: &Self, op: OpType) -> BooleanResult<MultiPolygon<Self::Scalar>> {
         let spec = BoolOp::from(op);
         let mut bop = Proc::new(spec, self.coords_count() + other.coords_count());
         bop.add_polygon(self, 0);
         bop.add_polygon(other, 1);
-        bop.sweep()
+        Ok(bop.sweep())
     }
 
     fn clip(
         &self,
         ls: &MultiLineString<Self::Scalar>,
         invert: bool,
-    ) -> MultiLineString<Self::Scalar> {
+    ) -> BooleanResult<MultiLineString<Self::Scalar>> {
         let spec = ClipOp::new(invert);
         let mut bop = Proc::new(spec, self.coords_count() + ls.coords_count());
         bop.add_polygon(self, 0);
         ls.0.iter().enumerate().for_each(|(idx, l)| {
             bop.add_line_string(l, idx + 1);
         });
-        bop.sweep()
+        Ok(bop.sweep())
     }
 }
 impl<T: GeoFloat> BooleanOps for MultiPolygon<T> {
     type Scalar = T;
 
-    fn boolean_op(&self, other: &Self, op: OpType) -> MultiPolygon<Self::Scalar> {
+    fn boolean_op(&self, other: &Self, op: OpType) -> BooleanResult<MultiPolygon<Self::Scalar>> {
         let spec = BoolOp::from(op);
         let mut bop = Proc::new(spec, self.coords_count() + other.coords_count());
         bop.add_multi_polygon(self, 0);
         bop.add_multi_polygon(other, 1);
-        bop.sweep()
+        Ok(bop.sweep())
     }
 
     fn clip(
         &self,
         ls: &MultiLineString<Self::Scalar>,
         invert: bool,
-    ) -> MultiLineString<Self::Scalar> {
+    ) -> BooleanResult<MultiLineString<Self::Scalar>> {
         let spec = ClipOp::new(invert);
         let mut bop = Proc::new(spec, self.coords_count() + ls.coords_count());
         bop.add_multi_polygon(self, 0);
         ls.0.iter().enumerate().for_each(|(idx, l)| {
             bop.add_line_string(l, idx + 1);
         });
-        bop.sweep()
+        Ok(bop.sweep())
     }
 }
 

geo/src/algorithm/bool_ops/tests.rs

@@ -179,7 +179,7 @@ fn test_clip_adhoc() -> Result<()> {
     let mls = MultiLineString::try_from_wkt_str(wkt2)
         .or_else(|_| LineString::<f64>::try_from_wkt_str(wkt2).map(MultiLineString::from))
         .unwrap();
-    let output = poly1.clip(&mls, true);
+    let output = poly1.clip(&mls, true)?;
     eprintln!("{wkt}", wkt = output.to_wkt());
     Ok(())
 }
@@ -209,16 +209,17 @@ fn test_issue_885_big_simplified() -> Result<()> {
 }
 
 #[test]
-fn test_issue_894() {
+fn test_issue_894() -> Result<()> {
     use geo_test_fixtures::multi_polygon;
     let a: MultiPolygon<f64> = multi_polygon("issue-894/inpa.wkt");
     let b = multi_polygon("issue-894/inpb.wkt");
     let c = multi_polygon("issue-894/inpc.wkt");
 
-    let aib = a.intersection(&b); // works
-    b.intersection(&c); // works
-    let intersection = aib.intersection(&c);
+    let aib = a.intersection(&b)?; // works
+    b.intersection(&c)?; // works
+    let intersection = aib.intersection(&c)?;
     println!("{}", intersection.to_wkt());
+    Ok(())
 }
 
 #[test]

jts-test-runner/src/runner.rs

@@ -324,9 +324,9 @@ impl TestRunner {
                     };
 
                     let actual = match (a, b) {
-                        (Geometry::Polygon(a), Geometry::Polygon(b)) => a.boolean_op(b, *op),
+                        (Geometry::Polygon(a), Geometry::Polygon(b)) => a.boolean_op(b, *op)?,
                         (Geometry::MultiPolygon(a), Geometry::MultiPolygon(b)) => {
-                            a.boolean_op(b, *op)
+                            a.boolean_op(b, *op)?
                         }
                         _ => {
                             info!("skipping unsupported Union combination: {:?}, {:?}", a, b);