Don't stop on first validation error

Instead, emit all validation error, to form a more complete picture.
hannobraun · Jan 12, 2023 · f9e76b1 · f9e76b1
1 parent 6a17480
commit f9e76b1
Show file tree

Hide file tree

Showing 11 changed files with 49 additions and 87 deletions.
diff --git a/crates/fj-kernel/src/objects/object.rs b/crates/fj-kernel/src/objects/object.rs
@@ -69,11 +69,7 @@ macro_rules! object {
             pub fn validate(&self, errors: &mut Vec<ValidationError>) {
                 match self {
                     $(
-                        Self::$ty((_, object)) => {
-                            if let Err(err) = object.validate() {
-                                errors.push(err.into());
-                            }
-                        }
+                        Self::$ty((_, object)) => object.validate(errors),
                     )*
                 }
             }

diff --git a/crates/fj-kernel/src/validate/curve.rs b/crates/fj-kernel/src/validate/curve.rs
@@ -6,16 +6,16 @@ impl Validate for Curve {
     fn validate_with_config(
         &self,
         _: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        Ok(())
+        _: &mut Vec<ValidationError>,
+    ) {
     }
 }
 
 impl Validate for GlobalCurve {
     fn validate_with_config(
         &self,
         _: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        Ok(())
+        _: &mut Vec<ValidationError>,
+    ) {
     }
 }
diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs
@@ -11,20 +11,13 @@ impl Validate for Cycle {
     fn validate_with_config(
         &self,
         _: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        let mut errors = Vec::new();
-
-        CycleValidationError::check_half_edge_connections(self, &mut errors);
+        errors: &mut Vec<ValidationError>,
+    ) {
+        CycleValidationError::check_half_edge_connections(self, errors);
 
         // We don't need to check that all half-edges are defined in the same
         // surface. We already check that they are connected by identical
         // surface vertices, so that would be redundant.
-
-        if let Some(error) = errors.into_iter().next() {
-            return Err(error);
-        }
-
-        Ok(())
     }
 }
 

diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs
@@ -15,39 +15,25 @@ impl Validate for HalfEdge {
     fn validate_with_config(
         &self,
         config: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        let mut errors = Vec::new();
-
-        HalfEdgeValidationError::check_curve_identity(self, &mut errors);
-        HalfEdgeValidationError::check_global_curve_identity(self, &mut errors);
-        HalfEdgeValidationError::check_global_vertex_identity(
-            self,
-            &mut errors,
-        );
-        HalfEdgeValidationError::check_vertex_positions(
-            self,
-            config,
-            &mut errors,
-        );
+        errors: &mut Vec<ValidationError>,
+    ) {
+        HalfEdgeValidationError::check_curve_identity(self, errors);
+        HalfEdgeValidationError::check_global_curve_identity(self, errors);
+        HalfEdgeValidationError::check_global_vertex_identity(self, errors);
+        HalfEdgeValidationError::check_vertex_positions(self, config, errors);
 
         // We don't need to check anything about surfaces here. We already check
         // curves, which makes sure the vertices are consistent with each other,
         // and the validation of those vertices checks the surfaces.
-
-        if let Some(err) = errors.into_iter().next() {
-            return Err(err);
-        }
-
-        Ok(())
     }
 }
 
 impl Validate for GlobalEdge {
     fn validate_with_config(
         &self,
         _: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        Ok(())
+        _: &mut Vec<ValidationError>,
+    ) {
     }
 }
 

diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs
@@ -11,17 +11,10 @@ impl Validate for Face {
     fn validate_with_config(
         &self,
         _: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        let mut errors = Vec::new();
-
-        FaceValidationError::check_surface_identity(self, &mut errors);
-        FaceValidationError::check_interior_winding(self, &mut errors);
-
-        if let Some(err) = errors.into_iter().next() {
-            return Err(err);
-        }
-
-        Ok(())
+        errors: &mut Vec<ValidationError>,
+    ) {
+        FaceValidationError::check_surface_identity(self, errors);
+        FaceValidationError::check_interior_winding(self, errors);
     }
 }
 

diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs
@@ -27,19 +27,27 @@ use fj_math::Scalar;
 pub trait Validate: Sized {
     /// Validate the object using default config and return on first error
     fn validate_and_return_first_error(&self) -> Result<(), ValidationError> {
-        self.validate()
+        let mut errors = Vec::new();
+        self.validate(&mut errors);
+
+        if let Some(err) = errors.into_iter().next() {
+            return Err(err);
+        }
+
+        Ok(())
     }
 
     /// Validate the object using default configuration
-    fn validate(&self) -> Result<(), ValidationError> {
-        self.validate_with_config(&ValidationConfig::default())
+    fn validate(&self, errors: &mut Vec<ValidationError>) {
+        self.validate_with_config(&ValidationConfig::default(), errors)
     }
 
     /// Validate the object
     fn validate_with_config(
         &self,
         config: &ValidationConfig,
-    ) -> Result<(), ValidationError>;
+        errors: &mut Vec<ValidationError>,
+    );
 }
 
 /// Configuration required for the validation process

diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs
@@ -6,7 +6,7 @@ impl Validate for Shell {
     fn validate_with_config(
         &self,
         _: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        Ok(())
+        _: &mut Vec<ValidationError>,
+    ) {
     }
 }
diff --git a/crates/fj-kernel/src/validate/sketch.rs b/crates/fj-kernel/src/validate/sketch.rs
@@ -6,7 +6,7 @@ impl Validate for Sketch {
     fn validate_with_config(
         &self,
         _: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        Ok(())
+        _: &mut Vec<ValidationError>,
+    ) {
     }
 }
diff --git a/crates/fj-kernel/src/validate/solid.rs b/crates/fj-kernel/src/validate/solid.rs
@@ -6,7 +6,7 @@ impl Validate for Solid {
     fn validate_with_config(
         &self,
         _: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        Ok(())
+        _: &mut Vec<ValidationError>,
+    ) {
     }
 }
diff --git a/crates/fj-kernel/src/validate/surface.rs b/crates/fj-kernel/src/validate/surface.rs
@@ -6,7 +6,7 @@ impl Validate for Surface {
     fn validate_with_config(
         &self,
         _: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        Ok(())
+        _: &mut Vec<ValidationError>,
+    ) {
     }
 }
diff --git a/crates/fj-kernel/src/validate/vertex.rs b/crates/fj-kernel/src/validate/vertex.rs
@@ -11,43 +11,29 @@ impl Validate for Vertex {
     fn validate_with_config(
         &self,
         config: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        let mut errors = Vec::new();
-
-        VertexValidationError::check_surface_identity(self, &mut errors);
-        VertexValidationError::check_position(self, config, &mut errors);
-
-        if let Some(err) = errors.into_iter().next() {
-            return Err(err);
-        }
-
-        Ok(())
+        errors: &mut Vec<ValidationError>,
+    ) {
+        VertexValidationError::check_surface_identity(self, errors);
+        VertexValidationError::check_position(self, config, errors);
     }
 }
 
 impl Validate for SurfaceVertex {
     fn validate_with_config(
         &self,
         config: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        let mut errors = Vec::new();
-
-        SurfaceVertexValidationError::check_position(self, config, &mut errors);
-
-        if let Some(err) = errors.into_iter().next() {
-            return Err(err);
-        }
-
-        Ok(())
+        errors: &mut Vec<ValidationError>,
+    ) {
+        SurfaceVertexValidationError::check_position(self, config, errors);
     }
 }
 
 impl Validate for GlobalVertex {
     fn validate_with_config(
         &self,
         _: &ValidationConfig,
-    ) -> Result<(), ValidationError> {
-        Ok(())
+        _: &mut Vec<ValidationError>,
+    ) {
     }
 }