diff --git a/src/bors/command/parser.rs b/src/bors/command/parser.rs index 43920c8b..dd208bc4 100644 --- a/src/bors/command/parser.rs +++ b/src/bors/command/parser.rs @@ -14,7 +14,7 @@ pub enum CommandParseError { MissingCommand, UnknownCommand(String), MissingArgValue { arg: String }, - UnknownArg(String), + UnknownArg { arg: String, did_you_mean: String }, DuplicateArg(String), ValidationError(String), } @@ -270,7 +270,10 @@ fn parser_try(command: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> ParseResu for part in parts { match part { CommandPart::Bare(key) => { - return Some(Err(CommandParseError::UnknownArg(key.to_string()))); + return Some(Err(CommandParseError::UnknownArg { + arg: key.to_string(), + did_you_mean: "jobs=|parent=".to_string(), + })); } CommandPart::KeyValue { key, value } => match (*key, *value) { ("parent", "last") => parent = Some(Parent::Last), @@ -299,7 +302,10 @@ fn parser_try(command: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> ParseResu jobs = raw_jobs; } _ => { - return Some(Err(CommandParseError::UnknownArg(key.to_string()))); + return Some(Err(CommandParseError::UnknownArg { + arg: key.to_string(), + did_you_mean: "jobs=|parent=".to_string(), + })); } }, } @@ -318,11 +324,24 @@ fn parser_try_cancel(command: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> Pa } /// Parses `@bors delegate=` or `@bors delegate+`. -fn parser_delegate(command: &CommandPart<'_>, _parts: &[CommandPart<'_>]) -> ParseResult { +fn parser_delegate(command: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> ParseResult { match command { CommandPart::Bare("delegate+") => { Some(Ok(BorsCommand::SetDelegate(DelegatedPermission::Review))) } + CommandPart::Bare("delegate") => { + if !parts.is_empty() { + Some(Err(CommandParseError::UnknownArg { + arg: match parts[0] { + CommandPart::Bare(arg) => arg.to_owned(), + CommandPart::KeyValue { key, .. } => key.to_owned(), + }, + did_you_mean: "delegate=".to_string(), + })) + } else { + Some(Ok(BorsCommand::SetDelegate(DelegatedPermission::Review))) + } + } CommandPart::KeyValue { key: "delegate", value, @@ -1163,15 +1182,31 @@ for the crater", #[test] fn parse_try_unknown_arg() { let cmds = parse_commands("@bors try a"); - assert_eq!(cmds.len(), 1); - assert_eq!(cmds[0], Err(CommandParseError::UnknownArg("a".to_string()))); + insta::assert_debug_snapshot!(cmds, @r#" + [ + Err( + UnknownArg { + arg: "a", + did_you_mean: "jobs=|parent=", + }, + ), + ] + "#); } #[test] fn parse_try_unknown_kv_arg() { let cmds = parse_commands("@bors try a=b"); - assert_eq!(cmds.len(), 1); - assert_eq!(cmds[0], Err(CommandParseError::UnknownArg("a".to_string()))); + insta::assert_debug_snapshot!(cmds, @r#" + [ + Err( + UnknownArg { + arg: "a", + did_you_mean: "jobs=|parent=", + }, + ), + ] + "#); } #[test] @@ -1228,7 +1263,7 @@ for the crater", } #[test] - fn parse_delegate_author() { + fn parse_delegate_plus_author() { let cmds = parse_commands("@bors delegate+"); assert_eq!(cmds.len(), 1); assert!(matches!( @@ -1237,6 +1272,31 @@ for the crater", )); } + #[test] + fn parse_delegate_author() { + let cmds = parse_commands("@bors delegate"); + assert_eq!(cmds.len(), 1); + assert!(matches!( + cmds[0], + Ok(BorsCommand::SetDelegate(DelegatedPermission::Review)) + )); + } + + #[test] + fn parse_delegate_unknown_arg() { + let cmds = parse_commands("@bors delegate try"); + insta::assert_debug_snapshot!(cmds, @r#" + [ + Err( + UnknownArg { + arg: "try", + did_you_mean: "delegate=", + }, + ), + ] + "#); + } + #[test] fn parse_delegate_review_author() { let cmds = parse_commands("@bors delegate=review"); diff --git a/src/bors/handlers/help.rs b/src/bors/handlers/help.rs index 499dba46..b765a419 100644 --- a/src/bors/handlers/help.rs +++ b/src/bors/handlers/help.rs @@ -43,7 +43,7 @@ mod tests { - `delegate=`: Delegate permissions for running try builds or approving to the PR author - `try` allows the PR author to start try builds. - `review` allows the PR author to both start try builds and approve the PR. - - `delegate+`: Delegate approval permissions to the PR author + - `delegate` | `delegate+`: Delegate approval permissions to the PR author - Shortcut for `delegate=review` - `delegate-`: Remove any previously granted permission delegation - `try [parent=] [job|jobs=]`: Start a try build. diff --git a/src/bors/handlers/mod.rs b/src/bors/handlers/mod.rs index 5ead3428..7b91ede6 100644 --- a/src/bors/handlers/mod.rs +++ b/src/bors/handlers/mod.rs @@ -549,8 +549,11 @@ async fn handle_comment( CommandParseError::MissingArgValue { arg } => { format!(r#"Unknown value for argument "{arg}"."#) } - CommandParseError::UnknownArg(arg) => { - format!(r#"Unknown argument "{arg}"."#) + CommandParseError::UnknownArg { arg, did_you_mean } => { + format!( + r#"Unknown argument "{arg}". Did you mean to use `{} {did_you_mean}`?"#, + ctx.parser.prefix() + ) } CommandParseError::DuplicateArg(arg) => { format!(r#"Argument "{arg}" found multiple times."#) diff --git a/src/bors/handlers/review.rs b/src/bors/handlers/review.rs index 4bcc7993..78b198aa 100644 --- a/src/bors/handlers/review.rs +++ b/src/bors/handlers/review.rs @@ -882,6 +882,19 @@ approved = { modifications = ["+foo", "+baz"], unless = ["label1", "label2"] } .await; } + #[sqlx::test] + async fn delegate_error_message(pool: sqlx::PgPool) { + BorsBuilder::new(pool) + .github(GitHub::unauthorized_pr_author()) + .run_test(async |ctx: &mut BorsTester| { + ctx.post_comment(review_comment("@bors delegate try")) + .await?; + insta::assert_snapshot!(ctx.get_next_comment_text(()).await?, @r#"Unknown argument "try". Did you mean to use `@bors delegate=`? Run `@bors help` to see available commands."#); + Ok(()) + }) + .await; + } + #[sqlx::test] async fn delegatee_can_try(pool: sqlx::PgPool) { let gh = BorsBuilder::new(pool) diff --git a/src/bors/mod.rs b/src/bors/mod.rs index 319846b8..e462c3af 100644 --- a/src/bors/mod.rs +++ b/src/bors/mod.rs @@ -97,7 +97,7 @@ You can use the following commands: - `delegate=`: Delegate permissions for running try builds or approving to the PR author - `try` allows the PR author to start try builds. - `review` allows the PR author to both start try builds and approve the PR. -- `delegate+`: Delegate approval permissions to the PR author +- `delegate` | `delegate+`: Delegate approval permissions to the PR author - Shortcut for `delegate=review` - `delegate-`: Remove any previously granted permission delegation - `try [parent=] [job|jobs=]`: Start a try build.