Merge pull request #1860 from Urgau/warn_default_branch-exceptions

Add support for exceptions in non default branch warning

Author: ehuss

src/config.rs

@@ -91,10 +91,10 @@ pub(crate) struct PingTeamConfig {
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
 #[serde(deny_unknown_fields)]
 pub(crate) struct AssignConfig {
-    /// If `true`, then posts a warning comment if the PR is opened against a
+    /// If enabled, then posts a warning comment if the PR is opened against a
     /// different branch than the default (usually master or main).
     #[serde(default)]
-    pub(crate) warn_non_default_branch: bool,
+    pub(crate) warn_non_default_branch: WarnNonDefaultBranchConfig,
     /// A URL to include in the welcome message.
     pub(crate) contributing_url: Option<String>,
     /// Ad-hoc groups that can be referred to in `owners`.
@@ -118,6 +118,45 @@ impl AssignConfig {
     }
 }
 
+#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
+#[serde(deny_unknown_fields)]
+#[serde(untagged)]
+pub(crate) enum WarnNonDefaultBranchConfig {
+    Simple(bool),
+    Extended {
+        enable: bool,
+        /// List of exceptions that have a different default branch
+        #[serde(default)]
+        exceptions: Vec<WarnNonDefaultBranchException>,
+    },
+}
+
+#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
+#[serde(deny_unknown_fields)]
+pub(crate) struct WarnNonDefaultBranchException {
+    /// Substring in the title that match this exception
+    pub(crate) title: String,
+    /// The actual branch that should be associated with the issue
+    pub(crate) branch: String,
+}
+
+impl Default for WarnNonDefaultBranchConfig {
+    fn default() -> WarnNonDefaultBranchConfig {
+        WarnNonDefaultBranchConfig::Simple(false)
+    }
+}
+
+impl WarnNonDefaultBranchConfig {
+    pub(crate) fn enabled_and_exceptions(&self) -> Option<&[WarnNonDefaultBranchException]> {
+        match self {
+            WarnNonDefaultBranchConfig::Simple(enable) => enable.then(|| &[] as &[_]),
+            WarnNonDefaultBranchConfig::Extended { enable, exceptions } => {
+                enable.then(|| exceptions.as_slice())
+            }
+        }
+    }
+}
+
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
 #[serde(deny_unknown_fields)]
 pub(crate) struct NoMergesConfig {
@@ -514,7 +553,7 @@ mod tests {
                     allow_unauthenticated: vec!["C-*".into()],
                 }),
                 assign: Some(AssignConfig {
-                    warn_non_default_branch: false,
+                    warn_non_default_branch: WarnNonDefaultBranchConfig::Simple(false),
                     contributing_url: None,
                     adhoc_groups: HashMap::new(),
                     owners: HashMap::new(),
@@ -544,4 +583,65 @@ mod tests {
             }
         );
     }
+
+    #[test]
+    fn warn_non_default_branch() {
+        let config = r#"
+            [assign]
+            warn_non_default_branch.enable = true
+
+            [[assign.warn_non_default_branch.exceptions]]
+            title = "[beta"
+            branch = "beta"
+
+            [[assign.warn_non_default_branch.exceptions]]
+            title = "[stable"
+            branch = "stable"
+        "#;
+        let config = toml::from_str::<Config>(&config).unwrap();
+        assert_eq!(
+            config,
+            Config {
+                relabel: None,
+                assign: Some(AssignConfig {
+                    warn_non_default_branch: WarnNonDefaultBranchConfig::Extended {
+                        enable: true,
+                        exceptions: vec![
+                            WarnNonDefaultBranchException {
+                                title: "[beta".to_string(),
+                                branch: "beta".to_string()
+                            },
+                            WarnNonDefaultBranchException {
+                                title: "[stable".to_string(),
+                                branch: "stable".to_string()
+                            },
+                        ],
+                    },
+                    contributing_url: None,
+                    adhoc_groups: HashMap::new(),
+                    owners: HashMap::new(),
+                    users_on_vacation: HashSet::new(),
+                }),
+                note: None,
+                ping: None,
+                nominate: None,
+                shortcut: None,
+                prioritize: None,
+                major_change: None,
+                glacier: None,
+                close: None,
+                autolabel: None,
+                notify_zulip: None,
+                github_releases: None,
+                review_submitted: None,
+                review_requested: None,
+                mentions: None,
+                no_merges: None,
+                validate_config: Some(ValidateConfig {}),
+                pr_tracking: None,
+                transfer: None,
+                merge_conflicts: None,
+            }
+        );
+    }
 }

src/handlers/assign.rs

@@ -18,7 +18,7 @@
 //! the PR modifies.
 
 use crate::{
-    config::AssignConfig,
+    config::{AssignConfig, WarnNonDefaultBranchException},
     github::{self, Event, FileDiff, Issue, IssuesAction, Selection},
     handlers::{Context, GithubClient, IssuesEvent},
     interactions::EditIssueBody,
@@ -73,6 +73,11 @@ const NON_DEFAULT_BRANCH: &str =
      but this one is against {target}. \
      Please double check that you specified the right target!";
 
+const NON_DEFAULT_BRANCH_EXCEPTION: &str =
+    "Pull requests targetting the {default} branch are usually filed against the {default} \
+     branch, but this one is against {target}. \
+     Please double check that you specified the right target!";
+
 const SUBMODULE_WARNING_MSG: &str = "These commits modify **submodules**.";
 
 fn on_vacation_msg(user: &str) -> String {
@@ -179,8 +184,8 @@ pub(super) async fn handle_input(
 
     // Compute some warning messages to post to new PRs.
     let mut warnings = Vec::new();
-    if config.warn_non_default_branch {
-        warnings.extend(non_default_branch(event));
+    if let Some(exceptions) = config.warn_non_default_branch.enabled_and_exceptions() {
+        warnings.extend(non_default_branch(exceptions, event));
     }
     warnings.extend(modifies_submodule(diff));
     if !warnings.is_empty() {
@@ -209,15 +214,25 @@ fn is_self_assign(assignee: &str, pr_author: &str) -> bool {
     assignee.to_lowercase() == pr_author.to_lowercase()
 }
 
-/// Returns a message if the PR is opened against the non-default branch.
-fn non_default_branch(event: &IssuesEvent) -> Option<String> {
+/// Returns a message if the PR is opened against the non-default branch (or the exception branch
+/// if it's an exception).
+fn non_default_branch(
+    exceptions: &[WarnNonDefaultBranchException],
+    event: &IssuesEvent,
+) -> Option<String> {
     let target_branch = &event.issue.base.as_ref().unwrap().git_ref;
-    let default_branch = &event.repository.default_branch;
+    let (default_branch, warn_msg) = exceptions
+        .iter()
+        .find(|e| event.issue.title.contains(&e.title))
+        .map_or_else(
+            || (&event.repository.default_branch, NON_DEFAULT_BRANCH),
+            |e| (&e.branch, NON_DEFAULT_BRANCH_EXCEPTION),
+        );
     if target_branch == default_branch {
         return None;
     }
     Some(
-        NON_DEFAULT_BRANCH
+        warn_msg
             .replace("{default}", default_branch)
             .replace("{target}", target_branch),
     )