From fe49f1a3ea85991c218deb71c059014135f2b415 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 23 Dec 2025 18:22:54 -0500 Subject: [PATCH 1/5] refactor(help): early return rather than nested if-else --- src/bin/cargo/commands/help.rs | 45 ++++++++++++++++------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/src/bin/cargo/commands/help.rs b/src/bin/cargo/commands/help.rs index d838a6277..dcc263ef2 100644 --- a/src/bin/cargo/commands/help.rs +++ b/src/bin/cargo/commands/help.rs @@ -31,28 +31,26 @@ pub fn cli() -> Command { } pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { - let subcommand = args.get_one::("COMMAND"); - if let Some(subcommand) = subcommand { - if !try_help(gctx, subcommand)? { - match check_builtin(&subcommand) { - Some(s) => { - crate::execute_internal_subcommand( - gctx, - &[OsStr::new(s), OsStr::new("--help")], - )?; - } - None => { - crate::execute_external_subcommand( - gctx, - subcommand, - &[OsStr::new(subcommand), OsStr::new("--help")], - )?; - } - } + let Some(subcommand) = args.get_one::("COMMAND") else { + let _ = crate::cli::cli(gctx).print_help(); + return Ok(()); + }; + + if try_help(gctx, subcommand)? { + return Ok(()); + } + + match check_builtin(&subcommand) { + Some(s) => { + crate::execute_internal_subcommand(gctx, &[OsStr::new(s), OsStr::new("--help")])?; + } + None => { + crate::execute_external_subcommand( + gctx, + subcommand, + &[OsStr::new(subcommand), OsStr::new("--help")], + )?; } - } else { - let mut cmd = crate::cli::cli(gctx); - let _ = cmd.print_help(); } Ok(()) } @@ -89,9 +87,8 @@ fn try_help(gctx: &GlobalContext, subcommand: &str) -> CargoResult { }; write_and_spawn(subcommand, &man, "man")?; } else { - let txt = match extract_man(subcommand, "txt") { - Some(txt) => txt, - None => return Ok(false), + let Some(txt) = extract_man(subcommand, "txt") else { + return Ok(false); }; if force_help_text { drop(std::io::stdout().write_all(&txt)); From 112ea913a559ba4bdc280d255061dd67ba9ad7fd Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 27 Dec 2025 14:46:09 -0500 Subject: [PATCH 2/5] refactor(help): inline check alias/builtin functions --- src/bin/cargo/commands/help.rs | 42 ++++++++++------------------------ 1 file changed, 12 insertions(+), 30 deletions(-) diff --git a/src/bin/cargo/commands/help.rs b/src/bin/cargo/commands/help.rs index dcc263ef2..52ac5c567 100644 --- a/src/bin/cargo/commands/help.rs +++ b/src/bin/cargo/commands/help.rs @@ -40,23 +40,20 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { return Ok(()); } - match check_builtin(&subcommand) { - Some(s) => { - crate::execute_internal_subcommand(gctx, &[OsStr::new(s), OsStr::new("--help")])?; - } - None => { - crate::execute_external_subcommand( - gctx, - subcommand, - &[OsStr::new(subcommand), OsStr::new("--help")], - )?; - } + if super::builtin_exec(subcommand).is_some() { + crate::execute_internal_subcommand(gctx, &[OsStr::new(subcommand), OsStr::new("--help")])?; + } else { + crate::execute_external_subcommand( + gctx, + subcommand, + &[OsStr::new(subcommand), OsStr::new("--help")], + )?; } Ok(()) } fn try_help(gctx: &GlobalContext, subcommand: &str) -> CargoResult { - let subcommand = match check_alias(gctx, subcommand) { + let subcommand = &match aliased_command(gctx, subcommand).ok().flatten() { // If this alias is more than a simple subcommand pass-through, show the alias. Some(argv) if argv.len() > 1 => { let alias = argv.join(" "); @@ -72,10 +69,9 @@ fn try_help(gctx: &GlobalContext, subcommand: &str) -> CargoResult { None => subcommand.to_string(), }; - let subcommand = match check_builtin(&subcommand) { - Some(s) => s, - None => return Ok(false), - }; + if super::builtin_exec(subcommand).is_none() { + return Ok(false); + } // ALLOWED: For testing cargo itself only. #[allow(clippy::disallowed_methods)] @@ -103,20 +99,6 @@ fn try_help(gctx: &GlobalContext, subcommand: &str) -> CargoResult { Ok(true) } -/// Checks if the given subcommand is an alias. -/// -/// Returns None if it is not an alias. -fn check_alias(gctx: &GlobalContext, subcommand: &str) -> Option> { - aliased_command(gctx, subcommand).ok().flatten() -} - -/// Checks if the given subcommand is a built-in command (not via an alias). -/// -/// Returns None if it is not a built-in command. -fn check_builtin(subcommand: &str) -> Option<&str> { - super::builtin_exec(subcommand).map(|_| subcommand) -} - /// Extracts the given man page from the compressed archive. /// /// Returns None if the command wasn't found. From db87b6ce45176d2db61bd9af3f071c15d31c4dd4 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 27 Dec 2025 14:49:52 -0500 Subject: [PATCH 3/5] refactor(help): move alias expansion out from `try_help` --- src/bin/cargo/commands/help.rs | 39 +++++++++++++++++----------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/bin/cargo/commands/help.rs b/src/bin/cargo/commands/help.rs index 52ac5c567..cca9e0eda 100644 --- a/src/bin/cargo/commands/help.rs +++ b/src/bin/cargo/commands/help.rs @@ -36,29 +36,13 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { return Ok(()); }; - if try_help(gctx, subcommand)? { - return Ok(()); - } - - if super::builtin_exec(subcommand).is_some() { - crate::execute_internal_subcommand(gctx, &[OsStr::new(subcommand), OsStr::new("--help")])?; - } else { - crate::execute_external_subcommand( - gctx, - subcommand, - &[OsStr::new(subcommand), OsStr::new("--help")], - )?; - } - Ok(()) -} - -fn try_help(gctx: &GlobalContext, subcommand: &str) -> CargoResult { - let subcommand = &match aliased_command(gctx, subcommand).ok().flatten() { + // Expand alias first + let subcommand = match aliased_command(gctx, subcommand).ok().flatten() { // If this alias is more than a simple subcommand pass-through, show the alias. Some(argv) if argv.len() > 1 => { let alias = argv.join(" "); drop_println!(gctx, "`{}` is aliased to `{}`", subcommand, alias); - return Ok(true); + return Ok(()); } // Otherwise, resolve the alias into its subcommand. Some(argv) => { @@ -69,6 +53,23 @@ fn try_help(gctx: &GlobalContext, subcommand: &str) -> CargoResult { None => subcommand.to_string(), }; + if try_help(&subcommand)? { + return Ok(()); + } + + if super::builtin_exec(&subcommand).is_some() { + crate::execute_internal_subcommand(gctx, &[OsStr::new(&subcommand), OsStr::new("--help")])?; + } else { + crate::execute_external_subcommand( + gctx, + &subcommand, + &[OsStr::new(&subcommand), OsStr::new("--help")], + )?; + } + Ok(()) +} + +fn try_help(subcommand: &str) -> CargoResult { if super::builtin_exec(subcommand).is_none() { return Ok(false); } From 5288e8204b3a6db41363ff47c1d3b8189972f768 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 27 Dec 2025 14:53:28 -0500 Subject: [PATCH 4/5] refactor(help): avoid checking built-in commands twice --- src/bin/cargo/commands/help.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/bin/cargo/commands/help.rs b/src/bin/cargo/commands/help.rs index cca9e0eda..6c1f03d69 100644 --- a/src/bin/cargo/commands/help.rs +++ b/src/bin/cargo/commands/help.rs @@ -53,27 +53,24 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { None => subcommand.to_string(), }; - if try_help(&subcommand)? { - return Ok(()); - } - if super::builtin_exec(&subcommand).is_some() { + if try_help(&subcommand)? { + return Ok(()); + } crate::execute_internal_subcommand(gctx, &[OsStr::new(&subcommand), OsStr::new("--help")])?; } else { + // If not built-in, try giving `--help` to external command. crate::execute_external_subcommand( gctx, &subcommand, &[OsStr::new(&subcommand), OsStr::new("--help")], )?; } + Ok(()) } fn try_help(subcommand: &str) -> CargoResult { - if super::builtin_exec(subcommand).is_none() { - return Ok(false); - } - // ALLOWED: For testing cargo itself only. #[allow(clippy::disallowed_methods)] let force_help_text = std::env::var("__CARGO_TEST_FORCE_HELP_TXT").is_ok(); From 4ac2278be211ce158a02f56a4841635d2e8e5906 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 12 Feb 2026 10:24:57 +0800 Subject: [PATCH 5/5] refactor(help): use `#[expect]` rather than `#[allow]` --- src/bin/cargo/commands/help.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bin/cargo/commands/help.rs b/src/bin/cargo/commands/help.rs index 6c1f03d69..868b14822 100644 --- a/src/bin/cargo/commands/help.rs +++ b/src/bin/cargo/commands/help.rs @@ -71,8 +71,10 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { } fn try_help(subcommand: &str) -> CargoResult { - // ALLOWED: For testing cargo itself only. - #[allow(clippy::disallowed_methods)] + #[expect( + clippy::disallowed_methods, + reason = "testing only, no reason for config support" + )] let force_help_text = std::env::var("__CARGO_TEST_FORCE_HELP_TXT").is_ok(); if resolve_executable(Path::new("man")).is_ok() && !force_help_text {