Merge pull request #1917 from Urgau/no_mentions

Add no GitHub mentions in commits handler

Author: Kobzol

Diff for: src/config.rs

@@ -47,6 +47,7 @@ pub(crate) struct Config {
     pub(crate) bot_pull_requests: Option<BotPullRequests>,
     pub(crate) rendered_link: Option<RenderedLinkConfig>,
     pub(crate) canonicalize_issue_links: Option<CanonicalizeIssueLinksConfig>,
+    pub(crate) no_mentions: Option<NoMentionsConfig>,
 }
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
@@ -420,6 +421,11 @@ pub(crate) struct RenderedLinkConfig {
 #[serde(deny_unknown_fields)]
 pub(crate) struct CanonicalizeIssueLinksConfig {}
 
+#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
+#[serde(rename_all = "kebab-case")]
+#[serde(deny_unknown_fields)]
+pub(crate) struct NoMentionsConfig {}
+
 fn get_cached_config(repo: &str) -> Option<Result<Arc<Config>, ConfigurationError>> {
     let cache = CONFIG_CACHE.read().unwrap();
     cache.get(repo).and_then(|(config, fetch_time)| {
@@ -545,6 +551,8 @@ mod tests {
 
             [rendered-link]
             trigger-files = ["posts/"]
+
+            [no-mentions]
         "#;
         let config = toml::from_str::<Config>(&config).unwrap();
         let mut ping_teams = HashMap::new();
@@ -608,6 +616,7 @@ mod tests {
                     trigger_files: vec!["posts/".to_string()]
                 }),
                 canonicalize_issue_links: Some(CanonicalizeIssueLinksConfig {}),
+                no_mentions: Some(NoMentionsConfig {}),
             }
         );
     }
@@ -671,7 +680,8 @@ mod tests {
                 merge_conflicts: None,
                 bot_pull_requests: None,
                 rendered_link: None,
-                canonicalize_issue_links: None
+                canonicalize_issue_links: None,
+                no_mentions: None,
             }
         );
     }

Diff for: src/handlers/check_commits.rs

@@ -8,6 +8,7 @@ use crate::{
 };
 
 mod modified_submodule;
+mod no_mentions;
 mod non_default_branch;
 
 /// Key for the state in the database
@@ -41,6 +42,7 @@ pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> any
             event.issue.number
         )
     };
+    let commits = event.issue.commits(&ctx.github).await?;
 
     let mut warnings = Vec::new();
 
@@ -58,6 +60,10 @@ pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> any
         warnings.extend(modified_submodule::modifies_submodule(diff));
     }
 
+    if let Some(no_mentions) = &config.no_mentions {
+        warnings.extend(no_mentions::mentions_in_commits(no_mentions, &commits));
+    }
+
     handle_warnings(ctx, event, warnings).await
 }
 
@@ -88,12 +94,7 @@ async fn handle_warnings(
                 .await?;
         }
 
-        // Format the warnings for user consumption on Github
-        let warnings: Vec<_> = warnings
-            .iter()
-            .map(|warning| format!("* {warning}"))
-            .collect();
-        let warning = format!(":warning: **Warning** :warning:\n\n{}", warnings.join("\n"));
+        let warning = warning_from_warnings(&warnings);
         let comment = event.issue.post_comment(&ctx.github, &warning).await?;
 
         // Save new state in the database
@@ -120,3 +121,12 @@ async fn handle_warnings(
 
     Ok(())
 }
+
+// Format the warnings for user consumption on Github
+fn warning_from_warnings(warnings: &[String]) -> String {
+    let warnings: Vec<_> = warnings
+        .iter()
+        .map(|warning| format!("* {warning}"))
+        .collect();
+    format!(":warning: **Warning** :warning:\n\n{}", warnings.join("\n"))
+}

Diff for: src/handlers/check_commits/modified_submodule.rs

@@ -1,6 +1,6 @@
 use crate::github::FileDiff;
 
-const SUBMODULE_WARNING_MSG: &str = "These commits modify **submodules**.";
+const SUBMODULE_WARNING_MSG: &str = "Some commits in this PR modify **submodules**.";
 
 /// Returns a message if the PR modifies a git submodule.
 pub(super) fn modifies_submodule(diff: &[FileDiff]) -> Option<String> {

Diff for: src/handlers/check_commits/no_mentions.rs

@@ -0,0 +1,78 @@
+//! Purpose: When opening a PR, or pushing new changes, check for github mentions
+//! in commits and notify the user of our no-mentions in commits policy.
+
+use std::fmt::Write;
+
+use crate::{config::NoMentionsConfig, github::GithubCommit};
+
+pub(super) fn mentions_in_commits(
+    _conf: &NoMentionsConfig,
+    commits: &[GithubCommit],
+) -> Option<String> {
+    let mut mentions_commits = Vec::new();
+
+    for commit in commits {
+        if !parser::get_mentions(&commit.commit.message).is_empty() {
+            mentions_commits.push(&*commit.sha);
+        }
+    }
+
+    if mentions_commits.is_empty() {
+        None
+    } else {
+        Some(mentions_in_commits_warn(mentions_commits))
+    }
+}
+
+fn mentions_in_commits_warn(commits: Vec<&str>) -> String {
+    let mut warning = format!("There are username mentions (such as `@user`) in the commit messages of the following commits.\n  *Please remove the mentions to avoid spamming these users.*\n");
+
+    for commit in commits {
+        let _ = writeln!(warning, "    - {commit}");
+    }
+
+    warning
+}
+
+#[test]
+fn test_mentions_in_commits() {
+    fn dummy_commit_from_body(sha: &str, body: &str) -> GithubCommit {
+        use chrono::{DateTime, FixedOffset};
+
+        GithubCommit {
+            sha: sha.to_string(),
+            commit: crate::github::GithubCommitCommitField {
+                author: crate::github::GitUser {
+                    date: DateTime::<FixedOffset>::MIN_UTC.into(),
+                },
+                message: body.to_string(),
+                tree: crate::github::GitCommitTree {
+                    sha: "60ff73dfdd81aa1e6737eb3dacdfd4a141f6e14d".to_string(),
+                },
+            },
+            parents: vec![],
+        }
+    }
+
+    let mut commits = vec![dummy_commit_from_body(
+        "d1992a392617dfb10518c3e56446b6c9efae38b0",
+        "This is simple without mentions!",
+    )];
+
+    assert_eq!(mentions_in_commits(&NoMentionsConfig {}, &commits), None);
+
+    commits.push(dummy_commit_from_body(
+        "d7daa17bc97df9377640b0d33cbd0bbeed703c3a",
+        "This is a body with a @mention!",
+    ));
+
+    assert_eq!(
+        mentions_in_commits(&NoMentionsConfig {}, &commits),
+        Some(
+            r#"There are username mentions (such as `@user`) in the commit messages of the following commits.
+  *Please remove the mentions to avoid spamming these users.*
+    - d7daa17bc97df9377640b0d33cbd0bbeed703c3a
+"#.to_string()
+        )
+    );
+}