From cbb6e60314971ae92e1423c4f35c647e5379ebe8 Mon Sep 17 00:00:00 2001
From: Kubat <maelle.martin@proton.me>
Date: Thu, 1 May 2025 19:16:56 +0200
Subject: [PATCH] [AST] Modifies spell arguments to evaluate the expressions at
 runtime, we can't do otherwise.

---
 grimoire/src/ast/expr.rs        |  6 +++
 grimoire/src/error.rs           |  5 +-
 grimoire/src/spell/arguments.rs | 87 ++++++++++++++++-----------------
 grimoire/src/spell/factory.rs   | 27 +++++-----
 grimoire/src/spell/std/log.rs   | 48 +++++++++++-------
 grimoire/src/state.rs           | 30 ++++++------
 6 files changed, 109 insertions(+), 94 deletions(-)

diff --git a/grimoire/src/ast/expr.rs b/grimoire/src/ast/expr.rs
index 8875400..c180dae 100644
--- a/grimoire/src/ast/expr.rs
+++ b/grimoire/src/ast/expr.rs
@@ -39,3 +39,9 @@ pub enum Expression {
     Unary(AstSpan, operator::Unary, Box<Expression>),
     Leaf(AstSpan, VarOrConst),
 }
+
+impl AsRef<Expression> for Expression {
+    fn as_ref(&self) -> &Expression {
+        self
+    }
+}
diff --git a/grimoire/src/error.rs b/grimoire/src/error.rs
index 915f91b..e61a1a0 100644
--- a/grimoire/src/error.rs
+++ b/grimoire/src/error.rs
@@ -6,7 +6,10 @@ use std::{
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
     #[error("already defined spell '{0}': {1}")]
-    Redefined(&'static str, &'static str),
+    RedefinedSpell(&'static str, &'static str),
+
+    #[error("already defined spell argument '{0}'")]
+    RedefinedSpellArgument(String),
 
     #[error("unexpect: {0}")]
     Unexpected(&'static str),
diff --git a/grimoire/src/spell/arguments.rs b/grimoire/src/spell/arguments.rs
index 76e9b98..89cb131 100644
--- a/grimoire/src/spell/arguments.rs
+++ b/grimoire/src/spell/arguments.rs
@@ -1,53 +1,62 @@
-use crate::{ast::Const, error::Error, types::Type};
+use crate::{ast::Expression, error::Error};
 use std::collections::HashMap;
 
 /// The arguments that may be passed to a spell for its instanciation.
 #[derive(Debug, Default)]
 pub struct SpellArguments {
-    named: HashMap<String, Const>,
-    trailing: Vec<Const>,
+    named: HashMap<String, Expression>,
+    trailing: Vec<Expression>,
 }
 
 impl SpellArguments {
-    /// Set/Override the value in the [Argument] list.
-    pub fn insert(&mut self, name: impl Into<String>, value: Const) {
-        _ = self.named.insert(name.into(), value);
+    pub fn new(arguments: impl IntoIterator<Item = (Option<String>, Expression)>) -> Self {
+        let arguments = arguments.into_iter();
+        let mut this = Self::empty_with_capacity({
+            let (min, maybe_max) = arguments.size_hint();
+            maybe_max.unwrap_or(min)
+        });
+
+        arguments.for_each(|(key, expr)| match key {
+            Some(key) => _ = this.named.entry(key).or_insert(expr),
+            None => this.trailing.push(expr),
+        });
+
+        this
     }
 
-    /// Push an unamed argument to the argument list.
-    pub fn push(&mut self, value: Const) {
-        self.trailing.push(value);
-    }
-
-    /// Removes a named argument of a [SpellArguments], and try to cast it into the asked type. On
-    /// success returns [Option<Argument>::Some], otherwise [None].
-    pub fn remove_as<S>(&mut self, name: S, ty: Type) -> Result<Const, Error>
-    where
-        S: AsRef<str>,
-    {
-        self.remove(name.as_ref())
-            .ok_or_else(|| Error::Undefined(name.as_ref().to_string()))?
-            .cast_into(ty)
+    fn empty_with_capacity(capacity: usize) -> Self {
+        Self {
+            named: HashMap::with_capacity(capacity),
+            trailing: Vec::with_capacity(capacity),
+        }
     }
 
     /// Removes a named argument of a [SpellArguments]. On success returns
     /// [Option<Argument>::Some], otherwise [None].
-    pub fn remove<S>(&mut self, name: S) -> Option<Const>
-    where
-        S: AsRef<str>,
-    {
+    pub fn remove(&mut self, name: impl AsRef<str>) -> Option<Expression> {
         self.named.remove(name.as_ref())
     }
 
-    /// Pops an unamed argument from the argument list.
-    pub fn pop_as(&mut self, ty: Type) -> Result<Const, Error> {
-        self.pop()
-            .ok_or(Error::Empty("unamed argument list"))?
-            .cast_into(ty)
+    /// Like [Self::remove], but we make sure that only the passed names are present.
+    pub fn remove_exact<const N: usize>(
+        &mut self,
+        names: [&str; N],
+    ) -> Result<[Expression; N], Error> {
+        if let Some(missing) = names.iter().find(|&&name| !self.named.contains_key(name)) {
+            Err(Error::Undefined(format!(
+                "mendatory spell argument '{missing}'"
+            )))
+        } else if self.named.len() > N {
+            Err(Error::Unexpected(
+                "too many named arguments in in spell arguments",
+            ))
+        } else {
+            Ok(names.map(|name| self.remove(name).unwrap()))
+        }
     }
 
     /// Pops an unamed argument from the argument list.
-    pub fn pop(&mut self) -> Option<Const> {
+    pub fn pop(&mut self) -> Option<Expression> {
         self.trailing.pop()
     }
 
@@ -56,18 +65,6 @@ impl SpellArguments {
         self.named.keys().map(String::as_str)
     }
 
-    /// Iterate over named arguments.
-    pub fn iter_named(&self) -> impl Iterator<Item = (&str, &Const)> {
-        self.named
-            .iter()
-            .map(|(key, constant)| (key.as_str(), constant))
-    }
-
-    /// Iterate over unnamed arguments.
-    pub fn iter_unamed(&self) -> impl Iterator<Item = &Const> {
-        self.trailing.iter()
-    }
-
     /// Tells wether we have named arguments or not.
     pub fn has_named(&self) -> bool {
         self.named.is_empty()
@@ -88,8 +85,8 @@ impl SpellArguments {
 /// Iterate over all the arguments. You are guarentied to have first the named arguments - in an
 /// unspecified order - then the unamed arguments in order.
 pub struct IntoIter {
-    named: <HashMap<String, Const> as IntoIterator>::IntoIter,
-    trailing: <Vec<Const> as IntoIterator>::IntoIter,
+    named: <HashMap<String, Expression> as IntoIterator>::IntoIter,
+    trailing: <Vec<Expression> as IntoIterator>::IntoIter,
 }
 
 impl IntoIterator for SpellArguments {
@@ -107,7 +104,7 @@ impl IntoIterator for SpellArguments {
 }
 
 impl Iterator for IntoIter {
-    type Item = (Option<String>, Const);
+    type Item = (Option<String>, Expression);
 
     fn next(&mut self) -> Option<Self::Item> {
         self.named
diff --git a/grimoire/src/spell/factory.rs b/grimoire/src/spell/factory.rs
index 53dffa8..e9502db 100644
--- a/grimoire/src/spell/factory.rs
+++ b/grimoire/src/spell/factory.rs
@@ -17,32 +17,27 @@ pub struct SpellFactory {
 impl Default for SpellFactory {
     fn default() -> Self {
         use crate::spell::std as spell;
-        let mut this = Self { dispatch: Default::default() };
-
-        this.register::<spell::Log>().expect("error in std spells");
-
-        this
+        Self::new_without_std().register::<spell::Log>()
     }
 }
 
 impl SpellFactory {
+    /// Create a new spell factory with standard spells.
+    pub fn new_without_std() -> Self {
+        Self {
+            dispatch: Default::default(),
+        }
+    }
+
     /// Get the list of all registered spells.
     pub fn get_registered_spells(&self) -> impl Iterator<Item = &'static str> {
         self.dispatch.keys().copied()
     }
 
     /// Register a spell into the factory. In case of re-registering returns an [Err].
-    pub fn register<Sp: BuildableSpell + 'static>(&mut self) -> Result<(), Error> {
-        Sp::name_list().try_for_each(|name| match self.dispatch.entry(name) {
-            std::collections::hash_map::Entry::Occupied(_) => Err(Error::Redefined(
-                Sp::name(),
-                "name or alias was already registered",
-            )),
-            std::collections::hash_map::Entry::Vacant(entry) => {
-                _ = entry.insert(Sp::create);
-                Ok(())
-            }
-        })
+    pub fn register<Sp: BuildableSpell + 'static>(mut self) -> Self {
+        Sp::name_list().for_each(|name| _ = self.dispatch.entry(name).or_insert(Sp::create));
+        self
     }
 
     /// Get the builder of a [Spell] by its name. In case of undefined [Spell], returns an [Err].
diff --git a/grimoire/src/spell/std/log.rs b/grimoire/src/spell/std/log.rs
index 5c7ba6f..6caf55e 100644
--- a/grimoire/src/spell/std/log.rs
+++ b/grimoire/src/spell/std/log.rs
@@ -2,29 +2,44 @@ use crate::prelude::*;
 
 #[derive(Debug)]
 pub struct Log {
-    level: String,
-    messages: Vec<String>,
+    if_predicate: Option<GrimoireExpression>,
+    level: GrimoireExpression,
+    messages: Vec<GrimoireExpression>,
 }
 
 impl Spell for Log {
     fn cast(&self, state: GrimoireState) -> Result<GrimoireState, GrimoireError> {
+        if let Some(pred) = &self.if_predicate {
+            if !(state.evaluate_as(pred, GrimoireType::Boolean)?).unwrap_boolean() {
+                return Ok(state);
+            }
+        }
+
         let (heading, complementary) = match self.messages.split_first() {
             Some((heading, complementary)) => (heading, complementary),
             None => return Ok(state),
         };
 
-        let logger = |ident: bool, str: &str| {
+        let level = state
+            .evaluate_as(&self.level, GrimoireType::String)?
+            .to_string();
+
+        let logger = |ident: bool, line: &GrimoireExpression| -> Result<(), GrimoireError> {
             let ident = ident.then_some("\t").unwrap_or_default();
-            match self.level.as_str() {
-                "error" => log::error!(target: "grimoire", "{ident}{str}"),
-                _ => log::info!(target: "grimoire", "{ident}{str}"),
+            let line = state.evaluate_as(line, GrimoireType::String)?.to_string();
+
+            match level.as_str() {
+                "error" => log::error!(target: "grimoire", "{ident}{line}"),
+                _ => log::info!(target: "grimoire", "{ident}{line}"),
             }
+
+            Ok(())
         };
 
-        logger(false, heading);
+        logger(false, heading)?;
         complementary
             .iter()
-            .for_each(|complementary| logger(true, complementary));
+            .try_for_each(|complementary| logger(true, complementary))?;
 
         Ok(state)
     }
@@ -34,19 +49,18 @@ impl TryFrom<SpellArguments> for Log {
     type Error = GrimoireError;
 
     fn try_from(mut args: SpellArguments) -> Result<Self, Self::Error> {
-        let level = args
-            .remove_as("level", GrimoireType::Identifier)?
-            .to_string();
-        args.has_named()
-            .then_some(())
-            .ok_or(GrimoireError::Unexpected("too much named arguments"))?;
-
+        let if_predicate = args.remove("if");
+        let [level] = args.remove_exact(["level"])?;
         let messages = args
             .into_iter()
-            .filter_map(|(key, msg)| key.is_none().then(|| msg.to_string()))
+            .filter_map(|(key, msg)| key.is_none().then_some(msg))
             .collect();
 
-        Ok(Self { level, messages })
+        Ok(Self {
+            if_predicate,
+            level,
+            messages,
+        })
     }
 }
 
diff --git a/grimoire/src/state.rs b/grimoire/src/state.rs
index 73dd415..6b390fb 100644
--- a/grimoire/src/state.rs
+++ b/grimoire/src/state.rs
@@ -24,9 +24,9 @@ impl State {
 
     fn evaluate_numeric_binop(
         &self,
-        left: Expression,
+        left: impl AsRef<Expression>,
         op: operator::Numeric,
-        right: Expression,
+        right: impl AsRef<Expression>,
     ) -> Result<Const, Error> {
         let left = self.evaluate_as(left, Type::Number)?;
         let right = self.evaluate_as(right, Type::Number)?;
@@ -82,9 +82,9 @@ impl State {
 
     fn evaluate_logic_binop(
         &self,
-        left: Expression,
+        left: impl AsRef<Expression>,
         op: operator::Logic,
-        right: Expression,
+        right: impl AsRef<Expression>,
     ) -> Result<Const, Error> {
         // Here we evaluate eagerly to catch errors!
         let left = self.evaluate_as(left, Type::Boolean)?.unwrap_boolean();
@@ -98,9 +98,9 @@ impl State {
 
     fn evaluate_equal_binop(
         &self,
-        left: Expression,
+        left: impl AsRef<Expression>,
         op: operator::Equal,
-        right: Expression,
+        right: impl AsRef<Expression>,
     ) -> Result<Const, Error> {
         Ok(Const::Bool(
             match (self.evaluate(left)?, self.evaluate(right)?) {
@@ -133,18 +133,18 @@ impl State {
         ))
     }
 
-    pub fn evaluate(&self, expr: Expression) -> Result<Const, Error> {
-        match expr {
+    pub fn evaluate(&self, expr: impl AsRef<Expression>) -> Result<Const, Error> {
+        match expr.as_ref() {
             // Leaf, simple.
-            Expression::Leaf(_, VarOrConst::Const(constant)) => Ok(constant),
+            Expression::Leaf(_, VarOrConst::Const(constant)) => Ok(constant.clone()),
             Expression::Leaf(_, VarOrConst::Var(var)) => self.resolve(var).cloned(),
 
             // Unary, not complicated.
             Expression::Unary(_, op, inner) => match op {
                 operator::Unary::Not => Ok(Const::Bool(
-                    !self.evaluate_as(*inner, Type::Boolean)?.unwrap_boolean(),
+                    !self.evaluate_as(inner, Type::Boolean)?.unwrap_boolean(),
                 )),
-                operator::Unary::Neg => match self.evaluate_as(*inner, Type::Number)? {
+                operator::Unary::Neg => match self.evaluate_as(inner, Type::Number)? {
                     Const::Int(x) => Ok(Const::Int(-x)),
                     Const::Flt(x) => Ok(Const::Flt(-x)),
                     _ => unreachable!(),
@@ -153,9 +153,9 @@ impl State {
 
             // Binary, just long to write...
             Expression::Binary(_, left, op, right) => match op {
-                operator::Binary::Numeric(op) => self.evaluate_numeric_binop(*left, op, *right),
-                operator::Binary::Logic(op) => self.evaluate_logic_binop(*left, op, *right),
-                operator::Binary::Equal(op) => self.evaluate_equal_binop(*left, op, *right),
+                operator::Binary::Numeric(op) => self.evaluate_numeric_binop(left, *op, right),
+                operator::Binary::Logic(op) => self.evaluate_logic_binop(left, *op, right),
+                operator::Binary::Equal(op) => self.evaluate_equal_binop(left, *op, right),
             },
         }
     }
@@ -184,7 +184,7 @@ impl State {
         }
     }
 
-    pub fn evaluate_as(&self, expr: Expression, ty: Type) -> Result<Const, Error> {
+    pub fn evaluate_as(&self, expr: impl AsRef<Expression>, ty: Type) -> Result<Const, Error> {
         let constant = self.evaluate(expr)?;
         match ty {
             Type::Identifier => self.check_identifier(constant.unwrap_identifier()),
-- 
GitLab