From ccc000f5d65834c1ea0c4ada43950f3e8420a0ba Mon Sep 17 00:00:00 2001 From: martinohmann Date: Sun, 17 Sep 2023 13:42:04 +0200 Subject: [PATCH] feat(eval): add `Evaluate::evaluate_in_place` (#292) Closes #184. Key difference to the existing `Evaluate::evaluate` method: - It mutably borrows the value and tries to evaluate all nested expressions in-place. - It returns all errors that it encounters, not just the first one. This allows for partial expression evaluation, e.g. to support use cases where a HCL document contains variable references to other parts within the same document that themselves contain expressions that are not evaluated yet. In this case one would: 1. Partially evaluate the document via `evaluate_in_place`. 2. Update the `Context` with newly discovered variables. 3. Repeat 1. and 2. until there are no more errors left. --- crates/hcl-rs/Cargo.toml | 4 + .../examples/in-place-expr-evaluation.rs | 134 ++++++++++++++ crates/hcl-rs/src/eval/error.rs | 93 +++++++++- crates/hcl-rs/src/eval/impls.rs | 167 +++++++++++++++++- crates/hcl-rs/src/eval/mod.rs | 20 ++- crates/hcl-rs/tests/eval.rs | 96 +++++++++- 6 files changed, 508 insertions(+), 6 deletions(-) create mode 100644 crates/hcl-rs/examples/in-place-expr-evaluation.rs diff --git a/crates/hcl-rs/Cargo.toml b/crates/hcl-rs/Cargo.toml index 8dd24502..07848247 100644 --- a/crates/hcl-rs/Cargo.toml +++ b/crates/hcl-rs/Cargo.toml @@ -45,3 +45,7 @@ vecmap-rs = { version = "0.1.11", features = ["serde"] } indoc = "2.0" pretty_assertions = "1.4.0" serde_json = { version = "1.0.105", features = ["preserve_order"] } + +[[example]] +name = "in-place-expr-evaluation" +test = true diff --git a/crates/hcl-rs/examples/in-place-expr-evaluation.rs b/crates/hcl-rs/examples/in-place-expr-evaluation.rs new file mode 100644 index 00000000..71f3a1dc --- /dev/null +++ b/crates/hcl-rs/examples/in-place-expr-evaluation.rs @@ -0,0 +1,134 @@ +use hcl::eval::{Context, Evaluate}; + +fn main() -> Result<(), Box> { + let Some(filename) = std::env::args().into_iter().skip(1).next() else { + eprintln!("filename argument required"); + std::process::exit(1); + }; + + let input = std::fs::read_to_string(filename)?; + let mut body = hcl::parse(&input)?; + let ctx = Context::new(); + + // This will try to evaluate all expressions in `body` and updates it in-place, returning all + // errors that occurred along the way. + if let Err(errors) = body.evaluate_in_place(&ctx) { + eprintln!("{errors}"); + } + + hcl::to_writer(std::io::stdout(), &body)?; + Ok(()) +} + +#[cfg(test)] +mod test { + use super::*; + use hcl::eval::{FuncDef, ParamType}; + use hcl::value::Value; + use hcl::Map; + use pretty_assertions::assert_eq; + + #[test] + fn exprs_are_evaluated_in_place() { + let input = indoc::indoc! {r#" + resource "aws_eks_cluster" "this" { + count = var.create_eks ? 1 : 0 + + name = var.cluster_name + role_arn = local.cluster_iam_role_arn + version = var.cluster_version + + vpc_config { + security_group_ids = compact([local.cluster_security_group_id]) + subnet_ids = var.subnets + } + + kubernetes_network_config { + service_ipv4_cidr = var.cluster_service_ipv4_cidr + } + + tags = merge( + var.tags, + var.cluster_tags, + ) + } + "#}; + + let mut body = hcl::parse(&input).unwrap(); + let mut ctx = Context::new(); + ctx.declare_var( + "var", + hcl::value!({ + "create_eks" = true + "cluster_name" = "mycluster" + "cluster_tags" = { + "team" = "ops" + } + "cluster_version" = "1.27.0" + "tags" = { + "environment" = "dev" + } + }), + ); + + ctx.declare_func( + "merge", + FuncDef::builder() + .variadic_param(ParamType::Any) + .build(|args| { + let mut map = Map::::new(); + for arg in args.variadic_args() { + if let Some(object) = arg.as_object() { + map.extend(object.clone()); + } else { + return Err(format!("Argument {:?} is not an object", arg)); + } + } + + Ok(Value::Object(map)) + }), + ); + + let res = body.evaluate_in_place(&ctx); + assert!(res.is_err()); + + let errors = res.unwrap_err(); + + assert_eq!(errors.len(), 4); + assert_eq!( + errors.to_string(), + indoc::indoc! {r#" + 4 errors occurred: + - undefined variable `local` in expression `local.cluster_iam_role_arn` + - undefined variable `local` in expression `local.cluster_security_group_id` + - no such key: `subnets` in expression `var.subnets` + - no such key: `cluster_service_ipv4_cidr` in expression `var.cluster_service_ipv4_cidr` + "#} + ); + + let expected = indoc::indoc! {r#" + resource "aws_eks_cluster" "this" { + count = 1 + name = "mycluster" + role_arn = local.cluster_iam_role_arn + version = "1.27.0" + + vpc_config { + security_group_ids = compact([local.cluster_security_group_id]) + subnet_ids = var.subnets + } + + kubernetes_network_config { + service_ipv4_cidr = var.cluster_service_ipv4_cidr + } + + tags = { + "environment" = "dev" + "team" = "ops" + } + } + "#}; + + assert_eq!(hcl::to_string(&body).unwrap(), expected); + } +} diff --git a/crates/hcl-rs/src/eval/error.rs b/crates/hcl-rs/src/eval/error.rs index 6078281d..d0219ea5 100644 --- a/crates/hcl-rs/src/eval/error.rs +++ b/crates/hcl-rs/src/eval/error.rs @@ -4,8 +4,97 @@ use std::fmt; /// The result type used by this module. pub type EvalResult = std::result::Result; +pub(super) trait EvalResultExt { + fn add_errors(self, rhs: Self) -> Self; +} + +impl EvalResultExt for EvalResult<(), Errors> { + fn add_errors(self, rhs: Self) -> Self { + match self { + Err(mut lhs) => { + lhs.extend_from_result(rhs); + Err(lhs) + } + _ => rhs, + } + } +} + +/// A type holding multiple errors that occurred during in-place expression evaluation via +/// [`Evaluate::evaluate_in_place`]. +/// +/// It is guaranteed that `Errors` instances hold at least one error. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Errors { + inner: Vec, +} + +impl Errors { + fn extend_from_result(&mut self, res: EvalResult<(), Errors>) { + if let Err(errors) = res { + self.inner.extend(errors); + } + } + + /// Returns the number of errors. + #[inline] + #[allow(clippy::len_without_is_empty)] + pub fn len(&self) -> usize { + self.inner.len() + } + + /// Returns an iterator over all errors. + #[inline] + pub fn iter(&self) -> std::slice::Iter { + self.inner.iter() + } +} + +impl fmt::Display for Errors { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.len() == 1 { + self.inner[0].fmt(f) + } else { + writeln!(f, "{} errors occurred:", self.len())?; + + for error in self { + writeln!(f, "- {error}")?; + } + + Ok(()) + } + } +} + +impl From for Errors { + #[inline] + fn from(error: Error) -> Self { + Errors { inner: vec![error] } + } +} + +impl std::error::Error for Errors {} + +impl IntoIterator for Errors { + type Item = Error; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.inner.into_iter() + } +} + +impl<'a> IntoIterator for &'a Errors { + type Item = &'a Error; + type IntoIter = std::slice::Iter<'a, Error>; + + fn into_iter(self) -> Self::IntoIter { + self.iter() + } +} + /// The error type returned by all fallible operations within this module. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Error { inner: Box, } @@ -73,7 +162,7 @@ impl std::error::Error for Error {} // The inner type that holds the actual error data. // // This is a separate type because it gets boxed to keep the size of the `Error` struct small. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] struct ErrorInner { kind: ErrorKind, expr: Option, diff --git a/crates/hcl-rs/src/eval/impls.rs b/crates/hcl-rs/src/eval/impls.rs index 51d61004..82c1e153 100644 --- a/crates/hcl-rs/src/eval/impls.rs +++ b/crates/hcl-rs/src/eval/impls.rs @@ -1,3 +1,4 @@ +use super::error::EvalResultExt; use super::*; use indexmap::map::Entry; use std::hash::Hash; @@ -12,6 +13,13 @@ impl Evaluate for Body { .map(|structure| structure.evaluate(ctx)) .collect() } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + #[allow(clippy::manual_try_fold)] + self.iter_mut().fold(Ok(()), |res, structure| { + res.add_errors(structure.evaluate_in_place(ctx)) + }) + } } impl private::Sealed for Structure {} @@ -25,6 +33,13 @@ impl Evaluate for Structure { Structure::Block(block) => block.evaluate(ctx).map(Structure::Block), } } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + match self { + Structure::Attribute(attr) => attr.evaluate_in_place(ctx), + Structure::Block(block) => block.evaluate_in_place(ctx), + } + } } impl private::Sealed for Attribute {} @@ -38,6 +53,10 @@ impl Evaluate for Attribute { expr: self.expr.evaluate(ctx).map(Into::into)?, }) } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + self.expr.evaluate_in_place(ctx) + } } impl private::Sealed for Block {} @@ -52,6 +71,10 @@ impl Evaluate for Block { body: self.body.evaluate(ctx)?, }) } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + self.body.evaluate_in_place(ctx) + } } impl private::Sealed for Expression {} @@ -77,6 +100,38 @@ impl Evaluate for Expression { other => Ok(Value::from(other.clone())), } } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + // We can ignore expressions that cannot be further evaluated. This avoids unnecessary + // clone operations. + if !matches!( + self, + Expression::Null | Expression::Bool(_) | Expression::Number(_) | Expression::String(_) + ) { + evaluate_nested_exprs(self, ctx)?; + let value = self.evaluate(ctx)?; + *self = value.into(); + } + + Ok(()) + } +} + +fn evaluate_nested_exprs(expr: &mut Expression, ctx: &Context) -> EvalResult<(), Errors> { + let expr_clone = expr.clone(); + let ctx = &ctx.child_with_expr(&expr_clone); + + match expr { + Expression::Array(array) => array.evaluate_in_place(ctx), + Expression::Object(object) => object.evaluate_in_place(ctx), + Expression::Traversal(traversal) => traversal.evaluate_in_place(ctx), + Expression::FuncCall(func_call) => func_call.evaluate_in_place(ctx), + Expression::Parenthesis(expr) => expr.evaluate_in_place(ctx), + Expression::Conditional(cond) => cond.evaluate_in_place(ctx), + Expression::Operation(op) => op.evaluate_in_place(ctx), + Expression::ForExpr(expr) => expr.evaluate_in_place(ctx), + _ => Ok(()), + } } impl private::Sealed for Vec where T: Evaluate {} @@ -90,6 +145,13 @@ where fn evaluate(&self, ctx: &Context) -> EvalResult { self.iter().map(|expr| expr.evaluate(ctx)).collect() } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + #[allow(clippy::manual_try_fold)] + self.iter_mut().fold(Ok(()), |res, element| { + res.add_errors(element.evaluate_in_place(ctx)) + }) + } } impl private::Sealed for Object @@ -101,7 +163,7 @@ where impl Evaluate for Object where - K: Evaluate, + K: Evaluate + Eq, K::Output: Hash + Eq, V: Evaluate, { @@ -112,6 +174,25 @@ where .map(|(key, expr)| Ok((key.evaluate(ctx)?, expr.evaluate(ctx)?))) .collect() } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + let mut new_object = Object::with_capacity(self.len()); + + #[allow(clippy::manual_try_fold)] + let res = self + .drain(..) + .fold(Ok(()), |mut res, (mut key, mut value)| { + res = res + .add_errors(key.evaluate_in_place(ctx)) + .add_errors(value.evaluate_in_place(ctx)); + new_object.insert(key, value); + res + }); + + *self = new_object; + + res + } } impl private::Sealed for ObjectKey {} @@ -125,6 +206,14 @@ impl Evaluate for ObjectKey { ident => Ok(ident.to_string()), } } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + if let ObjectKey::Expression(expr) = self { + expr.evaluate_in_place(ctx)?; + } + + Ok(()) + } } impl private::Sealed for TemplateExpr {} @@ -160,6 +249,35 @@ impl Evaluate for Template { template::evaluate_template(&mut result, self, ctx, Strip::None, Strip::None)?; Ok(result) } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + #[allow(clippy::manual_try_fold)] + self.elements_mut() + .iter_mut() + .fold(Ok(()), |mut res, element| match element { + Element::Literal(_) => res, + Element::Interpolation(interp) => { + res.add_errors(interp.expr.evaluate_in_place(ctx)) + } + Element::Directive(dir) => match dir { + Directive::If(dir) => { + res = res + .add_errors(dir.cond_expr.evaluate_in_place(ctx)) + .add_errors(dir.true_template.evaluate_in_place(ctx)); + + match &mut dir.false_template { + Some(false_template) => { + res.add_errors(false_template.evaluate_in_place(ctx)) + } + None => res, + } + } + Directive::For(dir) => res + .add_errors(dir.collection_expr.evaluate_in_place(ctx)) + .add_errors(dir.template.evaluate_in_place(ctx)), + }, + }) + } } impl private::Sealed for Traversal {} @@ -172,6 +290,16 @@ impl Evaluate for Traversal { let deque = self.operators.iter().collect(); expr::evaluate_traversal(value, deque, ctx) } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + #[allow(clippy::manual_try_fold)] + self.operators + .iter_mut() + .fold(Ok(()), |res, operator| match operator { + TraversalOperator::Index(expr) => res.add_errors(expr.evaluate_in_place(ctx)), + _ => res, + }) + } } impl private::Sealed for FuncCall {} @@ -196,6 +324,10 @@ impl Evaluate for FuncCall { func.call(args) .map_err(|err| ctx.error(ErrorKind::FuncCall(name.clone(), err))) } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + self.args.evaluate_in_place(ctx) + } } impl private::Sealed for Conditional {} @@ -210,6 +342,18 @@ impl Evaluate for Conditional { self.false_expr.evaluate(ctx) } } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + let cond = expr::evaluate_bool(&self.cond_expr, ctx)?; + + self.cond_expr = Expression::from(cond); + + if cond { + self.true_expr.evaluate_in_place(ctx) + } else { + self.false_expr.evaluate_in_place(ctx) + } + } } impl private::Sealed for Operation {} @@ -223,6 +367,13 @@ impl Evaluate for Operation { Operation::Binary(binary) => binary.evaluate(ctx), } } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + match self { + Operation::Unary(unary) => unary.evaluate_in_place(ctx), + Operation::Binary(binary) => binary.evaluate_in_place(ctx), + } + } } impl private::Sealed for UnaryOp {} @@ -243,6 +394,10 @@ impl Evaluate for UnaryOp { Ok(value) } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + self.expr.evaluate_in_place(ctx) + } } impl private::Sealed for BinaryOp {} @@ -276,6 +431,12 @@ impl Evaluate for BinaryOp { Ok(value) } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + self.lhs_expr + .evaluate_in_place(ctx) + .add_errors(self.rhs_expr.evaluate_in_place(ctx)) + } } impl private::Sealed for ForExpr {} @@ -328,4 +489,8 @@ impl Evaluate for ForExpr { } } } + + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + self.collection_expr.evaluate_in_place(ctx) + } } diff --git a/crates/hcl-rs/src/eval/mod.rs b/crates/hcl-rs/src/eval/mod.rs index b5d3bbad..5e65f0d9 100644 --- a/crates/hcl-rs/src/eval/mod.rs +++ b/crates/hcl-rs/src/eval/mod.rs @@ -224,7 +224,7 @@ mod func; mod impls; mod template; -pub use self::error::{Error, ErrorKind, EvalResult}; +pub use self::error::{Error, ErrorKind, Errors, EvalResult}; pub use self::func::{ Func, FuncArgs, FuncDef, FuncDefBuilder, ParamType, PositionalArgs, VariadicArgs, }; @@ -269,6 +269,24 @@ pub trait Evaluate: private::Sealed { /// - an undefined variable or function is encountered. /// - a defined function is called with unexpected arguments. fn evaluate(&self, ctx: &Context) -> EvalResult; + + /// Recursively tries to evaluate all nested expressions in place. + /// + /// This function does not stop at the first error but continues to evaluate expressions as far + /// as it can. + /// + /// The default implementation does nothing and always returns `Ok(())`. + /// + /// # Errors + /// + /// Returns an [`Errors`] value containing one of more [`Error`]s if the evaluation of any + /// (potentially nested) expression fails. + /// + /// See the errors section of [`evaluate`][Evaluate::evaluate] for a list of failure modes. + fn evaluate_in_place(&mut self, ctx: &Context) -> EvalResult<(), Errors> { + _ = ctx; + Ok(()) + } } /// A type holding the evaluation context. diff --git a/crates/hcl-rs/tests/eval.rs b/crates/hcl-rs/tests/eval.rs index e2b595e4..5a31a569 100644 --- a/crates/hcl-rs/tests/eval.rs +++ b/crates/hcl-rs/tests/eval.rs @@ -1,14 +1,14 @@ mod common; use common::{assert_eval, assert_eval_ctx, assert_eval_error}; -use hcl::eval::{Context, ErrorKind, EvalResult, FuncArgs, FuncDef, ParamType}; +use hcl::eval::{Context, ErrorKind, EvalResult, Evaluate, FuncArgs, FuncDef, ParamType}; use hcl::expr::{ BinaryOp, BinaryOperator, Conditional, Expression, ForExpr, FuncCall, TemplateExpr, Traversal, TraversalOperator, Variable, }; use hcl::structure::Body; use hcl::template::Template; -use hcl::{Identifier, Number, Value}; +use hcl::{Attribute, Block, Identifier, Number, Value}; use indoc::indoc; #[test] @@ -384,6 +384,98 @@ fn expr_error_context() { ) } +#[test] +fn eval_in_place() { + let mut ctx = Context::new(); + + let mut body = Body::builder() + .add_block( + Block::builder("foo") + .add_attribute(Attribute::new("bar", FuncCall::new("baz"))) + .add_attribute(Attribute::new( + "qux", + BinaryOp::new(1, BinaryOperator::Plus, 1), + )) + .build(), + ) + .add_block( + Block::builder("bar") + .add_attribute(Attribute::new("baz", Variable::unchecked("qux"))) + .build(), + ) + .build(); + + assert_eq!(body.evaluate_in_place(&ctx).unwrap_err().len(), 2); + + let expected = Body::builder() + .add_block( + Block::builder("foo") + .add_attribute(Attribute::new("bar", FuncCall::new("baz"))) + .add_attribute(Attribute::new("qux", 2)) + .build(), + ) + .add_block( + Block::builder("bar") + .add_attribute(Attribute::new("baz", Variable::unchecked("qux"))) + .build(), + ) + .build(); + + assert_eq!(body, expected); + + ctx.declare_var("qux", "quxval"); + ctx.declare_func("baz", FuncDef::builder().build(|_| Ok(Value::from("baz")))); + + assert!(body.evaluate_in_place(&ctx).is_ok()); + + let expected = Body::builder() + .add_block( + Block::builder("foo") + .add_attribute(Attribute::new("bar", "baz")) + .add_attribute(Attribute::new("qux", 2)) + .build(), + ) + .add_block( + Block::builder("bar") + .add_attribute(Attribute::new("baz", "quxval")) + .build(), + ) + .build(); + + assert_eq!(body, expected); +} + +#[test] +fn eval_in_place_error() { + let mut body = Body::builder() + .add_attribute(( + "foo", + BinaryOp::new(1, BinaryOperator::Plus, Variable::unchecked("bar")), + )) + .add_attribute(( + "bar", + Conditional::new( + BinaryOp::new(1, BinaryOperator::Less, 2), + FuncCall::builder("true_action").arg(1).build(), + FuncCall::new("false_action"), + ), + )) + .build(); + + let ctx = Context::new(); + let err = body.evaluate_in_place(&ctx).unwrap_err(); + + assert_eq!( + err.to_string(), + indoc! {r#" + 2 errors occurred: + - undefined variable `bar` in expression `1 + bar` + - undefined function `true_action` in expression `1 < 2 ? true_action(1) : false_action()` + "# + } + ) +} + #[test] fn interpolation_unwrapping() { // unwrapping