Merge pull request #1311 from hannobraun/host

Make some clean-ups in internal `fj` code

crates/fj/src/models/context.rs

@@ -1,215 +1,10 @@
-use std::{
-    collections::HashMap,
-    fmt::{self, Display, Formatter},
-    str::FromStr,
-};
-
-use crate::models::Error;
-
 /// Contextual information passed to a [`Model`][crate::models::Model] when it
 /// is being initialized.
-///
-/// Check out the [`ContextExt`] trait for some helper methods.
 pub trait Context {
     /// Get an argument that was passed to this model.
     fn get_argument(&self, name: &str) -> Option<&str>;
 }
 
-impl<C: Context + ?Sized> Context for &'_ C {
-    fn get_argument(&self, name: &str) -> Option<&str> {
-        (**self).get_argument(name)
-    }
-}
-
-impl<C: Context + ?Sized> Context for Box<C> {
-    fn get_argument(&self, name: &str) -> Option<&str> {
-        (**self).get_argument(name)
-    }
-}
-
-impl<C: Context + ?Sized> Context for std::rc::Rc<C> {
-    fn get_argument(&self, name: &str) -> Option<&str> {
-        (**self).get_argument(name)
-    }
-}
-
-impl<C: Context + ?Sized> Context for std::sync::Arc<C> {
-    fn get_argument(&self, name: &str) -> Option<&str> {
-        (**self).get_argument(name)
-    }
-}
-
-impl Context for HashMap<String, String> {
-    fn get_argument(&self, name: &str) -> Option<&str> {
-        self.get(name).map(|s| s.as_str())
-    }
-}
-
-/// Extension methods for the [`Context`] type.
-///
-/// By splitting these methods out into a separate trait, [`Context`] can stay
-/// object-safe while allowing convenience methods that use generics.
-pub trait ContextExt {
-    /// Get an argument, returning a [`MissingArgument`] error if it doesn't
-    /// exist.
-    fn get_required_argument(
-        &self,
-        name: &str,
-    ) -> Result<&str, MissingArgument>;
-
-    /// Parse an argument from its string representation using [`FromStr`].
-    fn parse_argument<T>(&self, name: &str) -> Result<T, ContextError>
-    where
-        T: FromStr,
-        T::Err: std::error::Error + Send + Sync + 'static;
-
-    /// Try to parse an argument, if it is present.
-    fn parse_optional_argument<T>(
-        &self,
-        name: &str,
-    ) -> Result<Option<T>, ParseFailed>
-    where
-        T: FromStr,
-        T::Err: std::error::Error + Send + Sync + 'static;
-}
-
-impl<C: Context + ?Sized> ContextExt for C {
-    fn get_required_argument(
-        &self,
-        name: &str,
-    ) -> Result<&str, MissingArgument> {
-        self.get_argument(name).ok_or_else(|| MissingArgument {
-            name: name.to_string(),
-        })
-    }
-
-    fn parse_argument<T>(&self, name: &str) -> Result<T, ContextError>
-    where
-        T: FromStr,
-        T::Err: std::error::Error + Send + Sync + 'static,
-    {
-        let value = self.get_required_argument(name)?;
-
-        value
-            .parse()
-            .map_err(|e| ParseFailed {
-                name: name.to_string(),
-                value: value.to_string(),
-                error: Box::new(e),
-            })
-            .map_err(ContextError::from)
-    }
-
-    fn parse_optional_argument<T>(
-        &self,
-        name: &str,
-    ) -> Result<Option<T>, ParseFailed>
-    where
-        T: FromStr,
-        T::Err: std::error::Error + Send + Sync + 'static,
-    {
-        let value = match self.get_argument(name) {
-            Some(value) => value,
-            None => return Ok(None),
-        };
-
-        let parsed = value.parse().map_err(|e| ParseFailed {
-            name: name.to_string(),
-            value: value.to_string(),
-            error: Box::new(e),
-        })?;
-
-        Ok(Some(parsed))
-    }
-}
-
-/// An error that may be returned from a [`Context`] method.
-#[derive(Debug)]
-pub enum ContextError {
-    /// An argument was missing.
-    MissingArgument(MissingArgument),
-    /// An argument was present, but we were unable to parse it into the final
-    /// type.
-    ParseFailed(ParseFailed),
-}
-
-impl From<MissingArgument> for ContextError {
-    fn from(m: MissingArgument) -> Self {
-        ContextError::MissingArgument(m)
-    }
-}
-
-impl From<ParseFailed> for ContextError {
-    fn from(p: ParseFailed) -> Self {
-        ContextError::ParseFailed(p)
-    }
-}
-
-impl Display for ContextError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        match self {
-            ContextError::MissingArgument(_) => {
-                write!(f, "An argument was missing")
-            }
-            ContextError::ParseFailed(_) => {
-                write!(f, "Unable to parse an argument")
-            }
-        }
-    }
-}
-
-impl std::error::Error for ContextError {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        match self {
-            ContextError::MissingArgument(m) => Some(m),
-            ContextError::ParseFailed(p) => Some(p),
-        }
-    }
-}
-
-/// The error returned when a required argument wasn't provided.
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct MissingArgument {
-    /// The argument's name.
-    pub name: String,
-}
-
-impl Display for MissingArgument {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        let MissingArgument { name } = self;
-
-        write!(f, "The \"{name}\" argument was missing")
-    }
-}
-
-impl std::error::Error for MissingArgument {}
-
-/// The error returned when [`ContextExt::parse_argument()`] is unable to parse
-/// the argument's value.
-#[derive(Debug)]
-pub struct ParseFailed {
-    /// The argument's name.
-    pub name: String,
-    /// The actual value.
-    pub value: String,
-    /// The error that occurred.
-    pub error: Error,
-}
-
-impl Display for ParseFailed {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        let ParseFailed { name, value, .. } = self;
-
-        write!(f, "Unable to parse the \"{name}\" argument (\"{value:?}\")")
-    }
-}
-
-impl std::error::Error for ParseFailed {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        Some(&*self.error)
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

crates/fj/src/models/host.rs

@@ -7,23 +7,12 @@ pub trait Host {
     /// This is mainly for more advanced use cases (e.g. when you need to close
     /// over extra state to load the model). For simpler models, you probably
     /// want to use [`HostExt::register_model()`] instead.
-    #[doc(hidden)]
     fn register_boxed_model(&mut self, model: Box<dyn Model>);
 }
 
-impl<H: Host + ?Sized> Host for &'_ mut H {
-    fn register_boxed_model(&mut self, model: Box<dyn Model>) {
-        (*self).register_boxed_model(model);
-    }
-}
-
-impl<H: Host + ?Sized> Host for Box<H> {
-    fn register_boxed_model(&mut self, model: Box<dyn Model>) {
-        (**self).register_boxed_model(model);
-    }
-}
-
 /// Extension methods to augment the [`Host`] API.
+///
+/// The purpose of this trait is to keep [`Host`] object-safe.
 pub trait HostExt {
     /// Register a model with the Fornjot runtime.
     fn register_model<M>(&mut self, model: M)

crates/fj/src/models/metadata.rs

@@ -4,16 +4,22 @@
 pub struct Metadata {
     /// A short, human-friendly name used to identify this module.
     pub name: String,
+
     /// A semver-compliant version number.
     pub version: String,
+
     /// A short, one-line description.
     pub short_description: Option<String>,
+
     /// A more elaborate description.
     pub description: Option<String>,
+
     /// A link to the homepage.
     pub homepage: Option<String>,
+
     /// A link to the source code.
     pub repository: Option<String>,
+
     /// The name of the software license(s) this software is released under.
     ///
     /// This is interpreted as a SPDX license expression (e.g.  `MIT OR
@@ -120,8 +126,10 @@ impl Metadata {
 pub struct ModelMetadata {
     /// A short, human-friendly name used to identify this model.
     pub name: String,
+
     /// A description of what this model does.
     pub description: Option<String>,
+
     /// Arguments that the model uses when calculating its geometry.
     pub arguments: Vec<ArgumentMetadata>,
 }
@@ -171,9 +179,11 @@ impl ModelMetadata {
 pub struct ArgumentMetadata {
     /// The name used to refer to this argument.
     pub name: String,
+
     /// A short description of this argument that could be shown to the user
     /// in something like a tooltip.
     pub description: Option<String>,
+
     /// Something that could be used as a default if no value was provided.
     pub default_value: Option<String>,
 }

crates/fj/src/models/mod.rs

@@ -6,9 +6,7 @@ mod metadata;
 mod model;
 
 pub use self::{
-    context::{
+    context::Context,
-        Context, ContextError, ContextExt, MissingArgument, ParseFailed,
-    },
     host::{Host, HostExt},
     metadata::{ArgumentMetadata, Metadata, ModelMetadata},
     model::Model,