diff --git a/Cargo.toml b/Cargo.toml index 62a9cea60..0132c9cb5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,6 +51,12 @@ categories = ["encoding", "mathematics", "rendering"] [workspace.lints.rust] missing_docs = "warn" +[workspace.lints.clippy] +# I really don't see any point in this. It can make a public API ugly, but +# a) that will be obvious, even without a lint, and +# b) it provides benefits in private APIs with top-level re-exports. +module_inception = "allow" + [workspace.dependencies.fj] version = "0.48.0" diff --git a/crates/fj-core/src/layers/layers.rs b/crates/fj-core/src/layers/layers.rs new file mode 100644 index 000000000..2682537a8 --- /dev/null +++ b/crates/fj-core/src/layers/layers.rs @@ -0,0 +1,49 @@ +use crate::{ + objects::Objects, + validate::{Validation, ValidationConfig}, +}; + +use super::Layer; + +/// # Loosely coupled layers, that together define shapes +/// +/// Shapes are not a monolithic thing in Fornjot, but instead are defined by +/// several, loosely coupled layers. These layers are owned by this struct. +/// +/// ## Implementation Note +/// +/// It is totally conceivable that one day, this system of layers is extensible +/// and more layers can be defined by third-party code. The foundation for that, +/// the loose coupling and inter-layer communication via events, is already +/// there, conceptually. +/// +/// For now, there is no need for this, and all layers are just hardcoded here. +/// That can be changed, once necessary. +#[derive(Default)] +pub struct Layers { + /// The objects layers + /// + /// Manages the stores of topological and geometric objects that make up + /// shapes. + pub objects: Layer, + + /// The validation layer + /// + /// Monitors objects and validates them, as they are inserted. + pub validation: Layer, +} + +impl Layers { + /// Construct an instance of `Layers` + pub fn new() -> Self { + Self::default() + } + + /// Construct an instance of `Layers`, using the provided configuration + pub fn with_validation_config(config: ValidationConfig) -> Self { + Self { + validation: Layer::new(Validation::with_validation_config(config)), + ..Default::default() + } + } +} diff --git a/crates/fj-core/src/layers/mod.rs b/crates/fj-core/src/layers/mod.rs index 780f1a15a..953ff94f3 100644 --- a/crates/fj-core/src/layers/mod.rs +++ b/crates/fj-core/src/layers/mod.rs @@ -2,88 +2,13 @@ //! //! See [`Layers`]. -mod layer; -mod objects; -mod validation; +pub mod objects; +pub mod validation; -use crate::{ - objects::{AboutToBeStored, AnyObject, Objects}, - validate::{ValidationConfig, ValidationErrors}, -}; +mod layer; +mod layers; pub use self::{ layer::{Layer, State}, - objects::{InsertObject, Operation}, - validation::{Validation, ValidationCommand, ValidationEvent}, + layers::Layers, }; - -/// # Loosely coupled layers, that together define shapes -/// -/// Shapes are not a monolithic thing in Fornjot, but instead are defined by -/// several, loosely coupled layers. These layers are owned by this struct. -/// -/// ## Implementation Note -/// -/// It is totally conceivable that one day, this system of layers is extensible -/// and more layers can be defined by third-party code. The foundation for that, -/// the loose coupling and inter-layer communication via events, is already -/// there, conceptually. -/// -/// For now, there is no need for this, and all layers are just hardcoded here. -/// That can be changed, once necessary. -#[derive(Default)] -pub struct Layers { - /// The objects layers - /// - /// Manages the stores of topological and geometric objects that make up - /// shapes. - pub objects: Layer, - - /// The validation layer - /// - /// Monitors objects and validates them, as they are inserted. - pub validation: Layer, -} - -impl Layers { - /// Construct an instance of `Layers` - pub fn new() -> Self { - Self::default() - } - - /// Construct an instance of `Layers`, using the provided configuration - pub fn with_validation_config(config: ValidationConfig) -> Self { - let objects = Layer::default(); - let validation = Layer::new(Validation::with_validation_config(config)); - - Self { - objects, - validation, - } - } - - /// Insert an object into the stores - pub fn insert_object(&mut self, object: AnyObject) { - let mut object_events = Vec::new(); - self.objects - .process(Operation::InsertObject { object }, &mut object_events); - - for object_event in object_events { - let command = ValidationCommand::ValidateObject { - object: object_event.object.into(), - }; - self.validation.process(command, &mut Vec::new()); - } - } - - /// Drop `Layers`; return any unhandled validation error - pub fn drop_and_validate(self) -> Result<(), ValidationErrors> { - let errors = self.validation.into_state().into_errors(); - - if errors.0.is_empty() { - Ok(()) - } else { - Err(errors) - } - } -} diff --git a/crates/fj-core/src/layers/objects.rs b/crates/fj-core/src/layers/objects.rs index 916a7ff13..4d6828ab4 100644 --- a/crates/fj-core/src/layers/objects.rs +++ b/crates/fj-core/src/layers/objects.rs @@ -1,24 +1,46 @@ -use crate::objects::{AboutToBeStored, AnyObject, Objects}; +//! Layer infrastructure for [`Objects`] -use super::State; +use crate::{ + objects::{AboutToBeStored, AnyObject, Objects}, + validate::Validation, +}; + +use super::{Layer, State}; + +impl Layer { + /// Insert and object into the stores + pub fn insert( + &mut self, + object: AnyObject, + validation: &mut Layer, + ) { + let mut events = Vec::new(); + self.process(ObjectsCommand::InsertObject { object }, &mut events); + + for event in events { + validation.on_objects_event(event); + } + } +} impl State for Objects { - type Command = Operation; - type Event = InsertObject; + type Command = ObjectsCommand; + type Event = ObjectsEvent; fn decide(&self, command: Self::Command, events: &mut Vec) { - let Operation::InsertObject { object } = command; - events.push(InsertObject { object }); + let ObjectsCommand::InsertObject { object } = command; + events.push(ObjectsEvent::InsertObject { object }); } fn evolve(&mut self, event: &Self::Event) { - event.object.clone().insert(self); + let ObjectsEvent::InsertObject { object } = event; + object.clone().insert(self); } } /// Command for `Layer` #[derive(Debug)] -pub enum Operation { +pub enum ObjectsCommand { /// Insert an object into the stores /// /// This is the one primitive operation that all other operations are built @@ -31,7 +53,10 @@ pub enum Operation { /// Event produced by `Layer` #[derive(Clone, Debug)] -pub struct InsertObject { - /// The object to insert - pub object: AnyObject, +pub enum ObjectsEvent { + /// Insert an object into the stores + InsertObject { + /// The object to insert + object: AnyObject, + }, } diff --git a/crates/fj-core/src/layers/validation.rs b/crates/fj-core/src/layers/validation.rs index a9e28bb25..b826045e4 100644 --- a/crates/fj-core/src/layers/validation.rs +++ b/crates/fj-core/src/layers/validation.rs @@ -1,62 +1,30 @@ -use std::{collections::HashMap, error::Error, thread}; +//! Layer infrastructure for [`Validation`] use crate::{ objects::{AnyObject, Stored}, - storage::ObjectId, - validate::{ValidationConfig, ValidationError, ValidationErrors}, + validate::{Validation, ValidationError, ValidationErrors}, }; -use super::State; +use super::{objects::ObjectsEvent, Layer, State}; -/// Errors that occurred while validating the objects inserted into the stores -#[derive(Default)] -pub struct Validation { - /// All unhandled validation errors - errors: HashMap, - - /// Validation configuration for the validation service - 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 Layer { + /// Handler for [`ObjectsEvent`] + pub fn on_objects_event(&mut self, event: ObjectsEvent) { + let ObjectsEvent::InsertObject { object } = event; + let command = ValidationCommand::ValidateObject { + object: object.into(), + }; + self.process(command, &mut Vec::new()); } - /// Drop this instance, returning the errors it contained - pub fn into_errors(mut self) -> ValidationErrors { - ValidationErrors(self.errors.drain().map(|(_, error)| error).collect()) - } -} - -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:" - ); + /// Consume the validation layer, returning any validation errors + pub fn into_result(self) -> Result<(), ValidationErrors> { + let errors = self.into_state().into_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!(); - } + if errors.0.is_empty() { + Ok(()) + } else { + Err(errors) } } } diff --git a/crates/fj-core/src/operations/insert/insert_trait.rs b/crates/fj-core/src/operations/insert/insert_trait.rs index 1e683a1c3..3ec696231 100644 --- a/crates/fj-core/src/operations/insert/insert_trait.rs +++ b/crates/fj-core/src/operations/insert/insert_trait.rs @@ -40,7 +40,10 @@ macro_rules! impl_insert { fn insert(self, core: &mut Instance) -> Self::Inserted { let handle = core.layers.objects.$store.reserve(); let object = (handle.clone(), self).into(); - core.layers.insert_object(object); + core.layers.objects.insert( + object, + &mut core.layers.validation, + ); handle } } diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index 9a0374458..178bf79d2 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -76,13 +76,17 @@ mod solid; mod surface; mod vertex; +use crate::storage::ObjectId; + pub use self::{ cycle::CycleValidationError, edge::EdgeValidationError, face::FaceValidationError, shell::ShellValidationError, sketch::SketchValidationError, solid::SolidValidationError, }; -use std::{convert::Infallible, fmt}; +use std::{ + collections::HashMap, convert::Infallible, error::Error, fmt, thread, +}; use fj_math::Scalar; @@ -99,6 +103,59 @@ 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, + + /// 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 } + } + + /// Drop this instance, returning the errors it contained + pub fn into_errors(mut self) -> ValidationErrors { + ValidationErrors(self.errors.drain().map(|(_, error)| error).collect()) + } +} + +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. diff --git a/crates/fj-core/src/validate/solid.rs b/crates/fj-core/src/validate/solid.rs index 21180e992..05366f644 100644 --- a/crates/fj-core/src/validate/solid.rs +++ b/crates/fj-core/src/validate/solid.rs @@ -236,7 +236,7 @@ mod tests { valid_solid.validate_and_return_first_error()?; // Ignore remaining validation errors. - let _ = core.layers.drop_and_validate(); + let _ = core.layers.validation.into_result(); Ok(()) } @@ -289,7 +289,7 @@ mod tests { valid_solid.validate_and_return_first_error()?; // Ignore remaining validation errors. - let _ = core.layers.drop_and_validate(); + let _ = core.layers.validation.into_result(); Ok(()) } @@ -339,7 +339,7 @@ mod tests { valid_solid.validate_and_return_first_error()?; // Ignore remaining validation errors. - let _ = core.layers.drop_and_validate(); + let _ = core.layers.validation.into_result(); Ok(()) } @@ -379,7 +379,7 @@ mod tests { valid_solid.validate_and_return_first_error()?; // Ignore remaining validation errors. - let _ = core.layers.drop_and_validate(); + let _ = core.layers.validation.into_result(); Ok(()) } diff --git a/crates/fj/src/handle_model.rs b/crates/fj/src/handle_model.rs index e029221f1..280c2abf0 100644 --- a/crates/fj/src/handle_model.rs +++ b/crates/fj/src/handle_model.rs @@ -1,4 +1,4 @@ -use std::{error::Error as _, fmt, mem}; +use std::{error::Error as _, fmt}; use fj_core::{ algorithms::{ @@ -35,10 +35,8 @@ where let args = Args::parse(); - if args.ignore_validation { - mem::forget(core); - } else { - core.layers.drop_and_validate()?; + if !args.ignore_validation { + core.layers.validation.into_result()?; } let aabb = model.aabb().unwrap_or(Aabb {