Dumb down ValidationError

fj-kernel/src/shape/mod.rs

@@ -13,7 +13,7 @@ pub use self::{
     handle::Handle,
     iter::Iter,
     topology::Topology,
-    validate::{ValidationError, ValidationResult},
+    validate::{StructuralIssues, ValidationError, ValidationResult},
 };
 
 use fj_math::{Point, Scalar};

fj-kernel/src/shape/topology.rs

@@ -9,7 +9,8 @@ use crate::{
 
 use super::{
     handle::{Handle, Storage},
-    Cycles, Edges, Geometry, Iter, ValidationError, ValidationResult, Vertices,
+    Cycles, Edges, Geometry, Iter, StructuralIssues, ValidationError,
+    ValidationResult, Vertices,
 };
 
 /// The vertices of a shape
@@ -48,7 +49,7 @@ impl Topology<'_> {
     /// does. See documentation of [`crate::kernel`] for some context on that.
     pub fn add_vertex(&mut self, vertex: Vertex) -> ValidationResult<Vertex> {
         if !self.geometry.points.contains(vertex.point.storage()) {
-            return Err(ValidationError::Structural(()));
+            return Err(StructuralIssues::default().into());
         }
         for existing in &*self.vertices {
             let distance =
@@ -96,10 +97,12 @@ impl Topology<'_> {
         }
 
         if missing_curve.is_some() || !missing_vertices.is_empty() {
-            return Err(ValidationError::Structural((
+            return Err(StructuralIssues {
                 missing_curve,
                 missing_vertices,
-            )));
+                ..StructuralIssues::default()
+            }
+            .into());
         }
 
         let storage = Storage::new(edge);
@@ -162,7 +165,11 @@ impl Topology<'_> {
         }
 
         if !missing_edges.is_empty() {
-            return Err(ValidationError::Structural(missing_edges));
+            return Err(StructuralIssues {
+                missing_edges,
+                ..StructuralIssues::default()
+            }
+            .into());
         }
 
         let storage = Storage::new(cycle);
@@ -198,10 +205,12 @@ impl Topology<'_> {
             }
 
             if missing_surface.is_some() || !missing_cycles.is_empty() {
-                return Err(ValidationError::Structural((
+                return Err(StructuralIssues {
                     missing_surface,
                     missing_cycles,
-                )));
+                    ..StructuralIssues::default()
+                }
+                .into());
             }
         }
 
@@ -267,7 +276,7 @@ mod tests {
         // Should fail, as `point` is not part of the shape.
         let point = other.geometry().add_point(Point::from([1., 0., 0.]));
         let result = shape.topology().add_vertex(Vertex { point });
-        assert!(matches!(result, Err(ValidationError::Structural(()))));
+        assert!(matches!(result, Err(ValidationError::Structural(_))));
 
         // `point` is too close to the original point. `assert!` is commented,
         // because that only causes a warning to be logged right now.

fj-kernel/src/shape/validate.rs

@@ -2,23 +2,23 @@ use std::collections::HashSet;
 
 use crate::{
     geometry::{Curve, Surface},
-    topology::{Cycle, Edge, Face, Vertex},
+    topology::{Cycle, Edge, Vertex},
 };
 
 use super::Handle;
 
 /// Returned by the various `add_` methods of the [`Shape`] API
-pub type ValidationResult<T> = Result<Handle<T>, ValidationError<T>>;
+pub type ValidationResult<T> = Result<Handle<T>, ValidationError>;
 
 /// An error that can occur during a validation
 #[derive(Debug, thiserror::Error)]
-pub enum ValidationError<T: Validatable> {
+pub enum ValidationError {
     /// Structural validation failed
     ///
     /// Structural validation verifies, that all the object that an object
     /// refers to are already part of the shape.
     #[error("Structural validation failed")]
-    Structural(T::Structural),
+    Structural(StructuralIssues),
 
     /// Uniqueness validation failed
     ///
@@ -39,12 +39,12 @@ pub enum ValidationError<T: Validatable> {
     Geometric,
 }
 
-impl ValidationError<Edge> {
+impl ValidationError {
     /// Indicate whether validation found a missing curve
     #[cfg(test)]
     pub fn missing_curve(&self, curve: &Handle<Curve>) -> bool {
-        if let Self::Structural(missing) = self {
-            return missing.0.as_ref() == Some(curve);
+        if let Self::Structural(StructuralIssues { missing_curve, .. }) = self {
+            return missing_curve.as_ref() == Some(curve);
         }
 
         false
@@ -53,32 +53,34 @@ impl ValidationError<Edge> {
     /// Indicate whether validation found a missing vertex
     #[cfg(test)]
     pub fn missing_vertex(&self, vertex: &Handle<Vertex>) -> bool {
-        if let Self::Structural(missing) = self {
-            return missing.1.contains(vertex);
+        if let Self::Structural(StructuralIssues {
+            missing_vertices, ..
+        }) = self
+        {
+            return missing_vertices.contains(vertex);
         }
 
         false
     }
-}
 
-impl ValidationError<Cycle> {
     /// Indicate whether validation found a missing edge
     #[cfg(test)]
-    pub fn missing_edge(&self, vertex: &Handle<Edge>) -> bool {
-        if let Self::Structural(missing) = self {
-            return missing.contains(vertex);
+    pub fn missing_edge(&self, edge: &Handle<Edge>) -> bool {
+        if let Self::Structural(StructuralIssues { missing_edges, .. }) = self {
+            return missing_edges.contains(edge);
         }
 
         false
     }
-}
 
-impl ValidationError<Face> {
     /// Indicate whether validation found a missing surface
     #[cfg(test)]
     pub fn missing_surface(&self, surface: &Handle<Surface>) -> bool {
-        if let Self::Structural(missing) = self {
-            return missing.0.as_ref() == Some(surface);
+        if let Self::Structural(StructuralIssues {
+            missing_surface, ..
+        }) = self
+        {
+            return missing_surface.as_ref() == Some(surface);
         }
 
         false
@@ -87,33 +89,38 @@ impl ValidationError<Face> {
     /// Indicate whether validation found a missing cycle
     #[cfg(test)]
     pub fn missing_cycle(&self, cycle: &Handle<Cycle>) -> bool {
-        if let Self::Structural(missing) = self {
-            return missing.1.contains(cycle);
+        if let Self::Structural(StructuralIssues { missing_cycles, .. }) = self
+        {
+            return missing_cycles.contains(cycle);
         }
 
         false
     }
 }
 
-/// Implemented for topological types, which can be validated
-///
-/// Used by [`ValidationError`] to provide context on how validation failed.
-pub trait Validatable {
-    type Structural;
+impl From<StructuralIssues> for ValidationError {
+    fn from(issues: StructuralIssues) -> Self {
+        Self::Structural(issues)
+    }
 }
 
-impl Validatable for Vertex {
-    type Structural = ();
-}
+/// Structural issues found during validation
+///
+/// Used by [`ValidationError`].
+#[derive(Debug, Default)]
+pub struct StructuralIssues {
+    /// Missing curve found in edge validation
+    pub missing_curve: Option<Handle<Curve>>,
 
-impl Validatable for Edge {
-    type Structural = (Option<Handle<Curve>>, HashSet<Handle<Vertex>>);
-}
+    /// Missing vertices found in edge validation
+    pub missing_vertices: HashSet<Handle<Vertex>>,
 
-impl Validatable for Cycle {
-    type Structural = HashSet<Handle<Edge>>;
-}
+    /// Missing edges found in cycle validation
+    pub missing_edges: HashSet<Handle<Edge>>,
+
+    /// Missing surface found in face validation
+    pub missing_surface: Option<Handle<Surface>>,
 
-impl Validatable for Face {
-    type Structural = (Option<Handle<Surface>>, HashSet<Handle<Cycle>>);
+    /// Missing cycles found in cycle validation
+    pub missing_cycles: HashSet<Handle<Cycle>>,
 }