deeper jinja integration

wolfv · wolfv · commit 0ae50f82b5ff · 2025-05-04T16:31:32.000+02:00
diff --git a/src/recipe/custom_yaml.rs b/src/recipe/custom_yaml.rs
@@ -228,7 +228,6 @@ impl Render<Node> for SequenceNode {
 
 impl Render<Node> for SequenceNodeInternal {
     fn render(&self, jinja: &Jinja, name: &str) -> Result<Node, Vec<PartialParsingError>> {
-        println!("Rendering: {:?}", self);
         match self {
             Self::Simple(n) => n.render(jinja, name),
             Self::Conditional(if_sel) => {
@@ -391,6 +390,15 @@ pub struct ScalarNode {
     may_coerce: bool,
 }
 
+/// Convert a string to a boolean
+pub fn string_to_bool(value: &str) -> Option<bool> {
+    match value {
+        "true" | "True" | "TRUE" => Some(true),
+        "false" | "False" | "FALSE" => Some(false),
+        _ => None,
+    }
+}
+
 impl ScalarNode {
     /// Create a new scalar node with a span
     pub fn new(span: marked_yaml::Span, value: String, may_coerce: bool) -> Self {
@@ -427,11 +435,8 @@ impl ScalarNode {
         if !self.may_coerce {
             return None;
         }
-        match self.value.as_str() {
-            "true" | "True" | "TRUE" => Some(true),
-            "false" | "False" | "FALSE" => Some(false),
-            _ => None,
-        }
+
+        string_to_bool(&self.value)
     }
 
     /// Convert the scalar node to an integer and follow coercion rules
diff --git a/src/recipe/custom_yaml/rendered.rs b/src/recipe/custom_yaml/rendered.rs
@@ -133,6 +133,59 @@ impl RenderedNode {
             _ => None,
         }
     }
+
+    pub fn from_jinja_value(
+        source: String,
+        value: minijinja::Value,
+        span: Span,
+        coercible: bool,
+    ) -> Result<Self, PartialParsingError> {
+        if !coercible {
+            return Ok(RenderedNode::Scalar(RenderedScalarNode::new(
+                span,
+                value.to_string(),
+                value.to_string(),
+                false,
+            )));
+        }
+
+        match value.kind() {
+            minijinja::value::ValueKind::Map => {
+                todo!("Maps not supported yet");
+            }
+            minijinja::value::ValueKind::Seq => {
+                let mut rendered: Vec<RenderedNode> = Vec::new();
+                for elem in value.try_iter().unwrap() {
+                    let node =
+                        RenderedNode::from_jinja_value(source.clone(), elem, span.clone(), true)?;
+                    rendered.push(node);
+                }
+                Ok(RenderedNode::Sequence(RenderedSequenceNode::from(rendered)))
+            }
+            minijinja::value::ValueKind::String
+            | minijinja::value::ValueKind::Bool
+            | minijinja::value::ValueKind::Number => {
+                let value = value.to_string();
+                if value.is_empty() {
+                    return Ok(RenderedNode::Null(RenderedScalarNode::new(
+                        span,
+                        source,
+                        String::new(),
+                        false,
+                    )));
+                }
+                Ok(RenderedNode::Scalar(RenderedScalarNode::new(
+                    span, source, value, true,
+                )))
+            }
+            minijinja::value::ValueKind::None | minijinja::value::ValueKind::Undefined => Ok(
+                RenderedNode::Null(RenderedScalarNode::new(span, source, String::new(), false)),
+            ),
+            _ => {
+                todo!("Other types not supported yet");
+            }
+        }
+    }
 }
 
 impl HasSpan for RenderedNode {
@@ -265,6 +318,14 @@ impl RenderedScalarNode {
         )
     }
 
+    pub fn new_string(value: String) -> Self {
+        Self::new(marked_yaml::Span::new_blank(), value.clone(), value, false)
+    }
+
+    pub fn new_coerceable(value: String) -> Self {
+        Self::new(marked_yaml::Span::new_blank(), value.clone(), value, true)
+    }
+
     /// Treat the scalar node as a string
     ///
     /// Since scalars are always stringish, this is always safe.
@@ -662,27 +723,44 @@ impl Render<RenderedNode> for Node {
 
 impl Render<RenderedNode> for ScalarNode {
     fn render(&self, jinja: &Jinja, _name: &str) -> Result<RenderedNode, Vec<PartialParsingError>> {
-        let (rendered, may_coerce) = jinja.render_str(self.as_str()).map_err(|err| {
+        // println!("Rendering scalar node: {}", self.as_str());
+        // let (rendered, may_coerce) = jinja.render_str(self.as_str()).map_err(|err| {
+        //     vec![_partialerror!(
+        //         *self.span(),
+        //         ErrorKind::JinjaRendering(err),
+        //         label = jinja_error_to_label(&err),
+        //     )]
+        // })?;
+
+        let (value, can_coerce) = jinja.render_to_value(self).map_err(|err| {
             vec![_partialerror!(
                 *self.span(),
                 ErrorKind::JinjaRendering(err),
                 label = jinja_error_to_label(&err),
             )]
         })?;
 
-        // unsure whether this should be allowed to coerce // check if it's quoted?
-        let rendered = RenderedScalarNode::new(
+        return Ok(RenderedNode::from_jinja_value(
+            self.to_string(),
+            value,
             *self.span(),
-            self.as_str().to_string(),
-            rendered,
-            self.may_coerce && may_coerce,
-        );
+            self.may_coerce && can_coerce,
+        )
+        .unwrap());
 
-        if rendered.is_empty() {
-            Ok(RenderedNode::Null(rendered))
-        } else {
-            Ok(RenderedNode::Scalar(rendered))
-        }
+        // unsure whether this should be allowed to coerce // check if it's quoted?
+        // let rendered = RenderedScalarNode::new(
+        //     *self.span(),
+        //     self.as_str().to_string(),
+        //     rendered,
+        //     self.may_coerce && may_coerce,
+        // );
+
+        // if rendered.is_empty() {
+        //     Ok(RenderedNode::Null(rendered))
+        // } else {
+        //     Ok(RenderedNode::Scalar(rendered))
+        // }
     }
 }
 
diff --git a/src/recipe/jinja.rs b/src/recipe/jinja.rs
@@ -17,6 +17,7 @@ use crate::render::pin::PinArgs;
 pub use crate::render::pin::{Pin, PinExpression};
 pub use crate::selectors::SelectorConfig;
 
+use super::custom_yaml::ScalarNode;
 use super::parser::{Dependency, PinCompatible, PinSubpackage};
 use super::variable::Variable;
 
@@ -54,6 +55,32 @@ pub struct Jinja {
     context: BTreeMap<String, Value>,
 }
 
+/// If we have a template that is _only_ an expression, we want to strip the
+/// `${{` and `}}` from it so that we can evaluate it as an expression instead of
+/// rendering it as a string.
+///
+/// The function checks for:
+/// - Whitespace before/after the expression
+/// - Single expression with no nested expressions
+/// - Proper opening `${{` and closing `}}`
+fn strip_expression(template: &str) -> Option<&str> {
+    let trimmed = template.trim();
+    if !trimmed.starts_with("${{") || !trimmed.ends_with("}}") {
+        return None;
+    }
+
+    // Extract content between ${{ and }}
+    let content = &trimmed[3..trimmed.len() - 2];
+    let content = content.trim();
+
+    // Check for nested expressions
+    if content.contains("${{") || content.contains("}}") {
+        return None;
+    }
+
+    Some(content)
+}
+
 impl Jinja {
     /// Create a new Jinja instance with the given selector configuration.
     pub fn new(config: SelectorConfig) -> Self {
@@ -94,26 +121,42 @@ impl Jinja {
         &mut self.context
     }
 
-    /// Render a template with the current context.
-    pub fn render_str(&self, template: &str) -> Result<(String, bool), minijinja::Error> {
-        if template.starts_with("${{") && template.ends_with("}}") {
-            // render as expression so that we know the type of the result, and can stringify accordingly
-            // If we find something like "${{ foo }}" then we want to evaluate it type-safely and make sure that the MiniJinja type is kept
-            let tmplt = &template[3..template.len() - 2];
-            let expr = self.env.compile_expression(tmplt)?;
-            let evaled = expr.eval(self.context())?;
-            if let Some(s) = evaled.to_str() {
-                // Make sure that the string stays a string by returning can_coerce: false
-                return Ok((s.to_string(), false));
-            } else {
-                return Ok((evaled.to_string(), true));
-            }
+    /// Render the given template to a Jinja value.
+    pub fn render_to_value(
+        &self,
+        template: &ScalarNode,
+    ) -> Result<(Value, bool), minijinja::Error> {
+        if let Some(simple_expr) = strip_expression(template) {
+            // render as expression so that we know the type of the result
+            let expr = self.env.compile_expression(simple_expr)?;
+            return Ok((expr.eval(self.context())?, true));
         }
 
+        // Otherwise just render it as string
         let rendered = self.env.render_str(template, &self.context)?;
-        Ok((rendered, !template.contains("${{")))
+        Ok((Value::from(rendered), !template.contains("${{")))
     }
 
+    /// Render a template with the current context.
+    // pub fn render_str(&self, template: &str) -> Result<(String, bool), minijinja::Error> {
+    //     if template.starts_with("${{") && template.ends_with("}}") {
+    //         // render as expression so that we know the type of the result, and can stringify accordingly
+    //         // If we find something like "${{ foo }}" then we want to evaluate it type-safely and make sure that the MiniJinja type is kept
+    //         let tmplt = &template[3..template.len() - 2];
+    //         let expr = self.env.compile_expression(tmplt)?;
+    //         let evaled = expr.eval(self.context())?;
+    //         if let Some(s) = evaled.to_str() {
+    //             // Make sure that the string stays a string by returning can_coerce: false
+    //             return Ok((s.to_string(), false));
+    //         } else {
+    //             return Ok((evaled.to_string(), true));
+    //         }
+    //     }
+
+    //     let rendered = self.env.render_str(template, &self.context)?;
+    //     Ok((rendered, !template.contains("${{")))
+    // }
+
     /// Render, compile and evaluate a expr string with the current context.
     pub fn eval(&self, str: &str) -> Result<Value, minijinja::Error> {
         let expr = self.env.compile_expression(str)?;
@@ -1381,4 +1424,23 @@ mod tests {
             default_compiler(platform, "cuda").unwrap().to_string()
         );
     }
+
+    #[test]
+    fn test_strip_expression() {
+        // Valid cases
+        assert_eq!(strip_expression("${{ expr }}"), Some("expr"));
+        assert_eq!(strip_expression("  ${{ expr }}  "), Some("expr"));
+        assert_eq!(strip_expression("${{expr}}"), Some("expr"));
+        assert_eq!(
+            strip_expression("${{ expr with spaces }}"),
+            Some("expr with spaces")
+        );
+
+        // Invalid cases
+        assert_eq!(strip_expression("not an expression"), None);
+        assert_eq!(strip_expression("${{ nested ${{ expr }} }}"), None);
+        assert_eq!(strip_expression("${{ unmatched"), None);
+        assert_eq!(strip_expression("unmatched }}"), None);
+        assert_eq!(strip_expression("text ${{ expr }} text"), None);
+    }
 }
diff --git a/src/recipe/parser.rs b/src/recipe/parser.rs
@@ -55,6 +55,8 @@ pub use self::{
 
 use crate::recipe::{custom_yaml::Node, variable::Variable};
 
+use super::custom_yaml::string_to_bool;
+
 /// A recipe that has been parsed and validated.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct Recipe {
@@ -149,19 +151,35 @@ impl Recipe {
                 help = "`context` values must always be scalars (booleans, integers or strings) or uniform lists of scalars"
             )]
         })?;
-        let rendered: Option<ScalarNode> = val.render(jinja, &format!("context.{}", k.as_str()))?;
-        if let Some(rendered) = rendered {
-            let variable = if let Some(value) = rendered.as_bool() {
-                Variable::from(value)
-            } else if let Some(value) = rendered.as_integer() {
-                Variable::from(value)
+        // val.render(jinja, &format!("context.{}", k.as_str()))?;
+        let (value, can_coerce) = jinja.render_to_value(&val).unwrap();
+
+        // See if we have to coerce a string-type to a boolean or integer
+        if can_coerce && value.as_str().is_some() {
+            // let's see if the value should be an integer or a boolean
+            let stringified = value.to_string();
+            if let Some(boolean) = string_to_bool(&stringified) {
+                return Ok(Some(Variable::from(boolean)));
+            } else if let Some(integer) = stringified.parse::<i64>().ok() {
+                return Ok(Some(Variable::from(integer)));
             } else {
-                Variable::from_string(&rendered)
-            };
-            Ok(Some(variable))
-        } else {
-            Ok(None)
+                return Ok(Some(Variable::from_string(&stringified)));
+            }
         }
+        // TODO handle null
+        return Ok(Some(Variable::from_value(value)));
+        // if let Some(rendered) = rendered {
+        //     let variable = if let Some(value) = rendered.as_bool() {
+        //         Variable::from(value)
+        //     } else if let Some(value) = rendered.as_integer() {
+        //         Variable::from(value)
+        //     } else {
+        //         Variable::from_string(&rendered)
+        //     };
+        //     Ok(Some(variable))
+        // } else {
+        //     Ok(None)
+        // }
     }
 
     /// Create recipes from a YAML [`Node`] structure.
diff --git a/src/recipe/snapshots/rattler_build__recipe__parser__tests__jinja_error.snap b/src/recipe/snapshots/rattler_build__recipe__parser__tests__jinja_error.snap
@@ -6,7 +6,7 @@ expression: err
 
 Error: 
   × failed to render Jinja expression: unknown function: zcompiler is unknown
-  │ (in <string>:1)
+  │ (in <expression>:1)
    ╭─[7:7]
  6 │   host:
  7 │     - ${{ zcompiler('c') }}
diff --git a/src/recipe/variable.rs b/src/recipe/variable.rs
@@ -61,6 +61,11 @@ impl Variable {
     pub fn from_string(value: &str) -> Self {
         Variable(Value::from_safe_string(value.to_string()))
     }
+
+    /// Create a variable from a `minijinja::Value`
+    pub fn from_value(value: Value) -> Self {
+        Variable(value)
+    }
 }
 
 impl Display for Variable {

Original file line number	Diff line number	Diff line change
`@@ -61,6 +61,11 @@ impl Variable {`
`61`	`61`	`pub fn from_string(value: &str) -> Self {`
`62`	`62`	`Variable(Value::from_safe_string(value.to_string()))`
`63`	`63`	`}`
	`64`	`+`
	`65`	+ /// Create a variable from a `minijinja::Value`
	`66`	`+ pub fn from_value(value: Value) -> Self {`
	`67`	`+ Variable(value)`
	`68`	`+ }`
`64`	`69`	`}`
`65`	`70`
`66`	`71`	`impl Display for Variable {`