From a7ebe4de0b4443a12757fbf2ad0e001e52a5c8d6 Mon Sep 17 00:00:00 2001 From: Ubiratan Soares Date: Wed, 11 Dec 2024 01:48:33 +0100 Subject: [PATCH] feat: evaluates a matrix expansion only once (#274) --- src/audit/self_hosted_runner.rs | 2 +- src/models.rs | 36 ++++++++++++++++++--------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/audit/self_hosted_runner.rs b/src/audit/self_hosted_runner.rs index 72a4fae0..c0122f37 100644 --- a/src/audit/self_hosted_runner.rs +++ b/src/audit/self_hosted_runner.rs @@ -109,7 +109,7 @@ impl WorkflowAudit for SelfHostedRunner { LoE::Expr(exp) => { let matrix = Matrix::try_from(&job)?; - let expansions = matrix.expand_values(); + let expansions = matrix.expanded_values; let self_hosted = expansions.iter().any(|(path, expansion)| { exp.as_bare() == path && expansion.contains("self-hosted") diff --git a/src/models.rs b/src/models.rs index 32675c4b..29a1d5f9 100644 --- a/src/models.rs +++ b/src/models.rs @@ -233,6 +233,7 @@ impl<'w> Iterator for Jobs<'w> { #[derive(Clone)] pub(crate) struct Matrix<'w> { inner: &'w LoE, + pub(crate) expanded_values: Vec<(String, String)>, } impl<'w> Deref for Matrix<'w> { @@ -252,30 +253,43 @@ impl<'w> TryFrom<&'w Job<'w>> for Matrix<'w> { }; let Some(Strategy { - matrix: Some(matrix), + matrix: Some(inner), .. }) = &job.strategy else { bail!("job does not define a strategy or interior matrix") }; - Ok(Matrix { inner: matrix }) + Ok(Matrix::new(inner)) } } impl<'w> Matrix<'w> { pub(crate) fn new(inner: &'w LoE) -> Self { - Self { inner } + Self { + inner, + expanded_values: Matrix::expand_values(inner), + } + } + + /// Checks whether some expanded path leads to an expression + pub(crate) fn is_static(&self) -> bool { + let expands_to_expression = self + .expanded_values + .iter() + .any(|(_, expansion)| expansion.starts_with("${{") && expansion.ends_with("}}")); + + !expands_to_expression } /// Expands the current Matrix into all possible values /// By default, the return is a pair (String, String), in which /// the first component is the expanded path (e.g. 'matrix.os') and /// the second component is the string representation for the expanded value - /// (eg ubuntu-latest) + /// (e.g. ubuntu-latest) /// - pub(crate) fn expand_values(&self) -> Vec<(String, String)> { - match &self.inner { + fn expand_values(inner: &LoE) -> Vec<(String, String)> { + match inner { LoE::Expr(_) => vec![], LoE::Literal(matrix) => { let LoE::Literal(dimensions) = &matrix.dimensions else { @@ -310,16 +324,6 @@ impl<'w> Matrix<'w> { } } - /// Checks whether some expanded path leads to an expression - pub(crate) fn is_static(&self) -> bool { - let expands_to_expression = self - .expand_values() - .iter() - .any(|(_, expansion)| expansion.starts_with("${{") && expansion.ends_with("}}")); - - !expands_to_expression - } - fn expand_explicit_rows( include: &IndexMap, ) -> Vec<(String, String)> {