diff --git a/src/config.rs b/src/config.rs index 39c665b6..244c35ef 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,7 +6,7 @@ use std::sync::{Arc, RwLock}; use std::time::{Duration, Instant}; use tracing as log; -static CONFIG_FILE_NAME: &str = "triagebot.toml"; +pub(crate) static CONFIG_FILE_NAME: &str = "triagebot.toml"; const REFRESH_EVERY: Duration = Duration::from_secs(2 * 60); // Every two minutes lazy_static::lazy_static! { @@ -36,6 +36,9 @@ pub(crate) struct Config { pub(crate) note: Option, pub(crate) mentions: Option, pub(crate) no_merges: Option, + // We want this validation to run even without the entry in the config file + #[serde(default = "ValidateConfig::default")] + pub(crate) validate_config: Option, } #[derive(PartialEq, Eq, Debug, serde::Deserialize)] @@ -167,6 +170,15 @@ pub(crate) struct PrioritizeConfig { pub(crate) label: String, } +#[derive(PartialEq, Eq, Debug, serde::Deserialize)] +pub(crate) struct ValidateConfig {} + +impl ValidateConfig { + fn default() -> Option { + Some(ValidateConfig {}) + } +} + #[derive(PartialEq, Eq, Debug, serde::Deserialize)] pub(crate) struct AutolabelConfig { #[serde(flatten)] @@ -450,6 +462,7 @@ mod tests { review_requested: None, mentions: None, no_merges: None, + validate_config: Some(ValidateConfig {}), } ); } diff --git a/src/github.rs b/src/github.rs index bbd824dd..040da27d 100644 --- a/src/github.rs +++ b/src/github.rs @@ -999,7 +999,7 @@ struct PullRequestEventFields {} #[derive(Clone, Debug, serde::Deserialize)] pub struct CommitBase { - sha: String, + pub sha: String, #[serde(rename = "ref")] pub git_ref: String, pub repo: Repository, diff --git a/src/handlers.rs b/src/handlers.rs index d2bf47d8..4838760e 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -46,6 +46,7 @@ mod rfc_helper; pub mod rustc_commits; mod shortcut; pub mod types_planning_updates; +mod validate_config; pub async fn handle(ctx: &Context, event: &Event) -> Vec { let config = config::get(&ctx.github, event.repo()).await; @@ -166,6 +167,7 @@ issue_handlers! { no_merges, notify_zulip, review_requested, + validate_config, } macro_rules! command_handlers { diff --git a/src/handlers/validate_config.rs b/src/handlers/validate_config.rs new file mode 100644 index 00000000..04814918 --- /dev/null +++ b/src/handlers/validate_config.rs @@ -0,0 +1,117 @@ +//! For pull requests that have changed the triagebot.toml, validate that the +//! changes are a valid configuration file. +//! It won't validate anything unless the PR is open and has changed. + +use crate::{ + config::{ValidateConfig, CONFIG_FILE_NAME}, + handlers::{Context, IssuesEvent}, +}; +use tracing as log; + +pub(super) async fn parse_input( + ctx: &Context, + event: &IssuesEvent, + _config: Option<&ValidateConfig>, +) -> Result, String> { + // All processing needs to be done in parse_input (instead of + // handle_input) because we want this to *always* run. handle_input + // requires the config to exist in triagebot.toml, but we want this to run + // even if it isn't configured. As a consequence, error handling needs to + // be a little more cautious here, since we don't want to relay + // un-actionable errors to the user. + let diff = match event.issue.diff(&ctx.github).await { + Ok(Some(diff)) => diff, + Ok(None) => return Ok(None), + Err(e) => { + log::error!("failed to get diff {e}"); + return Ok(None); + } + }; + if !diff.iter().any(|diff| diff.path == CONFIG_FILE_NAME) { + return Ok(None); + } + + let Some(pr_source) = &event.issue.head else { + log::error!("expected head commit in {event:?}"); + return Ok(None); + }; + let triagebot_content = match ctx + .github + .raw_file(&pr_source.repo.full_name, &pr_source.sha, CONFIG_FILE_NAME) + .await + { + Ok(Some(c)) => c, + Ok(None) => { + log::error!("{CONFIG_FILE_NAME} modified, but failed to get content"); + return Ok(None); + } + Err(e) => { + log::error!("failed to get {CONFIG_FILE_NAME}: {e}"); + return Ok(None); + } + }; + + let triagebot_content = String::from_utf8_lossy(&*triagebot_content); + if let Err(e) = toml::from_str::(&triagebot_content) { + let position = match e.span() { + // toml sometimes gives bad spans, see https://github.com/toml-rs/toml/issues/589 + Some(span) if span != (0..0) => { + let (line, col) = translate_position(&triagebot_content, span.start); + let url = format!( + "https://github.com/{}/blob/{}/{CONFIG_FILE_NAME}#L{line}", + pr_source.repo.full_name, pr_source.sha + ); + format!(" at position [{line}:{col}]({url})",) + } + Some(_) | None => String::new(), + }; + + return Err(format!( + "Invalid `triagebot.toml`{position}:\n\ + `````\n\ + {e}\n\ + `````", + )); + } + Ok(None) +} + +pub(super) async fn handle_input( + _ctx: &Context, + _config: &ValidateConfig, + _event: &IssuesEvent, + _input: (), +) -> anyhow::Result<()> { + Ok(()) +} + +/// Helper to translate a toml span to a `(line_no, col_no)` (1-based). +fn translate_position(input: &str, index: usize) -> (usize, usize) { + if input.is_empty() { + return (0, index); + } + + let safe_index = index.min(input.len() - 1); + let column_offset = index - safe_index; + + let nl = input[0..safe_index] + .as_bytes() + .iter() + .rev() + .enumerate() + .find(|(_, b)| **b == b'\n') + .map(|(nl, _)| safe_index - nl - 1); + let line_start = match nl { + Some(nl) => nl + 1, + None => 0, + }; + let line = input[0..line_start] + .as_bytes() + .iter() + .filter(|c| **c == b'\n') + .count(); + let column = input[line_start..=safe_index].chars().count() - 1; + let column = column + column_offset; + + (line + 1, column + 1) +}