Merge pull request #2171 from Urgau/mentions-content-changes

Add way to do mentions of content changes

Author: Urgau

src/config.rs

@@ -225,12 +225,23 @@ pub(crate) struct ConcernConfig {
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
 pub(crate) struct MentionsConfig {
     #[serde(flatten)]
-    pub(crate) paths: HashMap<String, MentionsPathConfig>,
+    pub(crate) entries: HashMap<String, MentionsEntryConfig>,
+}
+
+#[derive(PartialEq, Eq, Debug, Default, serde::Deserialize)]
+#[serde(rename_all = "kebab-case")]
+pub(crate) enum MentionsEntryType {
+    #[default]
+    Filename,
+    Content,
 }
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
 #[serde(deny_unknown_fields)]
-pub(crate) struct MentionsPathConfig {
+pub(crate) struct MentionsEntryConfig {
+    #[serde(alias = "type")]
+    #[serde(default)]
+    pub(crate) type_: MentionsEntryType,
     pub(crate) message: Option<String>,
     #[serde(default)]
     pub(crate) cc: Vec<String>,
@@ -661,7 +672,7 @@ mod tests {
 
     #[test]
     fn sample() {
-        let config = r#"
+        let config = r##"
             [relabel]
             allow-unauthenticated = [
                 "C-*"
@@ -692,6 +703,18 @@ mod tests {
             core = "T-core"
             infra = "T-infra"
 
+            [mentions."src/"]
+            cc = ["@someone"]
+            
+            [mentions."target/"]
+            message = "This is a message."
+            cc = ["@someone"]
+            
+            [mentions."#[rustc_attr]"]
+            type = "content"
+            message = "This is a message."
+            cc = ["@someone"]
+
             [shortcut]
 
             [issue-links]
@@ -713,7 +736,7 @@ mod tests {
             [range-diff]
 
             [review-changes-since]
-        "#;
+        "##;
         let config = toml::from_str::<Config>(&config).unwrap();
         let mut ping_teams = HashMap::new();
         ping_teams.insert(
@@ -780,7 +803,34 @@ mod tests {
                 github_releases: None,
                 review_submitted: None,
                 review_requested: None,
-                mentions: None,
+                mentions: Some(MentionsConfig {
+                    entries: HashMap::from([
+                        (
+                            "src/".to_string(),
+                            MentionsEntryConfig {
+                                type_: MentionsEntryType::Filename,
+                                message: None,
+                                cc: vec!["@someone".to_string()]
+                            }
+                        ),
+                        (
+                            "target/".to_string(),
+                            MentionsEntryConfig {
+                                type_: MentionsEntryType::Filename,
+                                message: Some("This is a message.".to_string()),
+                                cc: vec!["@someone".to_string()]
+                            }
+                        ),
+                        (
+                            "#[rustc_attr]".to_string(),
+                            MentionsEntryConfig {
+                                type_: MentionsEntryType::Content,
+                                message: Some("This is a message.".to_string()),
+                                cc: vec!["@someone".to_string()]
+                            }
+                        )
+                    ])
+                }),
                 no_merges: None,
                 pr_tracking: None,
                 transfer: None,

src/handlers/mentions.rs

@@ -3,26 +3,28 @@
 //! interested people.
 
 use crate::{
-    config::{MentionsConfig, MentionsPathConfig},
+    config::{MentionsConfig, MentionsEntryConfig, MentionsEntryType},
     db::issue_data::IssueData,
     github::{IssuesAction, IssuesEvent},
     handlers::Context,
 };
 use anyhow::Context as _;
+use itertools::Itertools;
 use serde::{Deserialize, Serialize};
-use std::fmt::Write;
 use std::path::Path;
+use std::{fmt::Write, path::PathBuf};
 use tracing as log;
 
 const MENTIONS_KEY: &str = "mentions";
 
 pub(super) struct MentionsInput {
-    paths: Vec<String>,
+    to_mention: Vec<(String, Vec<PathBuf>)>,
 }
 
 #[derive(Debug, Default, Deserialize, Serialize, Clone, PartialEq)]
 struct MentionState {
-    paths: Vec<String>,
+    #[serde(alias = "paths")]
+    entries: Vec<String>,
 }
 
 pub(super) async fn parse_input(
@@ -61,23 +63,42 @@ pub(super) async fn parse_input(
     {
         let file_paths: Vec<_> = files.iter().map(|fd| Path::new(&fd.filename)).collect();
         let to_mention: Vec<_> = config
-            .paths
+            .entries
             .iter()
-            .filter(|(path, MentionsPathConfig { cc, .. })| {
-                let path = Path::new(path);
-                // Only mention matching paths.
-                let touches_relevant_files = file_paths.iter().any(|p| p.starts_with(path));
+            .filter_map(|(entry, MentionsEntryConfig { cc, type_, .. })| {
+                let relevant_file_paths: Vec<PathBuf> = match type_ {
+                    MentionsEntryType::Filename => {
+                        let path = Path::new(entry);
+                        // Only mention matching paths.
+                        file_paths
+                            .iter()
+                            .filter(|p| p.starts_with(path))
+                            .map(|p| PathBuf::from(p))
+                            .collect()
+                    }
+                    MentionsEntryType::Content => {
+                        // Only mentions byte-for-byte matching content inside the patch.
+                        files
+                            .iter()
+                            .filter(|f| patch_contains(&f.patch, &**entry))
+                            .map(|f| PathBuf::from(&f.filename))
+                            .collect()
+                    }
+                };
                 // Don't mention if only the author is in the list.
                 let pings_non_author = match &cc[..] {
                     [only_cc] => only_cc.trim_start_matches('@') != &event.issue.user.login,
                     _ => true,
                 };
-                touches_relevant_files && pings_non_author
+                if !relevant_file_paths.is_empty() && pings_non_author {
+                    Some((entry.to_string(), relevant_file_paths))
+                } else {
+                    None
+                }
             })
-            .map(|(key, _mention)| key.to_string())
             .collect();
         if !to_mention.is_empty() {
-            return Ok(Some(MentionsInput { paths: to_mention }));
+            return Ok(Some(MentionsInput { to_mention }));
         }
     }
     Ok(None)
@@ -94,23 +115,36 @@ pub(super) async fn handle_input(
         IssueData::load(&mut client, &event.issue, MENTIONS_KEY).await?;
     // Build the message to post to the issue.
     let mut result = String::new();
-    for to_mention in &input.paths {
-        if state.data.paths.iter().any(|p| p == to_mention) {
+    for (entry, relevant_file_paths) in input.to_mention {
+        if state.data.entries.iter().any(|e| e == &entry) {
             // Avoid duplicate mentions.
             continue;
         }
-        let MentionsPathConfig { message, cc } = &config.paths[to_mention];
+        let MentionsEntryConfig { message, cc, type_ } = &config.entries[&entry];
         if !result.is_empty() {
             result.push_str("\n\n");
         }
         match message {
             Some(m) => result.push_str(m),
-            None => write!(result, "Some changes occurred in {to_mention}").unwrap(),
+            None => match type_ {
+                MentionsEntryType::Filename => {
+                    write!(result, "Some changes occurred in {entry}").unwrap()
+                }
+                MentionsEntryType::Content => write!(
+                    result,
+                    "Some changes regarding `{entry}` occurred in {}",
+                    relevant_file_paths
+                        .iter()
+                        .map(|f| f.to_string_lossy())
+                        .join(", ")
+                )
+                .unwrap(),
+            },
         }
         if !cc.is_empty() {
             write!(result, "\n\ncc {}", cc.join(", ")).unwrap();
         }
-        state.data.paths.push(to_mention.to_string());
+        state.data.entries.push(entry);
     }
     if !result.is_empty() {
         event
@@ -122,3 +156,64 @@ pub(super) async fn handle_input(
     }
     Ok(())
 }
+
+fn patch_contains(patch: &str, needle: &str) -> bool {
+    for line in patch.lines() {
+        if (!line.starts_with("+++") && line.starts_with('+'))
+            || (!line.starts_with("---") && line.starts_with('-'))
+        {
+            if line.contains(needle) {
+                return true;
+            }
+        }
+    }
+
+    false
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn finds_added_line() {
+        let patch = "\
+--- a/file.txt
++++ b/file.txt
++hello world
+ context line
+";
+        assert!(patch_contains(patch, "hello"));
+    }
+
+    #[test]
+    fn finds_removed_line() {
+        let patch = "\
+--- a/file.txt
++++ b/file.txt
+-old value
++new value
+";
+        assert!(patch_contains(patch, "old value"));
+    }
+
+    #[test]
+    fn ignores_diff_headers() {
+        let patch = "\
+--- a/file.txt
++++ b/file.txt
+ context line
+";
+        assert!(!patch_contains(patch, "file.txt")); // should *not* match header
+    }
+
+    #[test]
+    fn needle_not_present() {
+        let patch = "\
+--- a/file.txt
++++ b/file.txt
++added line
+";
+        assert!(!patch_contains(patch, "missing"));
+    }
+}