Merge pull request #2229 from hannobraun/validation

Start cleanup of validation infrastructure

crates/fj-core/src/algorithms/approx/face.rs

@@ -9,7 +9,7 @@ use fj_interop::Color;
 use crate::{
     objects::{Face, Handedness, ObjectSet},
     operations::presentation::GetColor,
-    validate::ValidationConfig,
+    validation::ValidationConfig,
     Core,
 };
 

crates/fj-core/src/core.rs

@@ -2,7 +2,7 @@
 //!
 //! See [`Core`].
 
-use crate::{layers::Layers, validate::ValidationConfig};
+use crate::{layers::Layers, validation::ValidationConfig};
 
 /// An instance of the Fornjot core
 ///

crates/fj-core/src/layers/layers.rs

@@ -1,7 +1,7 @@
 use crate::{
     objects::Objects,
     presentation::Presentation,
-    validate::{Validation, ValidationConfig},
+    validation::{Validation, ValidationConfig},
 };
 
 use super::Layer;

crates/fj-core/src/layers/objects.rs

@@ -2,7 +2,7 @@
 
 use crate::{
     objects::{AboutToBeStored, AnyObject, Objects},
-    validate::Validation,
+    validation::Validation,
 };
 
 use super::{Command, Event, Layer};

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

@@ -2,7 +2,7 @@
 
 use crate::{
     objects::{AnyObject, Stored},
-    validate::{Validation, ValidationError, ValidationErrors},
+    validation::{Validation, ValidationError, ValidationErrors},
 };
 
 use super::{objects::InsertObject, Command, Event, Layer};

crates/fj-core/src/lib.rs

@@ -90,6 +90,7 @@ pub mod presentation;
 pub mod queries;
 pub mod storage;
 pub mod validate;
+pub mod validation;
 
 mod core;
 

crates/fj-core/src/objects/any_object.rs

@@ -4,7 +4,8 @@ use crate::{
         Surface, Vertex,
     },
     storage::{Handle, HandleWrapper, ObjectId},
-    validate::{Validate, ValidationConfig, ValidationError},
+    validate::Validate,
+    validation::{ValidationConfig, ValidationError},
 };
 
 macro_rules! any_object {

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

@@ -1,6 +1,9 @@
-use crate::objects::Curve;
+use crate::{
+    objects::Curve,
+    validation::{ValidationConfig, ValidationError},
+};
 
-use super::{Validate, ValidationConfig, ValidationError};
+use super::Validate;
 
 impl Validate for Curve {
     fn validate_with_config(

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

@@ -3,9 +3,10 @@ use fj_math::{Point, Scalar};
 use crate::{
     objects::{Cycle, HalfEdge},
     storage::Handle,
+    validation::{ValidationConfig, ValidationError},
 };
 
-use super::{Validate, ValidationConfig, ValidationError};
+use super::Validate;
 
 impl Validate for Cycle {
     fn validate_with_config(
@@ -83,7 +84,8 @@ mod tests {
             build::{BuildCycle, BuildHalfEdge},
             update::UpdateCycle,
         },
-        validate::{cycle::CycleValidationError, Validate, ValidationError},
+        validate::{cycle::CycleValidationError, Validate},
+        validation::ValidationError,
         Core,
     };
 

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

@@ -1,8 +1,11 @@
 use fj_math::{Point, Scalar};
 
-use crate::objects::HalfEdge;
+use crate::{
+    objects::HalfEdge,
+    validation::{ValidationConfig, ValidationError},
+};
 
-use super::{Validate, ValidationConfig, ValidationError};
+use super::Validate;
 
 impl Validate for HalfEdge {
     fn validate_with_config(
@@ -70,7 +73,8 @@ mod tests {
         assert_contains_err,
         objects::HalfEdge,
         operations::build::BuildHalfEdge,
-        validate::{EdgeValidationError, Validate, ValidationError},
+        validate::{EdgeValidationError, Validate},
+        validation::ValidationError,
         Core,
     };
 

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

@@ -1,8 +1,11 @@
 use fj_math::Winding;
 
-use crate::objects::Face;
+use crate::{
+    objects::Face,
+    validation::{ValidationConfig, ValidationError},
+};
 
-use super::{Validate, ValidationConfig, ValidationError};
+use super::Validate;
 
 impl Validate for Face {
     fn validate_with_config(
@@ -94,7 +97,8 @@ mod tests {
             reverse::Reverse,
             update::{UpdateCycle, UpdateFace, UpdateRegion},
         },
-        validate::{FaceValidationError, Validate, ValidationError},
+        validate::{FaceValidationError, Validate},
+        validation::ValidationError,
         Core,
     };
 

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

@@ -1,14 +1,6 @@
 //! # Infrastructure for validating objects
 //!
-//! ## Structure and Nomenclature
-//!
-//! **Validation** is the process of checking that objects meet specific
-//! requirements. Each kind of object has its own set of requirements.
-//!
-//! An object that meets all the requirement for its kind is considered
-//! **valid**. An object that does not meet all of them is considered
-//! **invalid**. This results in a **validation error**, which is represented by
-//! [`ValidationError`].
+//! ## Structure
 //!
 //! Every single requirement is checked by a dedicated function. These functions
 //! are called **validation checks**. Validation checks are currently not
@@ -60,6 +52,11 @@
 //! [`Validate::validate_with_config`] and [`ValidationConfig`].
 //!
 //!
+//! ## Implementation Note
+//!
+//! This module is in the process of being replaced. See [`crate::validation`].
+//!
+//!
 //! [`fj-export`]: https://crates.io/crates/fj-export
 //! [issue tracker]: https://github.com/hannobraun/fornjot/issues
 //! [`Layers`]: crate::layers::Layers
@@ -76,20 +73,14 @@ mod solid;
 mod surface;
 mod vertex;
 
-use crate::storage::ObjectId;
+use crate::validation::{ValidationConfig, ValidationError};
 
 pub use self::{
     cycle::CycleValidationError, edge::EdgeValidationError,
     face::FaceValidationError, shell::ShellValidationError,
     sketch::SketchValidationError, solid::SolidValidationError,
 };
 
-use std::{
-    collections::HashMap, convert::Infallible, error::Error, fmt, thread,
-};
-
-use fj_math::Scalar;
-
 /// 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.
 #[macro_export]
@@ -103,54 +94,6 @@ macro_rules! assert_contains_err {
     };
 }
 
-/// Errors that occurred while validating the objects inserted into the stores
-#[derive(Default)]
-pub struct Validation {
-    /// All unhandled validation errors
-    pub errors: HashMap<ObjectId, ValidationError>,
-
-    /// Validation configuration for the validation service
-    pub config: ValidationConfig,
-}
-
-impl Validation {
-    /// Construct an instance of `Validation`, using the provided configuration
-    pub fn with_validation_config(config: ValidationConfig) -> Self {
-        let errors = HashMap::new();
-        Self { errors, config }
-    }
-}
-
-impl Drop for Validation {
-    fn drop(&mut self) {
-        let num_errors = self.errors.len();
-        if num_errors > 0 {
-            println!(
-                "Dropping `Validation` with {num_errors} unhandled validation \
-                errors:"
-            );
-
-            for err in self.errors.values() {
-                println!("{}", err);
-
-                // Once `Report` is stable, we can replace this:
-                // https://doc.rust-lang.org/std/error/struct.Report.html
-                let mut source = err.source();
-                while let Some(err) = source {
-                    println!("\nCaused by:\n\t{err}");
-                    source = err.source();
-                }
-
-                print!("\n\n");
-            }
-
-            if !thread::panicking() {
-                panic!();
-            }
-        }
-    }
-}
-
 /// Validate an object
 ///
 /// This trait is used automatically when inserting an object into a store.
@@ -180,87 +123,3 @@ pub trait Validate: Sized {
         errors: &mut Vec<ValidationError>,
     );
 }
-
-/// Configuration required for the validation process
-#[derive(Debug, Clone, Copy)]
-pub struct ValidationConfig {
-    /// The minimum distance between distinct objects
-    ///
-    /// Objects whose distance is less than the value defined in this field, are
-    /// considered identical.
-    pub distinct_min_distance: Scalar,
-
-    /// The maximum distance between identical objects
-    ///
-    /// Objects that are considered identical might still have a distance
-    /// between them, due to inaccuracies of the numerical representation. If
-    /// that distance is less than the one defined in this field, can not be
-    /// considered identical.
-    pub identical_max_distance: Scalar,
-}
-
-impl Default for ValidationConfig {
-    fn default() -> Self {
-        Self {
-            distinct_min_distance: Scalar::from_f64(5e-7), // 0.5 µm,
-
-            // This value was chosen pretty arbitrarily. Seems small enough to
-            // catch errors. If it turns out it's too small (because it produces
-            // false positives due to floating-point accuracy issues), we can
-            // adjust it.
-            identical_max_distance: Scalar::from_f64(5e-14),
-        }
-    }
-}
-
-/// An error that can occur during a validation
-#[derive(Clone, Debug, thiserror::Error)]
-pub enum ValidationError {
-    /// `Cycle` validation error
-    #[error("`Cycle` validation error")]
-    Cycle(#[from] CycleValidationError),
-
-    /// `Edge` validation error
-    #[error("`Edge` validation error")]
-    Edge(#[from] EdgeValidationError),
-
-    /// `Face` validation error
-    #[error("`Face` validation error")]
-    Face(#[from] FaceValidationError),
-
-    /// `Shell` validation error
-    #[error("`Shell` validation error")]
-    Shell(#[from] ShellValidationError),
-
-    /// `Solid` validation error
-    #[error("`Solid` validation error")]
-    Solid(#[from] SolidValidationError),
-
-    /// `Sketch` validation error
-    #[error("`Sketch` validation error")]
-    Sketch(#[from] SketchValidationError),
-}
-
-impl From<Infallible> for ValidationError {
-    fn from(infallible: Infallible) -> Self {
-        match infallible {}
-    }
-}
-
-/// A collection of validation errors
-#[derive(Debug, thiserror::Error)]
-pub struct ValidationErrors(pub Vec<ValidationError>);
-
-impl fmt::Display for ValidationErrors {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let num_errors = self.0.len();
-
-        writeln!(f, "{num_errors} unhandled validation errors:")?;
-
-        for err in &self.0 {
-            writeln!(f, "{err}")?;
-        }
-
-        Ok(())
-    }
-}

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

@@ -0,0 +1,33 @@
+use fj_math::Scalar;
+
+/// Configuration required for the validation process
+#[derive(Debug, Clone, Copy)]
+pub struct ValidationConfig {
+    /// The minimum distance between distinct objects
+    ///
+    /// Objects whose distance is less than the value defined in this field, are
+    /// considered identical.
+    pub distinct_min_distance: Scalar,
+
+    /// The maximum distance between identical objects
+    ///
+    /// Objects that are considered identical might still have a distance
+    /// between them, due to inaccuracies of the numerical representation. If
+    /// that distance is less than the one defined in this field, can not be
+    /// considered identical.
+    pub identical_max_distance: Scalar,
+}
+
+impl Default for ValidationConfig {
+    fn default() -> Self {
+        Self {
+            distinct_min_distance: Scalar::from_f64(5e-7), // 0.5 µm,
+
+            // This value was chosen pretty arbitrarily. Seems small enough to
+            // catch errors. If it turns out it's too small (because it produces
+            // false positives due to floating-point accuracy issues), we can
+            // adjust it.
+            identical_max_distance: Scalar::from_f64(5e-14),
+        }
+    }
+}