feat: generalized ignore comments (#200)

Author: woodruffw

Diff for: Cargo.lock

@@ -32,7 +32,7 @@ serde-sarif = "0.6.5"
 serde_json = "1.0.133"
 serde_yaml = "0.9.34"
 terminal-link = "0.1.0"
-yamlpath = "0.11.1"
+yamlpath = "0.12.0"
 
 [profile.release]
 lto = true

Diff for: Cargo.toml

@@ -112,21 +112,14 @@ choose to *selectively ignore* results via either special ignore comments
 
     Ignore comment support was added in `v0.6.0`.
 
-!!! tip
-
-    Ignore comments are currently limited to a single rule at a time.
-
-    This means that you **can't currently** write something like
-    `zizmor: ignore[*]` or `zizmor: ignore[foo,bar]` to ignore more than
-    one thing per line.
-
-    If support for more generalized ignore comments is important to you,
-    see #189 for ongoing discussion about design.
-
 Findings can be ignored inline with `# zizmor: ignore[rulename]` comments.
 This is ideal for one-off ignores, where a whole `zizmor.yml` configuration
 file would be too heavyweight.
 
+Multiple different audits can be ignored with a single comment by
+separating each rule with a comma, e.g.
+`# zizmor: ignore[artipacked,ref-confusion]`.
+
 These comments can be placed anywhere in any span identified by a finding.
 
 For example, to ignore a single `artipacked` finding:

Diff for: docs/usage.md

@@ -3,7 +3,7 @@
 
 use anyhow::Result;
 
-use super::{ConcreteLocation, Feature, SymbolicLocation};
+use super::{Comment, ConcreteLocation, Feature, SymbolicLocation};
 use crate::models::Workflow;
 
 pub(crate) struct Locator {}
@@ -51,6 +51,12 @@ impl Locator {
             location: ConcreteLocation::from(&feature.location),
             parent_location: ConcreteLocation::from(&parent_feature.location),
             feature: workflow.document.extract_with_leading_whitespace(&feature),
+            comments: workflow
+                .document
+                .feature_comments(&feature)
+                .into_iter()
+                .map(Comment)
+                .collect(),
             parent_feature: workflow
                 .document
                 .extract_with_leading_whitespace(&parent_feature),

Diff for: src/finding/locate.rs

@@ -1,10 +1,11 @@
 //! Models and APIs for handling findings and their locations.
 
-use std::borrow::Cow;
+use std::{borrow::Cow, sync::LazyLock};
 
 use anyhow::Result;
 use clap::ValueEnum;
 use locate::Locator;
+use regex::Regex;
 use serde::Serialize;
 use terminal_link::Link;
 
@@ -186,6 +187,29 @@ impl From<&yamlpath::Location> for ConcreteLocation {
     }
 }
 
+static IGNORE_EXPR: LazyLock<Regex> =
+    LazyLock::new(|| Regex::new(r"^# zizmor: ignore\[(.+)\]\s*$").unwrap());
+
+/// Represents a single source comment.
+#[derive(Serialize)]
+#[serde(transparent)]
+pub(crate) struct Comment<'w>(&'w str);
+
+impl<'w> Comment<'w> {
+    fn ignores(&self, rule_id: &str) -> bool {
+        // Extracts foo,bar from `# zizmor: ignore[foo,bar]`
+        let Some(caps) = IGNORE_EXPR.captures(self.0) else {
+            return false;
+        };
+
+        caps.get(1)
+            .unwrap()
+            .as_str()
+            .split(",")
+            .any(|r| r.trim() == rule_id)
+    }
+}
+
 /// An extracted feature, along with its concrete location.
 #[derive(Serialize)]
 pub(crate) struct Feature<'w> {
@@ -200,6 +224,9 @@ pub(crate) struct Feature<'w> {
     /// The feature's textual content.
     pub(crate) feature: &'w str,
 
+    /// Any comments within the feature's span.
+    pub(crate) comments: Vec<Comment<'w>>,
+
     /// The feature's parent's textual content.
     pub(crate) parent_feature: &'w str,
 }
@@ -273,7 +300,7 @@ impl<'w> FindingBuilder<'w> {
             .map(|l| l.clone().concretize(workflow))
             .collect::<Result<Vec<_>>>()?;
 
-        let should_ignore = self.ignored_from_inlined_comment(workflow, &locations, self.ident);
+        let should_ignore = self.ignored_from_inlined_comment(&locations, self.ident);
 
         Ok(Finding {
             ident: self.ident,
@@ -288,34 +315,54 @@ impl<'w> FindingBuilder<'w> {
         })
     }
 
-    fn ignored_from_inlined_comment(
-        &self,
-        workflow: &Workflow,
-        locations: &[Location],
-        id: &str,
-    ) -> bool {
-        let document_lines = &workflow.document.source().lines().collect::<Vec<_>>();
-        let line_ranges = locations
+    fn ignored_from_inlined_comment(&self, locations: &[Location], id: &str) -> bool {
+        locations
             .iter()
-            .map(|l| {
-                (
-                    l.concrete.location.start_point.row,
-                    l.concrete.location.end_point.row,
-                )
-            })
-            .collect::<Vec<_>>();
-
-        let inlined_ignore = format!("# zizmor: ignore[{}]", id);
-        for (start, end) in line_ranges {
-            for document_line in start..(end + 1) {
-                if let Some(line) = document_lines.get(document_line) {
-                    if line.rfind(&inlined_ignore).is_some() {
-                        return true;
-                    }
-                }
-            }
-        }
+            .flat_map(|l| &l.concrete.comments)
+            .any(|c| c.ignores(id))
+    }
+}
 
-        false
+#[cfg(test)]
+mod tests {
+    use crate::finding::Comment;
+
+    #[test]
+    fn test_comment_ignores() {
+        let cases = &[
+            // Trivial cases.
+            ("# zizmor: ignore[foo]", "foo", true),
+            ("# zizmor: ignore[foo,bar]", "foo", true),
+            // Dashes are OK.
+            ("# zizmor: ignore[foo,bar,foo-bar]", "foo-bar", true),
+            // Spaces are OK.
+            ("# zizmor: ignore[foo, bar,   foo-bar]", "foo-bar", true),
+            // Extra commas and duplicates are nonsense but OK.
+            ("# zizmor: ignore[foo,foo,,foo,,,,foo,]", "foo", true),
+            // Valid ignore, but not a match.
+            ("# zizmor: ignore[foo,bar]", "baz", false),
+            // Invalid ignore: empty rule list.
+            ("# zizmor: ignore[]", "", false),
+            ("# zizmor: ignore[]", "foo", false),
+            // Invalid ignore: no commas.
+            ("# zizmor: ignore[foo bar]", "foo", false),
+            // Invalid ignore: missing opening and/or closing [].
+            ("# zizmor: ignore[foo", "foo", false),
+            ("# zizmor: ignore foo", "foo", false),
+            ("# zizmor: ignore foo]", "foo", false),
+            // Invalid ignore: space after # and : is mandatory and fixed.
+            ("# zizmor:ignore[foo]", "foo", false),
+            ("#zizmor: ignore[foo]", "foo", false),
+            ("#  zizmor: ignore[foo]", "foo", false),
+            ("#  zizmor:  ignore[foo]", "foo", false),
+        ];
+
+        for (comment, rule, ignores) in cases {
+            assert_eq!(
+                Comment(comment).ignores(rule),
+                *ignores,
+                "{comment} does not ignore {rule}"
+            )
+        }
     }
 }