Merge pull request #2364 from hannobraun/validation

Continue cleanup of reference-counting validation code

crates/fj-core/src/validate/mod.rs

@@ -78,7 +78,10 @@ use crate::{
     validation::{ValidationConfig, ValidationError},
 };
 
-pub use self::{sketch::SketchValidationError, solid::SolidValidationError};
+pub use self::{
+    references::ObjectNotExclusivelyOwned, sketch::SketchValidationError,
+    solid::SolidValidationError,
+};
 
 /// Assert that some object has a validation error which matches a specific
 /// pattern. This is preferred to matching on [`Validate::validate_and_return_first_error`], since usually we don't care about the order.

crates/fj-core/src/validate/references.rs

@@ -1,46 +1,6 @@
 use std::{any::type_name_of_val, collections::HashMap, fmt};
 
-use crate::{
+use crate::storage::Handle;
-    storage::Handle,
-    topology::{Cycle, Face, HalfEdge, Region, Shell},
-};
-
-#[derive(Default)]
-pub struct ReferenceCounter<T, U>(HashMap<Handle<T>, Vec<Handle<U>>>);
-
-impl<T, U> ReferenceCounter<T, U> {
-    pub fn new() -> Self {
-        Self(HashMap::new())
-    }
-
-    pub fn count_reference(&mut self, to: Handle<T>, from: Handle<U>) {
-        self.0.entry(to).or_default().push(from);
-    }
-
-    pub fn find_multiples(&self) -> Vec<MultipleReferences<T, U>> {
-        self.0
-            .iter()
-            .filter(|(_, referenced_by)| referenced_by.len() > 1)
-            .map(|(object, referenced_by)| MultipleReferences {
-                object: object.clone(),
-                referenced_by: referenced_by.to_vec(),
-            })
-            .collect()
-    }
-}
-
-/// Find errors and convert to [`crate::validate::ValidationError`]
-#[macro_export]
-macro_rules! validate_references {
-    ($errors:ident, $error_ty:ty;$($counter:ident, $err:ident;)*) => {
-        $(
-            $counter.find_multiples().iter().for_each(|multiple| {
-                let reference_error = ObjectNotExclusivelyOwned::$err { references: multiple.clone() };
-                $errors.push(Into::<$error_ty>::into(reference_error).into());
-            });
-        )*
-    };
-}
 
 /// Object that should be exclusively owned by another, is not
 ///
@@ -48,36 +8,12 @@ macro_rules! validate_references {
 /// that only one reference to these objects must exist within the topological
 /// object graph.
 #[derive(Clone, Debug, thiserror::Error)]
-pub enum ObjectNotExclusivelyOwned {
+pub struct ObjectNotExclusivelyOwned<T, U> {
-    /// [`Region`] referenced by more than one [`Face`]
-    #[error(transparent)]
-    Region {
-        references: MultipleReferences<Region, Face>,
-    },
-    /// [`Face`] referenced by more than one [`Shell`]
-    #[error(transparent)]
-    Face {
-        references: MultipleReferences<Face, Shell>,
-    },
-    /// [`HalfEdge`] referenced by more than one [`Cycle`]
-    #[error(transparent)]
-    HalfEdge {
-        references: MultipleReferences<HalfEdge, Cycle>,
-    },
-    /// [`Cycle`] referenced by more than one [`Region`]
-    #[error(transparent)]
-    Cycle {
-        references: MultipleReferences<Cycle, Region>,
-    },
-}
-
-#[derive(Clone, Debug, thiserror::Error)]
-pub struct MultipleReferences<T, U> {
     object: Handle<T>,
     referenced_by: Vec<Handle<U>>,
 }
 
-impl<T, U> fmt::Display for MultipleReferences<T, U>
+impl<T, U> fmt::Display for ObjectNotExclusivelyOwned<T, U>
 where
     T: fmt::Debug,
     U: fmt::Debug,
@@ -93,3 +29,40 @@ where
         )
     }
 }
+
+#[derive(Default)]
+pub struct ReferenceCounter<T, U>(HashMap<Handle<T>, Vec<Handle<U>>>);
+
+impl<T, U> ReferenceCounter<T, U> {
+    pub fn new() -> Self {
+        Self(HashMap::new())
+    }
+
+    pub fn count_reference(&mut self, to: Handle<T>, from: Handle<U>) {
+        self.0.entry(to).or_default().push(from);
+    }
+
+    pub fn find_multiples(&self) -> Vec<ObjectNotExclusivelyOwned<T, U>> {
+        self.0
+            .iter()
+            .filter(|(_, referenced_by)| referenced_by.len() > 1)
+            .map(|(object, referenced_by)| ObjectNotExclusivelyOwned {
+                object: object.clone(),
+                referenced_by: referenced_by.to_vec(),
+            })
+            .collect()
+    }
+}
+
+/// Find errors and convert to [`crate::validate::ValidationError`]
+#[macro_export]
+macro_rules! validate_references {
+    ($errors:ident;$($counter:ident, $err:ident;)*) => {
+        $(
+            $counter.find_multiples().iter().for_each(|multiple| {
+                let reference_error = ValidationError::$err(multiple.clone());
+                $errors.push(reference_error.into());
+            });
+        )*
+    };
+}

crates/fj-core/src/validate/sketch.rs

@@ -9,8 +9,7 @@ use crate::{
 };
 
 use super::{
-    references::{ObjectNotExclusivelyOwned, ReferenceCounter},
+    references::ReferenceCounter, Validate, ValidationConfig, ValidationError,
-    Validate, ValidationConfig, ValidationError,
 };
 
 impl Validate for Sketch {
@@ -37,10 +36,6 @@ impl Validate for Sketch {
 /// [`Sketch`] validation failed
 #[derive(Clone, Debug, thiserror::Error)]
 pub enum SketchValidationError {
-    /// Object within sketch referenced by more than one other object
-    #[error("Object within sketch referenced by more than one other Object")]
-    ObjectNotExclusivelyOwned(#[from] ObjectNotExclusivelyOwned),
-
     /// Region within sketch has exterior cycle with clockwise winding
     #[error(
         "Exterior cycle within sketch region has clockwise winding\n
@@ -81,9 +76,9 @@ impl SketchValidationError {
         });
 
         validate_references!(
-            errors, SketchValidationError;
+            errors;
-            referenced_edges, HalfEdge;
+            referenced_edges, MultipleReferencesToHalfEdge;
-            referenced_cycles, Cycle;
+            referenced_cycles, MultipleReferencesToCycle;
         );
     }
 
@@ -135,10 +130,7 @@ mod tests {
             build::BuildHalfEdge, build::BuildRegion, insert::Insert,
         },
         topology::{Cycle, HalfEdge, Region, Sketch, Vertex},
-        validate::{
+        validate::{SketchValidationError, Validate, ValidationError},
-            references::ObjectNotExclusivelyOwned, SketchValidationError,
-            Validate, ValidationError,
-        },
         Core,
     };
 
@@ -170,11 +162,7 @@ mod tests {
         assert_contains_err!(
             core,
             invalid_sketch,
-            ValidationError::Sketch(
+            ValidationError::MultipleReferencesToCycle(_)
-                SketchValidationError::ObjectNotExclusivelyOwned(
-                    ObjectNotExclusivelyOwned::Cycle { references: _ }
-                )
-            )
         );
 
         Ok(())
@@ -210,11 +198,7 @@ mod tests {
         assert_contains_err!(
             core,
             invalid_sketch,
-            ValidationError::Sketch(
+            ValidationError::MultipleReferencesToHalfEdge(_)
-                SketchValidationError::ObjectNotExclusivelyOwned(
-                    ObjectNotExclusivelyOwned::HalfEdge { references: _ }
-                )
-            )
         );
 
         Ok(())

crates/fj-core/src/validate/solid.rs

@@ -9,8 +9,7 @@ use crate::{
 use fj_math::Point;
 
 use super::{
-    references::{ObjectNotExclusivelyOwned, ReferenceCounter},
+    references::ReferenceCounter, Validate, ValidationConfig, ValidationError,
-    Validate, ValidationConfig, ValidationError,
 };
 
 impl Validate for Solid {
@@ -67,10 +66,6 @@ pub enum SolidValidationError {
         /// Position of second vertex
         position_b: Point<3>,
     },
-
-    /// Object within solid referenced by more than one other object
-    #[error("Object within solid referenced by more than one other Object")]
-    MultipleReferences(#[from] ObjectNotExclusivelyOwned),
 }
 
 impl SolidValidationError {
@@ -168,11 +163,11 @@ impl SolidValidationError {
         });
 
         validate_references!(
-            errors, SolidValidationError;
+            errors;
-            referenced_regions, Region;
+            referenced_regions, MultipleReferencesToRegion;
-            referenced_faces, Face;
+            referenced_faces, MultipleReferencesToFace;
-            referenced_edges, HalfEdge;
+            referenced_edges, MultipleReferencesToHalfEdge;
-            referenced_cycles, Cycle;
+            referenced_cycles, MultipleReferencesToCycle;
         );
     }
 }
@@ -187,10 +182,7 @@ mod tests {
             insert::Insert,
         },
         topology::{Cycle, Face, HalfEdge, Region, Shell, Solid, Surface},
-        validate::{
+        validate::{Validate, ValidationError},
-            references::ObjectNotExclusivelyOwned, SolidValidationError,
-            Validate, ValidationError,
-        },
         Core,
     };
 
@@ -238,9 +230,7 @@ mod tests {
         assert_contains_err!(
             core,
             invalid_solid,
-            ValidationError::Solid(SolidValidationError::MultipleReferences(
+            ValidationError::MultipleReferencesToFace(_)
-                ObjectNotExclusivelyOwned::Face { references: _ }
-            ))
         );
 
         let valid_solid = Solid::new(vec![]).insert(&mut core);
@@ -284,9 +274,7 @@ mod tests {
         assert_contains_err!(
             core,
             invalid_solid,
-            ValidationError::Solid(SolidValidationError::MultipleReferences(
+            ValidationError::MultipleReferencesToRegion(_)
-                ObjectNotExclusivelyOwned::Region { references: _ }
-            ))
         );
 
         let valid_solid = Solid::new(vec![]).insert(&mut core);
@@ -334,9 +322,7 @@ mod tests {
         assert_contains_err!(
             core,
             invalid_solid,
-            ValidationError::Solid(SolidValidationError::MultipleReferences(
+            ValidationError::MultipleReferencesToCycle(_)
-                ObjectNotExclusivelyOwned::Cycle { references: _ }
-            ))
         );
 
         let valid_solid = Solid::new(vec![]).insert(&mut core);
@@ -376,9 +362,7 @@ mod tests {
         assert_contains_err!(
             core,
             invalid_solid,
-            ValidationError::Solid(SolidValidationError::MultipleReferences(
+            ValidationError::MultipleReferencesToHalfEdge(_)
-                ObjectNotExclusivelyOwned::HalfEdge { references: _ }
-            ))
         );
 
         let valid_solid = Solid::new(vec![]).insert(&mut core);

crates/fj-core/src/validation/error.rs

@@ -1,6 +1,11 @@
 use std::{convert::Infallible, fmt};
 
-use crate::validate::{SketchValidationError, SolidValidationError};
+use crate::{
+    topology::{Cycle, Face, HalfEdge, Region, Shell},
+    validate::{
+        ObjectNotExclusivelyOwned, SketchValidationError, SolidValidationError,
+    },
+};
 
 use super::checks::{
     AdjacentHalfEdgesNotConnected, CoincidentHalfEdgesAreNotSiblings,
@@ -37,6 +42,22 @@ pub enum ValidationError {
     #[error(transparent)]
     InteriorCycleHasInvalidWinding(#[from] InteriorCycleHasInvalidWinding),
 
+    /// Multiple references to [`Cycle`]
+    #[error(transparent)]
+    MultipleReferencesToCycle(ObjectNotExclusivelyOwned<Cycle, Region>),
+
+    /// Multiple references to [`Face`]
+    #[error(transparent)]
+    MultipleReferencesToFace(ObjectNotExclusivelyOwned<Face, Shell>),
+
+    /// Multiple references to [`HalfEdge`]
+    #[error(transparent)]
+    MultipleReferencesToHalfEdge(ObjectNotExclusivelyOwned<HalfEdge, Cycle>),
+
+    /// Multiple references to [`Region`]
+    #[error(transparent)]
+    MultipleReferencesToRegion(ObjectNotExclusivelyOwned<Region, Face>),
+
     /// `Solid` validation error
     #[error("`Solid` validation error")]
     Solid(#[from] SolidValidationError),