fix: strict typing for variants (#1415)

src/recipe/custom_yaml.rs

@@ -432,6 +432,14 @@ impl ScalarNode {
             _ => None,
         }
     }
+
+    /// Convert the scalar node to an integer and follow coercion rules
+    pub fn as_integer(&self) -> Option<i64> {
+        if !self.may_coerce {
+            return None;
+        }
+        self.value.parse().ok()
+    }
 }
 
 impl HasSpan for ScalarNode {

src/recipe/custom_yaml/rendered.rs

@@ -234,6 +234,7 @@ pub struct RenderedScalarNode {
     span: marked_yaml::Span,
     source: String,
     value: String,
+    may_coerce: bool,
 }
 
 impl Serialize for RenderedScalarNode {
@@ -246,16 +247,22 @@ impl Serialize for RenderedScalarNode {
 }
 
 impl RenderedScalarNode {
-    pub fn new(span: marked_yaml::Span, source: String, value: String) -> Self {
+    pub fn new(span: marked_yaml::Span, source: String, value: String, may_coerce: bool) -> Self {
         Self {
             span,
             source,
             value,
+            may_coerce,
         }
     }
 
     pub fn new_blank() -> Self {
-        Self::new(marked_yaml::Span::new_blank(), String::new(), String::new())
+        Self::new(
+            marked_yaml::Span::new_blank(),
+            String::new(),
+            String::new(),
+            false,
+        )
     }
 
     /// Treat the scalar node as a string
@@ -286,12 +293,22 @@ impl RenderedScalarNode {
     ///
     /// Everything else is not a boolean and so will return None
     pub fn as_bool(&self) -> Option<bool> {
+        if !self.may_coerce {
+            return None;
+        }
         match self.value.as_str() {
             "true" | "True" | "TRUE" => Some(true),
             "false" | "False" | "FALSE" => Some(false),
             _ => None,
         }
     }
+
+    pub fn as_integer(&self) -> Option<i64> {
+        if !self.may_coerce {
+            return None;
+        }
+        self.value.parse().ok()
+    }
 }
 
 impl HasSpan for RenderedScalarNode {
@@ -340,14 +357,15 @@ impl<'a> From<&'a str> for RenderedScalarNode {
             marked_yaml::Span::new_blank(),
             value.to_owned(),
             value.to_owned(),
+            false,
         )
     }
 }
 
 impl From<String> for RenderedScalarNode {
     /// Convert from any owned string into a node
     fn from(value: String) -> Self {
-        Self::new(marked_yaml::Span::new_blank(), value.clone(), value)
+        Self::new(marked_yaml::Span::new_blank(), value.clone(), value, false)
     }
 }
 
@@ -377,6 +395,7 @@ impl From<&MarkedScalarNode> for RenderedScalarNode {
             *value.span(),
             value.as_str().to_owned(),
             value.as_str().to_owned(),
+            value.may_coerce(),
         )
     }
 }
@@ -639,6 +658,7 @@ impl Render<RenderedNode> for Node {
                 *n.span(),
                 n.as_str().to_owned(),
                 n.as_str().to_owned(),
+                false,
             ))),
         }
     }
@@ -653,7 +673,13 @@ impl Render<RenderedNode> for ScalarNode {
                 label = jinja_error_to_label(&err),
             )]
         })?;
-        let rendered = RenderedScalarNode::new(*self.span(), self.as_str().to_string(), 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,
+        );
 
         if rendered.is_empty() {
             Ok(RenderedNode::Null(rendered))
@@ -677,7 +703,12 @@ impl Render<Option<RenderedNode>> for ScalarNode {
             )]
         })?;
 
-        let rendered = RenderedScalarNode::new(*self.span(), self.as_str().to_string(), rendered);
+        let rendered = RenderedScalarNode::new(
+            *self.span(),
+            self.as_str().to_string(),
+            rendered,
+            self.may_coerce,
+        );
 
         if rendered.is_empty() {
             Ok(None)
@@ -708,6 +739,7 @@ impl Render<RenderedMappingNode> for MappingNode {
                 *key.span(),
                 key.as_str().to_owned(),
                 key.as_str().to_owned(),
+                false,
             );
             let value: RenderedNode = value.render(jinja, &format!("{name}.{}", key.as_str()))?;
             if value.is_null() {
@@ -730,6 +762,7 @@ impl Render<RenderedNode> for SequenceNode {
                 *self.span(),
                 String::new(),
                 String::new(),
+                false,
             )));
         }
 

src/recipe/parser.rs

@@ -184,7 +184,7 @@ impl Recipe {
                 if let Some(rendered) = rendered {
                     let variable = if let Some(value) = rendered.as_bool() {
                         Variable::from(value)
-                    } else if let Some(value) = rendered.as_i64() {
+                    } else if let Some(value) = rendered.as_integer() {
                         Variable::from(value)
                     } else {
                         Variable::from_string(&rendered)

src/recipe/variable.rs

@@ -85,7 +85,7 @@ impl TryConvertNode<Variable> for RenderedScalarNode {
     fn try_convert(&self, _name: &str) -> Result<Variable, Vec<PartialParsingError>> {
         let variable = if let Some(value) = self.as_bool() {
             Variable::from(value)
-        } else if let Some(value) = self.as_i64() {
+        } else if let Some(value) = self.as_integer() {
             Variable::from(value)
         } else {
             Variable::from_string(self)

src/snapshots/rattler_build__variant_config__tests__flatten_selectors-2.snap

@@ -7,7 +7,7 @@ zip_keys: ~
 c_compiler:
   - vs2019
 cplusplus:
-  - 100
+  - "100"
 cxx_compiler:
   - vs2019
 python:

src/snapshots/rattler_build__variant_config__tests__flatten_selectors.snap

@@ -7,8 +7,8 @@ zip_keys: ~
 c_compiler:
   - super_g++
 cplusplus:
-  - 10
-  - 1000
+  - "10"
+  - "1000"
 cxx_compiler:
   - super_gcc
 python:

src/snapshots/rattler_build__variant_config__tests__load_config.snap

@@ -66,8 +66,12 @@ blas_impl:
   - openblas
   - mkl
   - blis
+boolean:
+  - true
 build_platform:
   - linux-64
+integer:
+  - 5
 libblas:
   - 3.9 *netlib
 libcblas:
@@ -76,5 +80,9 @@ liblapack:
   - 3.9 *netlib
 liblapacke:
   - 3.9 *netlib
+noboolean:
+  - "true"
+nointeger:
+  - "5"
 target_platform:
   - linux-64

src/used_variables.rs

@@ -73,6 +73,9 @@ fn extract_variable_from_expression(expr: &Expr, variables: &mut HashSet<String>
         Expr::Var(var) => {
             variables.insert(var.id.into());
         }
+        Expr::Test(test) => {
+            extract_variable_from_expression(&test.expr, variables);
+        }
         Expr::BinOp(binop) => {
             extract_variable_from_expression(&binop.left, variables);
             extract_variable_from_expression(&binop.right, variables);
@@ -337,6 +340,7 @@ mod test {
             - ${{ pin_compatible(abc ~ def) }}
             - if: match(xpython, ">=3.7")
               then: numpy 100
+            - ${{ testexprvar is string }}
         "#;
 
         let recipe_node = crate::recipe::custom_yaml::Node::parse_yaml(0, recipe).unwrap();
@@ -354,6 +358,7 @@ mod test {
         assert!(used_vars.contains("abc"));
         assert!(used_vars.contains("def"));
         assert!(used_vars.contains("xpython"));
+        assert!(used_vars.contains("testexprvar"));
     }
 
     #[test]

src/variant_config.rs

@@ -260,6 +260,7 @@ impl VariantConfig {
         let rendered_node: RenderedNode = yaml_node
             .render(&jinja, path.to_string_lossy().as_ref())
             .map_err(|e| ParseErrors::from_partial_vec(source.clone(), e))?;
+
         let config: VariantConfig = rendered_node
             .try_convert(path.to_string_lossy().as_ref())
             .map_err(|e| {
@@ -577,9 +578,9 @@ impl TryConvertNode<VariantConfig> for RenderedMappingNode {
                     config.zip_keys = value.try_convert(key_str)?;
                 }
                 _ => {
-                    let variants: Option<Vec<_>> = value.try_convert(key_str)?;
+                    let variants: Option<Vec<Variable>> = value.try_convert(key_str)?;
                     if let Some(variants) = variants {
-                        config.variants.insert(key_str.into(), variants.clone());
+                        config.variants.insert(key_str.into(), variants);
                     }
                 }
             }
@@ -740,7 +741,10 @@ mod tests {
         };
 
         let variant = VariantConfig::from_files(&[yaml_file], &selector_config).unwrap();
-
+        assert_eq!(
+            variant.variants.get(&"noboolean".into()).unwrap(),
+            &vec![Variable::from_string("true")]
+        );
         insta::assert_yaml_snapshot!(variant);
     }
 

test-data/recipes/jinja-types/recipe.yaml

@@ -7,6 +7,7 @@ context:
   cfloat: 1.23
   cstring: "blah"
   cboolean: true
+  cquoted_bool: "true"
   cquoted: "5"
   is_integer: ${{ xinteger is integer }}
   is_float: ${{ xfloat is float }}
@@ -17,6 +18,13 @@ context:
   is_cstring: ${{ cstring is string }}
   is_cboolean: ${{ cboolean is boolean }}
   is_cquoted: ${{ cquoted is string }}
+  computed_int: ${{ "12.3" | replace(".", "") | int }}
+  is_computed_int: ${{ computed_int is integer }}
+  is_quoted_true: ${{ cquoted_bool is string }}
+  # quoted bool is quoted in variants.yaml and thus should be a string
+  is_quoted_true_var: ${{ quoted_bool is string }}
+  is_quoted_int_var: ${{ quoted_int is string }}
+
 
 package:
   name: testtypes

test-data/recipes/jinja-types/variants.yaml

@@ -6,3 +6,7 @@ xstring:
   - "blah"
 xboolean:
   - true
+quoted_bool:
+  - "true"
+quoted_int:
+  - "5"

test-data/variant_files/variant_config_1.yaml

@@ -74,3 +74,7 @@ arrow_cpp:
 assimp:
   - 5.2.5
 aws_sdk_cpp: "4.5"
+integer: 5
+boolean: true
+noboolean: "true"
+nointeger: "5"

test/end-to-end/__snapshots__/test_simple/test_jinja_types.1.json

@@ -1,4 +1,6 @@
 {
+  "quoted_bool": "true",
+  "quoted_int": "5",
   "xboolean": true,
   "xfloat": "1.23",
   "xinteger": 5,

test/end-to-end/__snapshots__/test_simple/test_jinja_types.json

@@ -3,18 +3,24 @@
   "cboolean": true,
   "cfloat": "1.23",
   "cinteger": 5,
+  "computed_int": 123,
   "cquoted": "5",
+  "cquoted_bool": "true",
   "cstring": "blah",
   "float": "1.23",
   "integer": 5,
   "is_boolean": true,
   "is_cboolean": true,
   "is_cfloat": false,
   "is_cinteger": true,
+  "is_computed_int": true,
   "is_cquoted": true,
   "is_cstring": true,
   "is_float": false,
   "is_integer": true,
+  "is_quoted_int_var": true,
+  "is_quoted_true": true,
+  "is_quoted_true_var": true,
   "is_string": true,
   "string": "blah"
 }