diff --git a/parser/src/command.rs b/parser/src/command.rs index ecdd2ec86..fb3910b6e 100644 --- a/parser/src/command.rs +++ b/parser/src/command.rs @@ -5,6 +5,7 @@ use regex::Regex; pub mod assign; pub mod close; +pub mod concern; pub mod nominate; pub mod note; pub mod ping; @@ -25,6 +26,7 @@ pub enum Command<'a> { Shortcut(Result>), Close(Result>), Note(Result>), + Concern(Result>), Transfer(Result>), } @@ -97,6 +99,11 @@ impl<'a> Input<'a> { Command::Note, &original_tokenizer, )); + success.extend(parse_single_command( + concern::ConcernCommand::parse, + Command::Concern, + &original_tokenizer, + )); success.extend(parse_single_command( ping::PingCommand::parse, Command::Ping, @@ -206,6 +213,7 @@ impl<'a> Command<'a> { Command::Shortcut(r) => r.is_ok(), Command::Close(r) => r.is_ok(), Command::Note(r) => r.is_ok(), + Command::Concern(r) => r.is_ok(), Command::Transfer(r) => r.is_ok(), } } @@ -353,3 +361,27 @@ fn review_ignored() { assert_eq!(input.next(), None); } } + +#[test] +fn concern() { + let input = "@bot concern this is my concern"; + let mut input = Input::new(input, vec!["bot"]); + assert_eq!( + input.next(), + Some(Command::Concern(Ok(concern::ConcernCommand::Concern { + title: "this is my concern".to_string() + }))) + ); +} + +#[test] +fn resolve() { + let input = "@bot resolve this is my concern"; + let mut input = Input::new(input, vec!["bot"]); + assert_eq!( + input.next(), + Some(Command::Concern(Ok(concern::ConcernCommand::Resolve { + title: "this is my concern".to_string() + }))) + ); +} diff --git a/parser/src/command/concern.rs b/parser/src/command/concern.rs new file mode 100644 index 000000000..5c95a7454 --- /dev/null +++ b/parser/src/command/concern.rs @@ -0,0 +1,48 @@ +use crate::error::Error; +use crate::token::{Token, Tokenizer}; +use std::fmt; + +#[derive(PartialEq, Eq, Debug)] +pub enum ConcernCommand { + Concern { title: String }, + Resolve { title: String }, +} + +#[derive(PartialEq, Eq, Debug)] +pub enum ParseError { + MissingTitle, +} + +impl std::error::Error for ParseError {} +impl fmt::Display for ParseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + ParseError::MissingTitle => write!(f, "missing required title"), + } + } +} + +impl ConcernCommand { + pub fn parse<'a>(input: &mut Tokenizer<'a>) -> Result, Error<'a>> { + let mut toks = input.clone(); + if let Some(Token::Word(action @ ("concern" | "resolve"))) = toks.peek_token()? { + toks.next_token()?; + + let title = toks.take_line()?.trim().to_string(); + + if title.is_empty() { + return Err(toks.error(ParseError::MissingTitle)); + } + + let command = if action == "resolve" { + ConcernCommand::Resolve { title } + } else { + ConcernCommand::Concern { title } + }; + + Ok(Some(command)) + } else { + Ok(None) + } + } +} diff --git a/src/config.rs b/src/config.rs index de2462b4a..ddae9bc97 100644 --- a/src/config.rs +++ b/src/config.rs @@ -35,6 +35,7 @@ pub(crate) struct Config { pub(crate) review_requested: Option, pub(crate) shortcut: Option, pub(crate) note: Option, + pub(crate) concern: Option, pub(crate) mentions: Option, pub(crate) no_merges: Option, // We want this validation to run even without the entry in the config file @@ -191,6 +192,14 @@ pub(crate) struct NoteConfig { _empty: (), } +#[derive(PartialEq, Eq, Debug, serde::Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct ConcernConfig { + /// Set the labels on the PR when concerns are active. + #[serde(default)] + pub(crate) labels: Vec, +} + #[derive(PartialEq, Eq, Debug, serde::Deserialize)] pub(crate) struct MentionsConfig { #[serde(flatten)] @@ -594,6 +603,9 @@ mod tests { [note] + [concern] + labels = ["has-concerns"] + [ping.compiler] message = """\ So many people!\ @@ -691,6 +703,9 @@ mod tests { behind_upstream: Some(BehindUpstreamConfig { days_threshold: Some(14), }), + concern: Some(ConcernConfig { + labels: vec!["has-concerns".to_string()], + }), } ); } @@ -742,6 +757,7 @@ mod tests { }), note: None, ping: None, + concern: None, nominate: None, shortcut: None, prioritize: None, diff --git a/src/github.rs b/src/github.rs index 357a14f1c..1be76e761 100644 --- a/src/github.rs +++ b/src/github.rs @@ -463,6 +463,7 @@ pub struct Comment { pub updated_at: chrono::DateTime, #[serde(default, rename = "state")] pub pr_review_state: Option, + pub author_association: AuthorAssociation, } #[derive(Debug, serde::Deserialize, serde::Serialize, Eq, PartialEq)] diff --git a/src/handlers.rs b/src/handlers.rs index 90e5a6a04..d97de507b 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -29,6 +29,7 @@ mod autolabel; mod bot_pull_requests; mod check_commits; mod close; +mod concern; pub mod docs_update; mod github_releases; mod issue_links; @@ -365,6 +366,7 @@ command_handlers! { shortcut: Shortcut, close: Close, note: Note, + concern: Concern, transfer: Transfer, } diff --git a/src/handlers/concern.rs b/src/handlers/concern.rs new file mode 100644 index 000000000..596664525 --- /dev/null +++ b/src/handlers/concern.rs @@ -0,0 +1,223 @@ +use std::fmt::Write; + +use anyhow::{bail, Context as _}; +use octocrab::models::AuthorAssociation; + +use crate::{ + config::ConcernConfig, + github::{Event, Label}, + handlers::Context, + interactions::EditIssueBody, +}; +use parser::command::concern::ConcernCommand; + +const CONCERN_ISSUE_KEY: &str = "CONCERN-ISSUE"; + +#[derive(Debug, PartialEq, Eq, Default, serde::Serialize, serde::Deserialize)] +struct ConcernData { + concerns: Vec, +} + +#[derive(Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +struct Concern { + title: String, + author: String, + comment_url: String, + status: ConcernStatus, +} + +#[derive(Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +enum ConcernStatus { + Active, + Resolved { comment_url: String }, +} + +pub(super) async fn handle_command( + ctx: &Context, + config: &ConcernConfig, + event: &Event, + cmd: ConcernCommand, +) -> anyhow::Result<()> { + let Event::IssueComment(issue_comment) = event else { + bail!("concern issued on something other than a issue") + }; + let Some(comment_url) = event.html_url() else { + bail!("unable to retrieve the comment url") + }; + let author = event.user().login.to_owned(); + let issue = &issue_comment.issue; + + // Verify that this issue isn't a rfcbot FCP, skip if it is + match crate::rfcbot::get_all_fcps().await { + Ok(fcps) => { + if fcps.iter().any(|(_, fcp)| { + fcp.issue.number as u64 == issue.number + && fcp.issue.repository == issue_comment.repository.full_name + }) { + tracing::info!( + "{}#{} tried to register a concern, blocked by our rfcbot FCP check", + issue_comment.repository.full_name, + issue.number, + ); + return Ok(()); + } + } + Err(err) => { + tracing::warn!( + "unable to fetch rfcbot active FCPs: {:?}, skipping check", + err + ); + } + } + + // Verify that the comment author is at least a member of the org, error if it's not + if !matches!( + issue_comment.comment.author_association, + AuthorAssociation::Member | AuthorAssociation::Owner + ) { + issue + .post_comment( + &ctx.github, + "Only organization members can add or resolve concerns.", + ) + .await?; + return Ok(()); + } + + let edit = EditIssueBody::new(&issue, CONCERN_ISSUE_KEY); + let mut concern_data: ConcernData = edit.current_data().unwrap_or_default(); + + // Process the command by either adding a new comment or "deactivating" the old one + match cmd { + ConcernCommand::Concern { title } => concern_data.concerns.push(Concern { + title, + author, + status: ConcernStatus::Active, + comment_url: comment_url.to_string(), + }), + ConcernCommand::Resolve { title } => concern_data + .concerns + .iter_mut() + .filter(|c| c.title == title) + .for_each(|c| { + c.status = ConcernStatus::Resolved { + comment_url: comment_url.to_string(), + } + }), + } + + // Create the new markdown content listing all the concerns + let new_content = markdown_content(&concern_data.concerns); + + // Add or remove the labels + if concern_data + .concerns + .iter() + .any(|c| matches!(c.status, ConcernStatus::Active)) + { + if let Err(err) = issue + .add_labels( + &ctx.github, + config + .labels + .iter() + .map(|l| Label { + name: l.to_string(), + }) + .collect(), + ) + .await + { + tracing::error!("unable to add concern labels: {:?}", err); + let labels = config.labels.join(", "); + issue.post_comment( + &ctx.github, + &format!("*Psst, I was unable to add the labels ({labels}), could someone do it for me.*"), + ).await?; + } + } else { + for l in &config.labels { + issue.remove_label(&ctx.github, &l).await?; + } + } + + // Apply the new markdown concerns list to the issue + edit.apply(&ctx.github, new_content, concern_data) + .await + .context("failed to apply the new concerns section markdown")?; + + Ok(()) +} + +fn markdown_content(concerns: &[Concern]) -> String { + if concerns.is_empty() { + return "".to_string(); + } + + let mut md = String::new(); + + let _ = writeln!(md, "\n# Concerns"); + let _ = writeln!(md, ""); + + for &Concern { + ref title, + ref author, + ref status, + ref comment_url, + } in concerns + { + let _ = match status { + ConcernStatus::Active => { + writeln!( + md, + " - [{title}]({comment_url}) by [{author}](https://github.com/{author})" + ) + } + ConcernStatus::Resolved { + comment_url: resolved_comment_url, + } => { + writeln!( + md, + " - ~~[{title}]({comment_url}) by [{author}](https://github.com/{author})~~ resolved [in this comment]({resolved_comment_url})" + ) + } + }; + } + + let _ = writeln!(md, ""); + let _ = writeln!(md, "Generated by triagebot, see [help](https://forge.rust-lang.org/triagebot/concern.html) for how to use them."); + + md +} + +#[test] +fn simple_markdown_content() { + let concerns = &[ + Concern { + title: "This is my concern about concern".to_string(), + author: "Urgau".to_string(), + status: ConcernStatus::Active, + comment_url: "https://github.com/fake-comment-1234".to_string(), + }, + Concern { + title: "This is a resolved concern".to_string(), + author: "Kobzol".to_string(), + status: ConcernStatus::Resolved { + comment_url: "https:://github.com/fake-comment-8888".to_string(), + }, + comment_url: "https://github.com/fake-comment-4561".to_string(), + }, + ]; + + assert_eq!( + markdown_content(concerns), + r#" +# Concerns + + - [This is my concern about concern](https://github.com/fake-comment-1234) by [Urgau](https://github.com/Urgau) + - ~~[This is a resolved concern](https://github.com/fake-comment-4561) by [Kobzol](https://github.com/Kobzol)~~ resolved [in this comment](https:://github.com/fake-comment-8888) + +Generated by triagebot, see [help](https://forge.rust-lang.org/triagebot/concern.html) for how to use them. +"# + ); +}