diff --git a/crates/fj-kernel/src/objects/object.rs b/crates/fj-kernel/src/objects/object.rs index 4bbb24fa89..0c0e981335 100644 --- a/crates/fj-kernel/src/objects/object.rs +++ b/crates/fj-kernel/src/objects/object.rs @@ -66,14 +66,12 @@ macro_rules! object { } /// Validate the object - pub fn validate(&self) -> Result<(), ValidationError> { + pub fn validate(&self, errors: &mut Vec) { match self { $( - Self::$ty((_, object)) => object.validate()?, + Self::$ty((_, object)) => object.validate(errors), )* } - - Ok(()) } } diff --git a/crates/fj-kernel/src/services/validation.rs b/crates/fj-kernel/src/services/validation.rs index c3f8f9d4db..bd73692016 100644 --- a/crates/fj-kernel/src/services/validation.rs +++ b/crates/fj-kernel/src/services/validation.rs @@ -37,9 +37,12 @@ impl State for Validation { type Event = ValidationFailed; fn decide(&self, command: Self::Command, events: &mut Vec) { - if let Err(err) = command.object.validate() { + let mut errors = Vec::new(); + command.object.validate(&mut errors); + + for err in errors { events.push(ValidationFailed { - object: command.object.into(), + object: command.object.clone().into(), err, }); } diff --git a/crates/fj-kernel/src/validate/curve.rs b/crates/fj-kernel/src/validate/curve.rs index d7a8e6e872..250eaea471 100644 --- a/crates/fj-kernel/src/validate/curve.rs +++ b/crates/fj-kernel/src/validate/curve.rs @@ -1,27 +1,21 @@ -use std::convert::Infallible; - use crate::objects::{Curve, GlobalCurve}; -use super::{Validate, ValidationConfig}; +use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Curve { - type Error = Infallible; - fn validate_with_config( &self, _: &ValidationConfig, - ) -> Result<(), Self::Error> { - Ok(()) + _: &mut Vec, + ) { } } impl Validate for GlobalCurve { - type Error = Infallible; - fn validate_with_config( &self, _: &ValidationConfig, - ) -> Result<(), Self::Error> { - Ok(()) + _: &mut Vec, + ) { } } diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 6b5cbd1fdc..89a9b030cb 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -5,22 +5,19 @@ use crate::{ storage::Handle, }; -use super::{Validate, ValidationConfig}; +use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Cycle { - type Error = CycleValidationError; - fn validate_with_config( &self, _: &ValidationConfig, - ) -> Result<(), Self::Error> { - CycleValidationError::check_half_edge_connections(self)?; + errors: &mut Vec, + ) { + 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. - - Ok(()) } } @@ -43,7 +40,10 @@ pub enum CycleValidationError { } impl CycleValidationError { - fn check_half_edge_connections(cycle: &Cycle) -> Result<(), Self> { + fn check_half_edge_connections( + cycle: &Cycle, + errors: &mut Vec, + ) { for (a, b) in cycle.half_edges().circular_tuple_windows() { let [_, prev] = a.vertices(); let [next, _] = b.vertices(); @@ -52,14 +52,15 @@ impl CycleValidationError { let next = next.surface_form(); if prev.id() != next.id() { - return Err(Self::HalfEdgeConnection { - prev: prev.clone(), - next: next.clone(), - }); + errors.push( + Self::HalfEdgeConnection { + prev: prev.clone(), + next: next.clone(), + } + .into(), + ); } } - - Ok(()) } } @@ -109,8 +110,8 @@ mod tests { Cycle::new(half_edges) }; - valid.validate()?; - assert!(invalid.validate().is_err()); + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); Ok(()) } diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 4745222838..c79b1f72c8 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -1,5 +1,3 @@ -use std::convert::Infallible; - use fj_interop::ext::ArrayExt; use fj_math::{Point, Scalar}; @@ -11,36 +9,31 @@ use crate::{ storage::Handle, }; -use super::{Validate, ValidationConfig}; +use super::{Validate, ValidationConfig, ValidationError}; impl Validate for HalfEdge { - type Error = HalfEdgeValidationError; - fn validate_with_config( &self, config: &ValidationConfig, - ) -> Result<(), Self::Error> { - HalfEdgeValidationError::check_curve_identity(self)?; - HalfEdgeValidationError::check_global_curve_identity(self)?; - HalfEdgeValidationError::check_global_vertex_identity(self)?; - HalfEdgeValidationError::check_vertex_positions(self, config)?; + errors: &mut Vec, + ) { + 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. - - Ok(()) } } impl Validate for GlobalEdge { - type Error = Infallible; - fn validate_with_config( &self, _: &ValidationConfig, - ) -> Result<(), Self::Error> { - Ok(()) + _: &mut Vec, + ) { } } @@ -128,38 +121,49 @@ pub enum HalfEdgeValidationError { } impl HalfEdgeValidationError { - fn check_curve_identity(half_edge: &HalfEdge) -> Result<(), Self> { + fn check_curve_identity( + half_edge: &HalfEdge, + errors: &mut Vec, + ) { let back_curve = half_edge.back().curve(); let front_curve = half_edge.front().curve(); if back_curve.id() != front_curve.id() { - return Err(Self::CurveMismatch { - back_curve: back_curve.clone(), - front_curve: front_curve.clone(), - half_edge: half_edge.clone(), - }); + errors.push( + Self::CurveMismatch { + back_curve: back_curve.clone(), + front_curve: front_curve.clone(), + half_edge: half_edge.clone(), + } + .into(), + ); } - - Ok(()) } - fn check_global_curve_identity(half_edge: &HalfEdge) -> Result<(), Self> { + fn check_global_curve_identity( + half_edge: &HalfEdge, + errors: &mut Vec, + ) { let global_curve_from_curve = half_edge.curve().global_form(); let global_curve_from_global_form = half_edge.global_form().curve(); if global_curve_from_curve.id() != global_curve_from_global_form.id() { - return Err(Self::GlobalCurveMismatch { - global_curve_from_curve: global_curve_from_curve.clone(), - global_curve_from_global_form: global_curve_from_global_form - .clone(), - half_edge: half_edge.clone(), - }); + errors.push( + Self::GlobalCurveMismatch { + global_curve_from_curve: global_curve_from_curve.clone(), + global_curve_from_global_form: + global_curve_from_global_form.clone(), + half_edge: half_edge.clone(), + } + .into(), + ); } - - Ok(()) } - fn check_global_vertex_identity(half_edge: &HalfEdge) -> Result<(), Self> { + fn check_global_vertex_identity( + half_edge: &HalfEdge, + errors: &mut Vec, + ) { let global_vertices_from_vertices = { let (global_vertices_from_vertices, _) = VerticesInNormalizedOrder::new( @@ -183,35 +187,38 @@ impl HalfEdgeValidationError { .map(|global_vertex| global_vertex.id()); if ids_from_vertices != ids_from_global_form { - return Err(Self::GlobalVertexMismatch { - global_vertices_from_vertices, - global_vertices_from_global_form, - half_edge: half_edge.clone(), - }); + errors.push( + Self::GlobalVertexMismatch { + global_vertices_from_vertices, + global_vertices_from_global_form, + half_edge: half_edge.clone(), + } + .into(), + ); } - - Ok(()) } fn check_vertex_positions( half_edge: &HalfEdge, config: &ValidationConfig, - ) -> Result<(), Self> { + errors: &mut Vec, + ) { let back_position = half_edge.back().position(); let front_position = half_edge.front().position(); let distance = (back_position - front_position).magnitude(); if distance < config.distinct_min_distance { - return Err(Self::VerticesAreCoincident { - back_position, - front_position, - distance, - half_edge: half_edge.clone(), - }); + errors.push( + Self::VerticesAreCoincident { + back_position, + front_position, + distance, + half_edge: half_edge.clone(), + } + .into(), + ); } - - Ok(()) } } @@ -254,8 +261,8 @@ mod tests { HalfEdge::new(vertices, valid.global_form().clone()) }; - valid.validate()?; - assert!(invalid.validate().is_err()); + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); Ok(()) } @@ -280,8 +287,8 @@ mod tests { tmp.build(&mut services.objects) }); - valid.validate()?; - assert!(invalid.validate().is_err()); + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); Ok(()) } @@ -312,8 +319,8 @@ mod tests { tmp.build(&mut services.objects) }); - valid.validate()?; - assert!(invalid.validate().is_err()); + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); Ok(()) } @@ -349,8 +356,8 @@ mod tests { valid.global_form().clone(), ); - valid.validate()?; - assert!(invalid.validate().is_err()); + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); Ok(()) } diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 2aabcda3ea..b3f3dc5c64 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -5,18 +5,16 @@ use crate::{ storage::Handle, }; -use super::{Validate, ValidationConfig}; +use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Face { - type Error = FaceValidationError; - fn validate_with_config( &self, _: &ValidationConfig, - ) -> Result<(), Self::Error> { - FaceValidationError::check_surface_identity(self)?; - FaceValidationError::check_interior_winding(self)?; - Ok(()) + errors: &mut Vec, + ) { + FaceValidationError::check_surface_identity(self, errors); + FaceValidationError::check_interior_winding(self, errors); } } @@ -61,43 +59,40 @@ pub enum FaceValidationError { } impl FaceValidationError { - fn check_surface_identity(face: &Face) -> Result<(), Self> { + fn check_surface_identity(face: &Face, errors: &mut Vec) { let surface = face.surface(); for interior in face.interiors() { if surface.id() != interior.surface().id() { - return Err(Self::SurfaceMismatch { - surface: surface.clone(), - interior: interior.clone(), - face: face.clone(), - }); + errors.push( + Self::SurfaceMismatch { + surface: surface.clone(), + interior: interior.clone(), + face: face.clone(), + } + .into(), + ); } } - - Ok(()) } - fn check_interior_winding(face: &Face) -> Result<(), Self> { + fn check_interior_winding(face: &Face, errors: &mut Vec) { let exterior_winding = face.exterior().winding(); for interior in face.interiors() { let interior_winding = interior.winding(); if exterior_winding == interior_winding { - return Err(Self::InvalidInteriorWinding { - exterior_winding, - interior_winding, - face: face.clone(), - }); + errors.push( + Self::InvalidInteriorWinding { + exterior_winding, + interior_winding, + face: face.clone(), + } + .into(), + ); } - assert_ne!( - exterior_winding, - interior.winding(), - "Interior cycles must have opposite winding of exterior cycle" - ); } - - Ok(()) } } @@ -149,8 +144,8 @@ mod tests { Face::new(valid.exterior().clone(), interiors, valid.color()) }; - valid.validate()?; - assert!(invalid.validate().is_err()); + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); Ok(()) } @@ -186,8 +181,8 @@ mod tests { Face::new(valid.exterior().clone(), interiors, valid.color()) }; - valid.validate()?; - assert!(invalid.validate().is_err()); + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); Ok(()) } diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 4cb607f72a..3539962af2 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -25,19 +25,29 @@ use fj_math::Scalar; /// /// This trait is used automatically when inserting an object into a store. pub trait Validate: Sized { - /// The error that validation of the implementing type can result in - type Error: Into; + /// Validate the object using default config and return on first error + fn validate_and_return_first_error(&self) -> Result<(), ValidationError> { + 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<(), Self::Error> { - self.validate_with_config(&ValidationConfig::default()) + fn validate(&self, errors: &mut Vec) { + self.validate_with_config(&ValidationConfig::default(), errors) } /// Validate the object fn validate_with_config( &self, config: &ValidationConfig, - ) -> Result<(), Self::Error>; + errors: &mut Vec, + ); } /// Configuration required for the validation process diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index e71626fada..f6603ad551 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -1,16 +1,12 @@ -use std::convert::Infallible; - use crate::objects::Shell; -use super::{Validate, ValidationConfig}; +use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Shell { - type Error = Infallible; - fn validate_with_config( &self, _: &ValidationConfig, - ) -> Result<(), Self::Error> { - Ok(()) + _: &mut Vec, + ) { } } diff --git a/crates/fj-kernel/src/validate/sketch.rs b/crates/fj-kernel/src/validate/sketch.rs index 1a1896658e..d3eefbebed 100644 --- a/crates/fj-kernel/src/validate/sketch.rs +++ b/crates/fj-kernel/src/validate/sketch.rs @@ -1,16 +1,12 @@ -use std::convert::Infallible; - use crate::objects::Sketch; -use super::{Validate, ValidationConfig}; +use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Sketch { - type Error = Infallible; - fn validate_with_config( &self, _: &ValidationConfig, - ) -> Result<(), Self::Error> { - Ok(()) + _: &mut Vec, + ) { } } diff --git a/crates/fj-kernel/src/validate/solid.rs b/crates/fj-kernel/src/validate/solid.rs index 9e484cb4b2..e2982bd6a2 100644 --- a/crates/fj-kernel/src/validate/solid.rs +++ b/crates/fj-kernel/src/validate/solid.rs @@ -1,16 +1,12 @@ -use std::convert::Infallible; - use crate::objects::Solid; -use super::{Validate, ValidationConfig}; +use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Solid { - type Error = Infallible; - fn validate_with_config( &self, _: &ValidationConfig, - ) -> Result<(), Self::Error> { - Ok(()) + _: &mut Vec, + ) { } } diff --git a/crates/fj-kernel/src/validate/surface.rs b/crates/fj-kernel/src/validate/surface.rs index 8c19987879..4462f91470 100644 --- a/crates/fj-kernel/src/validate/surface.rs +++ b/crates/fj-kernel/src/validate/surface.rs @@ -1,16 +1,12 @@ -use std::convert::Infallible; - use crate::objects::Surface; -use super::{Validate, ValidationConfig}; +use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Surface { - type Error = Infallible; - fn validate_with_config( &self, _: &ValidationConfig, - ) -> Result<(), Self::Error> { - Ok(()) + _: &mut Vec, + ) { } } diff --git a/crates/fj-kernel/src/validate/vertex.rs b/crates/fj-kernel/src/validate/vertex.rs index 6fa0163632..97617b087f 100644 --- a/crates/fj-kernel/src/validate/vertex.rs +++ b/crates/fj-kernel/src/validate/vertex.rs @@ -1,5 +1,3 @@ -use std::convert::Infallible; - use fj_math::{Point, Scalar}; use crate::{ @@ -7,41 +5,35 @@ use crate::{ storage::Handle, }; -use super::{Validate, ValidationConfig}; +use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Vertex { - type Error = VertexValidationError; - fn validate_with_config( &self, config: &ValidationConfig, - ) -> Result<(), Self::Error> { - VertexValidationError::check_surface_identity(self)?; - VertexValidationError::check_position(self, config)?; - Ok(()) + errors: &mut Vec, + ) { + VertexValidationError::check_surface_identity(self, errors); + VertexValidationError::check_position(self, config, errors); } } impl Validate for SurfaceVertex { - type Error = SurfaceVertexValidationError; - fn validate_with_config( &self, config: &ValidationConfig, - ) -> Result<(), Self::Error> { - SurfaceVertexValidationError::check_position(self, config)?; - Ok(()) + errors: &mut Vec, + ) { + SurfaceVertexValidationError::check_position(self, config, errors); } } impl Validate for GlobalVertex { - type Error = Infallible; - fn validate_with_config( &self, _: &ValidationConfig, - ) -> Result<(), Self::Error> { - Ok(()) + _: &mut Vec, + ) { } } @@ -86,24 +78,29 @@ pub enum VertexValidationError { } impl VertexValidationError { - fn check_surface_identity(vertex: &Vertex) -> Result<(), Self> { + fn check_surface_identity( + vertex: &Vertex, + errors: &mut Vec, + ) { let curve_surface = vertex.curve().surface(); let surface_form_surface = vertex.surface_form().surface(); if curve_surface.id() != surface_form_surface.id() { - return Err(Self::SurfaceMismatch { - curve_surface: curve_surface.clone(), - surface_form_surface: surface_form_surface.clone(), - }); + errors.push( + Self::SurfaceMismatch { + curve_surface: curve_surface.clone(), + surface_form_surface: surface_form_surface.clone(), + } + .into(), + ); } - - Ok(()) } fn check_position( vertex: &Vertex, config: &ValidationConfig, - ) -> Result<(), Self> { + errors: &mut Vec, + ) { let curve_position_as_surface = vertex .curve() .path() @@ -113,15 +110,16 @@ impl VertexValidationError { let distance = curve_position_as_surface.distance_to(&surface_position); if distance > config.identical_max_distance { - return Err(Self::PositionMismatch { - vertex: vertex.clone(), - surface_vertex: vertex.surface_form().clone_object(), - curve_position_as_surface, - distance, - }); + errors.push( + Self::PositionMismatch { + vertex: vertex.clone(), + surface_vertex: vertex.surface_form().clone_object(), + curve_position_as_surface, + distance, + } + .into(), + ); } - - Ok(()) } } @@ -155,7 +153,8 @@ impl SurfaceVertexValidationError { fn check_position( surface_vertex: &SurfaceVertex, config: &ValidationConfig, - ) -> Result<(), Self> { + errors: &mut Vec, + ) { let surface_position_as_global = surface_vertex .surface() .geometry() @@ -165,15 +164,16 @@ impl SurfaceVertexValidationError { let distance = surface_position_as_global.distance_to(&global_position); if distance > config.identical_max_distance { - return Err(Self::PositionMismatch { - surface_vertex: surface_vertex.clone(), - global_vertex: surface_vertex.global_form().clone_object(), - surface_position_as_global, - distance, - }); + errors.push( + Self::PositionMismatch { + surface_vertex: surface_vertex.clone(), + global_vertex: surface_vertex.global_form().clone_object(), + surface_position_as_global, + distance, + } + .into(), + ); } - - Ok(()) } } @@ -220,8 +220,8 @@ mod tests { Vertex::new(valid.position(), valid.curve().clone(), surface_form) }; - valid.validate()?; - assert!(invalid.validate().is_err()); + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); Ok(()) } @@ -257,8 +257,8 @@ mod tests { Vertex::new(valid.position(), valid.curve().clone(), surface_form) }; - valid.validate()?; - assert!(invalid.validate().is_err()); + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); Ok(()) } @@ -279,8 +279,8 @@ mod tests { GlobalVertex::new([1., 0., 0.]).insert(&mut services.objects), ); - valid.validate()?; - assert!(invalid.validate().is_err()); + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); Ok(()) }