refactor to test positive matches

codex-rs/execpolicy2/src/error.rs

@@ -2,7 +2,7 @@ use thiserror::Error;
 
 pub type Result<T> = std::result::Result<T, Error>;
 
-#[derive(Debug, Error)]
+#[derive(Debug, Error, PartialEq, Eq)]
 pub enum Error {
     #[error("invalid decision: {0}")]
     InvalidDecision(String),
@@ -10,8 +10,14 @@ pub enum Error {
     InvalidPattern(String),
     #[error("invalid example: {0}")]
     InvalidExample(String),
-    #[error("expected example to match rule `{rule}`: {example}")]
-    ExampleDidNotMatch { rule: String, example: String },
+    #[error(
+        "expected every example to match at least one rule. rules: {rules:?}; unmatched examples: \
+         {examples:?}"
+    )]
+    ExampleDidNotMatch {
+        rules: Vec<String>,
+        examples: Vec<String>,
+    },
     #[error("expected example to not match rule `{rule}`: {example}")]
     ExampleDidMatch { rule: String, example: String },
     #[error("starlark error: {0}")]

codex-rs/execpolicy2/src/parser.rs

@@ -17,6 +17,7 @@ use std::sync::Arc;
 use crate::decision::Decision;
 use crate::error::Error;
 use crate::error::Result;
+use crate::policy::validate_match_examples;
 use crate::rule::PatternToken;
 use crate::rule::PrefixPattern;
 use crate::rule::PrefixRule;
@@ -203,17 +204,28 @@ fn policy_builtins(builder: &mut GlobalsBuilder) {
 
         let rest: Arc<[PatternToken]> = remaining_tokens.to_vec().into();
 
-        for head in first_token.alternatives() {
-            let rule = Rule::Prefix(PrefixRule {
-                pattern: PrefixPattern {
-                    first: Arc::from(head.as_str()),
-                    rest: rest.clone(),
-                },
-                decision,
-            });
-            rule.validate_examples(&matches, &not_matches)?;
-            builder.add_rule(rule);
-        }
+        let rules: Vec<Rule> = first_token
+            .alternatives()
+            .iter()
+            .zip(std::iter::repeat(rest.clone()))
+            .map(|(head, rest)| {
+                Rule::Prefix(PrefixRule {
+                    pattern: PrefixPattern {
+                        first: Arc::from(head.as_str()),
+                        rest,
+                    },
+                    decision,
+                })
+            })
+            .collect();
+
+        rules
+            .iter()
+            .try_for_each(|rule| rule.validate_not_matches(&not_matches))?;
+
+        validate_match_examples(&rules, &matches)?;
+
+        rules.into_iter().for_each(|rule| builder.add_rule(rule));
         Ok(NoneType)
     }
 }

codex-rs/execpolicy2/src/policy.rs

@@ -1,9 +1,12 @@
 use crate::decision::Decision;
+use crate::error::Error;
+use crate::error::Result;
 use crate::rule::Rule;
 use crate::rule::RuleMatch;
 use multimap::MultiMap;
 use serde::Deserialize;
 use serde::Serialize;
+use shlex::try_join;
 
 #[derive(Clone, Debug)]
 pub struct Policy {
@@ -55,3 +58,33 @@ impl Evaluation {
         matches!(self, Self::Match { .. })
     }
 }
+
+pub(crate) fn validate_match_examples(rules: &[Rule], matches: &[Vec<String>]) -> Result<()> {
+    let match_counts = rules.iter().fold(vec![0; matches.len()], |counts, rule| {
+        counts
+            .iter()
+            .zip(rule.validate_matches(matches))
+            .map(|(count, matched)| if matched { count + 1 } else { *count })
+            .collect()
+    });
+
+    let unmatched_examples: Vec<String> = matches
+        .iter()
+        .zip(&match_counts)
+        .filter(|(_, count)| **count == 0)
+        .map(|(example, _)| {
+            try_join(example.iter().map(String::as_str))
+                .unwrap_or_else(|_| "unable to render example".to_string())
+        })
+        .collect();
+
+    if unmatched_examples.is_empty() {
+        return Ok(());
+    }
+
+    let rules: Vec<String> = rules.iter().map(|rule| format!("{rule:?}")).collect();
+    Err(Error::ExampleDidNotMatch {
+        rules,
+        examples: unmatched_examples,
+    })
+}

codex-rs/execpolicy2/src/rule.rs

@@ -101,19 +101,14 @@ impl PrefixRule {
             })
     }
 
-    pub fn validate_examples(
-        &self,
-        matches: &[Vec<String>],
-        not_matches: &[Vec<String>],
-    ) -> Result<()> {
-        for example in matches {
-            if self.matches(example).is_none() {
-                return Err(Error::ExampleDidNotMatch {
-                    rule: format!("{self:?}"),
-                    example: join_command(example),
-                });
-            }
-        }
+    pub fn validate_matches(&self, matches: &[Vec<String>]) -> Vec<bool> {
+        matches
+            .iter()
+            .map(|example| self.matches(example).is_some())
+            .collect()
+    }
+
+    pub fn validate_not_matches(&self, not_matches: &[Vec<String>]) -> Result<()> {
         for example in not_matches {
             if self.matches(example).is_some() {
                 return Err(Error::ExampleDidMatch {
@@ -144,13 +139,15 @@ impl Rule {
         }
     }
 
-    pub fn validate_examples(
-        &self,
-        matches: &[Vec<String>],
-        not_matches: &[Vec<String>],
-    ) -> Result<()> {
+    pub fn validate_matches(&self, matches: &[Vec<String>]) -> Vec<bool> {
         match self {
-            Self::Prefix(rule) => rule.validate_examples(matches, not_matches),
+            Self::Prefix(rule) => rule.validate_matches(matches),
+        }
+    }
+
+    pub fn validate_not_matches(&self, not_matches: &[Vec<String>]) -> Result<()> {
+        match self {
+            Self::Prefix(rule) => rule.validate_not_matches(not_matches),
         }
     }
 }

codex-rs/execpolicy2/tests/basic.rs

@@ -1,6 +1,7 @@
 use std::sync::Arc;
 
 use codex_execpolicy2::Decision;
+use codex_execpolicy2::Error;
 use codex_execpolicy2::Evaluation;
 use codex_execpolicy2::PolicyParser;
 use codex_execpolicy2::Rule;
@@ -180,6 +181,40 @@ prefix_rule(
     assert_eq!(Evaluation::NoMatch, no_match_eval);
 }
 
+#[test]
+fn reports_unmatched_example_for_first_token_alternatives() {
+    let policy_src = r#"
+prefix_rule(
+    pattern = [["bash", "sh"], "-c"],
+    match = [
+        ["bash", "-c"],
+        ["zsh", "-c"],
+    ],
+)
+    "#;
+
+    let err = PolicyParser::parse("test.codexpolicy", policy_src)
+        .expect_err("expected unmatched example error");
+    let expected_message = r#"Traceback (most recent call last):
+  * test.codexpolicy:2, in <module>
+      prefix_rule(
+error: expected every example to match at least one rule. rules: ["Prefix(PrefixRule { pattern: PrefixPattern { first: "bash", rest: [Single("-c")] }, decision: Allow })", "Prefix(PrefixRule { pattern: PrefixPattern { first: "sh", rest: [Single("-c")] }, decision: Allow })"]; unmatched examples: ["zsh -c"]
+ --> test.codexpolicy:2:1
+  |
+2 | / prefix_rule(
+3 | |     pattern = [["bash", "sh"], "-c"],
+4 | |     match = [
+5 | |         ["bash", "-c"],
+6 | |         ["zsh", "-c"],
+7 | |     ],
+8 | | )
+  | |_^
+  |
+"#;
+    let expected_err = Error::Starlark(expected_message.to_string());
+    assert_eq!(expected_err, err);
+}
+
 #[test]
 fn strictest_decision_wins_across_matches() {
     let policy_src = r#"