Re: [PATCH proxmox 01/29] add new proxmox-match-expression crate

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

On Thu, Jul 09, 2026 at 01:56:48PM +0200, Lukas Wagner wrote:
> This new crate is a generic implementation of a serializable expression
> language. It supports basic combinators, such as AnyOf, OneOf and AllOf,
> constants, Not, as well as custom matcher leave nodes that are injected
> via a generic type parameter.
> 
> The first user of this implementation will be the notification stack.
> 
i really like this crate, great job! beeing able to serialize evaluated
expressions is a cool feature for notifications history. some comments
inline. 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  Cargo.toml                                    |   2 +
>  proxmox-match-expression/Cargo.toml           |  18 +
>  proxmox-match-expression/debian/changelog     |   6 +
>  proxmox-match-expression/debian/control       |  34 ++
>  proxmox-match-expression/debian/copyright     |  18 +
>  proxmox-match-expression/debian/debcargo.toml |   7 +
>  proxmox-match-expression/src/lib.rs           | 511 ++++++++++++++++++
>  7 files changed, 596 insertions(+)
>  create mode 100644 proxmox-match-expression/Cargo.toml
>  create mode 100644 proxmox-match-expression/debian/changelog
>  create mode 100644 proxmox-match-expression/debian/control
>  create mode 100644 proxmox-match-expression/debian/copyright
>  create mode 100644 proxmox-match-expression/debian/debcargo.toml
>  create mode 100644 proxmox-match-expression/src/lib.rs
> 
> diff --git a/Cargo.toml b/Cargo.toml
> index ef407166..fcb06704 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -31,6 +31,7 @@ members = [
>      "proxmox-log",
>      "proxmox-login",
>      "proxmox-metrics",
> +    "proxmox-match-expression",
>      "proxmox-network-api",
>      "proxmox-network-types",
>      "proxmox-node-status",
> @@ -177,6 +178,7 @@ proxmox-io = { version = "1.2.1", path = "proxmox-io" }
>  proxmox-lang = { version = "1.5", path = "proxmox-lang" }
>  proxmox-log = { version = "1.0.0", path = "proxmox-log" }
>  proxmox-login = { version = "1.0.0", path = "proxmox-login" }
> +proxmox-match-expression = { version = "0.1", path = "proxmox-match-expression" }
>  proxmox-network-types = { version = "1.0.2", path = "proxmox-network-types" }
>  proxmox-parallel-handler = { version = "1.0.0", path = "proxmox-parallel-handler" }
>  proxmox-pgp = { version = "1.0.0", path = "proxmox-pgp" }
> diff --git a/proxmox-match-expression/Cargo.toml b/proxmox-match-expression/Cargo.toml
> new file mode 100644
> index 00000000..294537bc
> --- /dev/null
> +++ b/proxmox-match-expression/Cargo.toml
> @@ -0,0 +1,18 @@
> +[package]
> +name = "proxmox-match-expression"
> +description = "evaluation logic for nested match expressions"
> +version = "0.1.0"
> +
> +authors.workspace = true
> +edition.workspace = true
> +exclude.workspace = true
> +homepage.workspace = true
> +license.workspace = true
> +repository.workspace = true
> +
> +[dependencies]
> +serde = { workspace = true, features = ["derive"] }
> +
> +[dev-dependencies]
> +serde_json.workspace = true
> +pretty_assertions.workspace = true
> diff --git a/proxmox-match-expression/debian/changelog b/proxmox-match-expression/debian/changelog
> new file mode 100644
> index 00000000..44b20a10
> --- /dev/null
> +++ b/proxmox-match-expression/debian/changelog
> @@ -0,0 +1,6 @@
> +rust-proxmox-match-expression (0.1.0-1) unstable; urgency=medium
> +
> +  * initial version.
> +
> + -- Proxmox Support Team <support@proxmox.com>  Thu, 18 Jun 2026 14:02:03 +0200
> +
> diff --git a/proxmox-match-expression/debian/control b/proxmox-match-expression/debian/control
> new file mode 100644
> index 00000000..14bc8999
> --- /dev/null
> +++ b/proxmox-match-expression/debian/control
> @@ -0,0 +1,34 @@
> +Source: rust-proxmox-match-expression
> +Section: rust
> +Priority: optional
> +Build-Depends: debhelper-compat (= 13),
> + dh-sequence-cargo
> +Build-Depends-Arch: cargo:native <!nocheck>,
> + rustc:native <!nocheck>,
> + libstd-rust-dev <!nocheck>,
> + librust-serde-1+default-dev <!nocheck>,
> + librust-serde-1+derive-dev <!nocheck>
> +Maintainer: Proxmox Support Team <support@proxmox.com>
> +Standards-Version: 4.7.2
> +Vcs-Git: git://git.proxmox.com/git/proxmox.git
> +Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
> +Homepage: https://proxmox.com
> +X-Cargo-Crate: proxmox-match-expression
> +
> +Package: librust-proxmox-match-expression-dev
> +Architecture: any
> +Multi-Arch: same
> +Depends:
> + ${misc:Depends},
> + librust-serde-1+default-dev,
> + librust-serde-1+derive-dev
> +Provides:
> + librust-proxmox-match-expression+default-dev (= ${binary:Version}),
> + librust-proxmox-match-expression-0-dev (= ${binary:Version}),
> + librust-proxmox-match-expression-0+default-dev (= ${binary:Version}),
> + librust-proxmox-match-expression-0.1-dev (= ${binary:Version}),
> + librust-proxmox-match-expression-0.1+default-dev (= ${binary:Version}),
> + librust-proxmox-match-expression-0.1.0-dev (= ${binary:Version}),
> + librust-proxmox-match-expression-0.1.0+default-dev (= ${binary:Version})
> +Description: Evaluation logic for nested match expressions - Rust source code
> + Source code for Debianized Rust crate "proxmox-match-expression"
> diff --git a/proxmox-match-expression/debian/copyright b/proxmox-match-expression/debian/copyright
> new file mode 100644
> index 00000000..279c2689
> --- /dev/null
> +++ b/proxmox-match-expression/debian/copyright
> @@ -0,0 +1,18 @@
> +Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> +
> +Files:
> + *
> +Copyright: 2024 - 2026 Proxmox Server Solutions GmbH <support@proxmox.com>
> +License: AGPL-3.0-or-later
> + This program is free software: you can redistribute it and/or modify it under
> + the terms of the GNU Affero General Public License as published by the Free
> + Software Foundation, either version 3 of the License, or (at your option) any
> + later version.
> + .
> + This program is distributed in the hope that it will be useful, but WITHOUT
> + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
> + details.
> + .
> + You should have received a copy of the GNU Affero General Public License along
> + with this program. If not, see <https://www.gnu.org/licenses/>.
> diff --git a/proxmox-match-expression/debian/debcargo.toml b/proxmox-match-expression/debian/debcargo.toml
> new file mode 100644
> index 00000000..b7864cdb
> --- /dev/null
> +++ b/proxmox-match-expression/debian/debcargo.toml
> @@ -0,0 +1,7 @@
> +overlay = "."
> +crate_src_path = ".."
> +maintainer = "Proxmox Support Team <support@proxmox.com>"
> +
> +[source]
> +vcs_git = "git://git.proxmox.com/git/proxmox.git"
> +vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
> diff --git a/proxmox-match-expression/src/lib.rs b/proxmox-match-expression/src/lib.rs
> new file mode 100644
> index 00000000..0b9b0208
> --- /dev/null
> +++ b/proxmox-match-expression/src/lib.rs
> @@ -0,0 +1,511 @@
> +//! De/serializable expression evaluation with custom leaf-nodes.
> +//!
[...]
> +//! assert_str_eq!(serialized, expected);
> +//!
> +//! assert!(!evaluated_expression.children()[0].is_match());
> +//! assert!(evaluated_expression.children()[1].is_match());
> +//!
> +//! assert_eq!(evaluated_expression.children()[1].children(), &[]);
> +//!
> +//! ```
> +
thanks for the extensive docs :))

> +use serde::{Deserialize, Serialize};
> +
> +/// Trait for custom expression leaf-nodes.
> +pub trait MatchExpression {
> +    /// Use this type to specify the type of the data the expression is evaluated against.
> +    type Data;
> +
> +    /// Error type returned when evaluating this custom expression.
> +    type Error;
> +
> +    /// Evaluate this expression.
> +    ///
> +    /// If there are no errors, this should return `Ok(true)` or `Ok(false)`, depending on
> +    /// whether this leaf-node expression matched the provided data or not.
> +    fn evaluate(&self, data: &Self::Data) -> Result<bool, Self::Error>;
> +}
> +
> +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
> +#[serde(rename_all = "kebab-case")]
> +/// Match expression.
> +pub enum Expression<M> {
> +    /// Match if all of the sub-expression match.
> +    AllOf(Vec<Expression<M>>),
> +    /// Match if any of the sub-expressions matches.
> +    AnyOf(Vec<Expression<M>>),
> +    /// Match if exactly one of the sub-expressions matches.
> +    OneOf(Vec<Expression<M>>),
> +    /// Match if the sub-expression does not match.
> +    Not(Box<Expression<M>>),
> +    /// Constant value (true or false).
> +    Constant(bool),
> +    /// Custom matcher leaf-nodes.
> +    Match(M),
> +}
> +
> +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
> +#[serde(rename_all = "kebab-case")]
> +/// Evaluated match expression.
> +pub enum EvaluatedExpression<M> {
> +    /// Match if all of the sub-expressions match.
> +    AllOf(Vec<EvaluatedExpressionWithResult<M>>),
> +    /// Match if any of the sub-expressions matches.
> +    AnyOf(Vec<EvaluatedExpressionWithResult<M>>),
> +    /// Match if exactly one of the sub-expressions matches.
> +    OneOf(Vec<EvaluatedExpressionWithResult<M>>),
> +    /// Match if the sub-expression does not match.
> +    Not(Box<EvaluatedExpressionWithResult<M>>),
> +    /// Constant value (true or false).
> +    Constant(bool),
> +    /// Custom matcher leaf-nodes.
> +    Match(M),
> +}
> +
> +/// Evaluated expression resulting from [`Expression::evaluate`].
> +///
> +/// This type contains the original expression, augmented with per-node
> +/// results. This is useful for recording 'traces', e.g. for recording
> +/// exactly why a notification matcher matched a notification or not.
> +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
> +pub struct EvaluatedExpressionWithResult<M> {
> +    /// The actual expression.
> +    #[serde(flatten)]
> +    expression: EvaluatedExpression<M>,
> +    /// The evaluated expression matched the notification.
since this crate is meant to be generic, we might want to drop
'notification' in favor of 'input' or something like that
> +    matches: bool,
> +}
> +
> +impl<M> EvaluatedExpressionWithResult<M> {
> +    /// Return whether the expression matched the notification.
here as well
> +    pub fn is_match(&self) -> bool {
> +        self.matches
> +    }
> +
> +    /// Borrow the contained [`EvaluatedExpression`].
> +    pub fn expression(&self) -> &EvaluatedExpression<M> {
> +        &self.expression
> +    }
> +
> +    /// Return the contained [`EvaluatedExpression`], consuming `self`.
> +    pub fn into_expression(self) -> EvaluatedExpression<M> {
> +        self.expression
> +    }
nit: is there any reason why this is not an impl From<A> for B? if not i
would personally find such an API more idiomatic
> +
> +    /// The direct child nodes, uniform across all combinator kinds.
> +    ///
> +    /// 'not' yields its single operand as a one-element slice; leaf nodes
> +    /// ('constant', 'match') yield an empty slice. This lets a generic tree
> +    /// walker recurse without special-casing each variant.
> +    pub fn children(&self) -> &[EvaluatedExpressionWithResult<M>] {
> +        match &self.expression {
> +            EvaluatedExpression::AllOf(expressions)
> +            | EvaluatedExpression::AnyOf(expressions)
> +            | EvaluatedExpression::OneOf(expressions) => expressions,
> +            EvaluatedExpression::Not(expression) => std::slice::from_ref(&**expression),
> +            EvaluatedExpression::Constant(_) | EvaluatedExpression::Match(_) => &[],
> +        }
> +    }
> +}
> +
> +impl<M, D, E> Expression<M>
> +where
> +    M: Clone + MatchExpression<Data = D, Error = E>,
> +{
> +    /// Evaluate this expression against a provided variable.
> +    ///
> +    /// This function returns an `EvaluatedExpression`, replicating the exact structure
> +    /// of `self`, but annotated with a per-subexpression trace to record which of
> +    /// the subexpressions matched or not.
> +    ///
> +    /// # Note
> +    /// This evaluates the expression and *all* sub-expressions fully and does not return
> +    /// early if the final result is already determined by the already evaluated sub-expressions
> +    /// (for instance, [`Expression::AnyOf`] and the first sub-expression matched).
> +    /// This is done so that we can return the full trace via the [`EvaluatedExpressionWithResult`]
> +    /// type.
> +    ///
> +    /// If you need fast evaluation and do not care about the trace, feel free to add a
> +    /// `evaluate_fast` (or similar), which does early returns as appropriate and simply returns
> +    /// `bool`.
> +    pub fn evaluate(&self, data: &D) -> Result<EvaluatedExpressionWithResult<M>, E> {
> +        let evaluated_expression = match self {
> +            Expression::AllOf(expressions) => {
> +                if expressions.is_empty() {
> +                    // 'all-of' without any sub-expressions is *not* a match
> +                    EvaluatedExpressionWithResult {
> +                        expression: EvaluatedExpression::AllOf(Vec::new()),
> +                        matches: false,
> +                    }
this probaly does not matter too much as long as it's documented, but
intuitively (and based on implementations for all-expressions in 
other languages like rust and python) i would expect all([]) to evaluate to
true.
> +                } else {
> +                    let (matches, evaluated_expressions) =
> +                        Self::evaluate_subexpressions(data, expressions, true, |a, b| a && b)?;
> +
> +                    EvaluatedExpressionWithResult {
> +                        expression: EvaluatedExpression::AllOf(evaluated_expressions),
> +                        matches,
> +                    }
> +                }
> +            }
> +            Expression::AnyOf(expressions) => {
> +                let (matches, evaluated_expressions) =
> +                    Self::evaluate_subexpressions(data, expressions, false, |a, b| a || b)?;
> +
> +                EvaluatedExpressionWithResult {
> +                    expression: EvaluatedExpression::AnyOf(evaluated_expressions),
> +                    matches,
> +                }
> +            }
> +            Expression::OneOf(expressions) => {
> +                let mut matched_count = 0;
> +                let mut evaluated_expressions = Vec::new();
> +
> +                for expression in expressions {
> +                    let evaluated_expression = expression.evaluate(data)?;
> +                    if evaluated_expression.is_match() {
> +                        matched_count += 1;
> +                    }
> +                    evaluated_expressions.push(evaluated_expression)
> +                }
> +
> +                EvaluatedExpressionWithResult {
> +                    expression: EvaluatedExpression::OneOf(evaluated_expressions),
> +                    matches: matched_count == 1,
> +                }
> +            }
> +            Expression::Not(expression) => {
> +                let evaluated_expression = expression.evaluate(data)?;
> +                let matches = !evaluated_expression.is_match();
> +                EvaluatedExpressionWithResult {
> +                    expression: EvaluatedExpression::Not(Box::new(evaluated_expression)),
> +                    matches,
> +                }
> +            }
> +            Expression::Constant(value) => EvaluatedExpressionWithResult {
> +                expression: EvaluatedExpression::Constant(*value),
> +                matches: *value,
> +            },
> +            Expression::Match(inner) => {
> +                let matches = inner.evaluate(data)?;
> +                EvaluatedExpressionWithResult {
> +                    expression: EvaluatedExpression::Match(inner.clone()),
> +                    matches,
> +                }
> +            }
> +        };
> +
> +        Ok(evaluated_expression)
> +    }
> +
> +    fn evaluate_subexpressions(
> +        data: &D,
> +        expressions: &[Expression<M>],
> +        initial: bool,
> +        op: impl Fn(bool, bool) -> bool,
> +    ) -> Result<(bool, Vec<EvaluatedExpressionWithResult<M>>), E> {
> +        let mut matches = initial;
> +
> +        let mut evaluated_expressions = Vec::new();
> +
> +        for expression in expressions {
> +            let evaluated_expression = expression.evaluate(data)?;
> +            matches = op(matches, evaluated_expression.is_match());
> +            evaluated_expressions.push(evaluated_expression);
> +        }
> +
> +        Ok((matches, evaluated_expressions))
> +    }
> +}
> +
> +/// Convenience macro that can be used to create an 'AnyOf' expression.
> +///
> +/// ```
> +/// use proxmox_match_expression::{any_of, Expression};
> +///
> +/// let x: Expression<()> = any_of![
> +///     Expression::Constant(true),
> +/// ];
> +/// assert_eq!(x, Expression::AnyOf(vec![Expression::Constant(true)]));
> +/// ```
> +///
> +#[macro_export]
> +macro_rules! any_of {
> +    ($($x:expr),* $(,)?) => (
> +        $crate::Expression::AnyOf(vec![$($x),*])
> +    );
> +}
> +
> +/// Convenience macro that can be used to create an 'AllOf' expression.
> +///
> +/// ```
> +/// use proxmox_match_expression::{all_of, Expression};
> +///
> +/// let x: Expression<()> = all_of![
> +///     Expression::Constant(true),
> +/// ];
> +/// assert_eq!(x, Expression::AllOf(vec![Expression::Constant(true)]));
> +/// ```
> +#[macro_export]
> +macro_rules! all_of {
> +    ($($x:expr),* $(,)?) => (
> +        $crate::Expression::AllOf(vec![$($x),*])
> +    );
> +}
> +
> +/// Convenience macro that can be used to create an 'OneOf' expression.
> +///
> +/// ```
> +/// use proxmox_match_expression::{one_of, Expression};
> +///
> +/// let x: Expression<()> = one_of![
> +///     Expression::Constant(true),
> +/// ];
> +/// assert_eq!(x, Expression::OneOf(vec![Expression::Constant(true)]));
> +/// ```
> +#[macro_export]
> +macro_rules! one_of {
> +    ($($x:expr),* $(,)?) => (
> +        $crate::Expression::OneOf(vec![$($x),*])
> +    );
> +}
> +
> +/// Convenience macro that can be used to create a 'Not' expression;
> +///
> +/// ```
> +/// use proxmox_match_expression::{not, Expression};
> +///
> +/// let x: Expression<()> = not!(Expression::Constant(true));
> +/// assert_eq!(x, Expression::Not(Box::new(Expression::Constant(true))));
> +/// ```
> +#[macro_export]
> +macro_rules! not {
> +    ($e:expr) => {
> +        $crate::Expression::Not(Box::new($e))
> +    };
> +}
> +
> +#[cfg(test)]
> +mod test {
> +    use super::*;
> +
> +    #[derive(Serialize, Deserialize, Debug, Clone)]
> +    #[serde(rename_all = "kebab-case", tag = "type")]
> +    pub enum TestMatcher {
> +        CustomFailure,
> +        CustomTrue,
> +        CustomFalse,
> +    }
nit: this enum does not pass clippy because of the common prefix
> +
> +    impl MatchExpression for TestMatcher {
> +        type Data = ();
> +        type Error = ();
> +
> +        fn evaluate(&self, _data: &Self::Data) -> Result<bool, ()> {
> +            match self {
> +                TestMatcher::CustomFailure => Err(()),
> +                TestMatcher::CustomTrue => Ok(true),
> +                TestMatcher::CustomFalse => Ok(false),
> +            }
> +        }
> +    }
> +
[...]