diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index de2023195ab..720d865c17b 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -32,9 +32,6 @@ pub fn main(config: &mut LazyConfig) -> CliResult { // the [alias] table). let config = config.get_mut(); - // Global args need to be extracted before expanding aliases because the - // clap code for extracting a subcommand discards global options - // (appearing before the subcommand). let (expanded_args, global_args) = expand_aliases(config, args, vec![])?; if expanded_args @@ -224,26 +221,34 @@ fn add_ssl(version_string: &mut String) { } } +/// Expands aliases recursively to collect all the command line arguments. +/// +/// [`GlobalArgs`] need to be extracted before expanding aliases because the +/// clap code for extracting a subcommand discards global options +/// (appearing before the subcommand). fn expand_aliases( config: &mut Config, args: ArgMatches, mut already_expanded: Vec, ) -> Result<(ArgMatches, GlobalArgs), CliError> { if let Some((cmd, args)) = args.subcommand() { - match ( - commands::builtin_exec(cmd), - super::aliased_command(config, cmd)?, - ) { - (Some(_), Some(_)) => { + let exec = commands::builtin_exec(cmd); + let aliased_cmd = super::aliased_command(config, cmd); + + match (exec, aliased_cmd) { + (Some(_), Ok(Some(_))) => { // User alias conflicts with a built-in subcommand config.shell().warn(format!( "user-defined alias `{}` is ignored, because it is shadowed by a built-in command", cmd, ))?; } - (Some(_), None) => { - // Command is built-in and is not conflicting with alias, but contains ignored values. + (Some(_), Ok(None) | Err(_)) => { + // Here we ignore errors from aliasing as we already favor built-in command, + // and alias doesn't involve in this context. + if let Some(values) = args.get_many::("") { + // Command is built-in and is not conflicting with alias, but contains ignored values. return Err(anyhow::format_err!( "\ trailing arguments after built-in command `{}` are unsupported: `{}` @@ -255,8 +260,8 @@ To pass the arguments to the subcommand, remove `--`", .into()); } } - (None, None) => {} - (_, Some(alias)) => { + (None, Ok(None)) => {} + (None, Ok(Some(alias))) => { // Check if this alias is shadowing an external subcommand // (binary of the form `cargo-`) // Currently this is only a warning, but after a transition period this will become @@ -300,6 +305,7 @@ For more information, see issue #10049 return Err(e.into()), } }; diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index 341b2c42d63..e6fb71dd95d 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -53,6 +53,14 @@ fn builtin_aliases_execs(cmd: &str) -> Option<&(&str, &str, &str)> { BUILTIN_ALIASES.iter().find(|alias| alias.0 == cmd) } +/// Resolve the aliased command from the [`Config`] with a given command string. +/// +/// The search fallback chain is: +/// +/// 1. Get the aliased command as a string. +/// 2. If an `Err` occurs (missing key, type mismatch, or any possible error), +/// try to get it as an array again. +/// 3. If still cannot find any, finds one insides [`BUILTIN_ALIASES`]. fn aliased_command(config: &Config, command: &str) -> CargoResult>> { let alias_name = format!("alias.{}", command); let user_alias = match config.get_string(&alias_name) { diff --git a/src/cargo/util/config/de.rs b/src/cargo/util/config/de.rs index 26b149c79e2..23d0b1b3ccd 100644 --- a/src/cargo/util/config/de.rs +++ b/src/cargo/util/config/de.rs @@ -80,7 +80,7 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> { where V: de::Visitor<'de>, { - if self.config.has_key(&self.key, self.env_prefix_ok) { + if self.config.has_key(&self.key, self.env_prefix_ok)? { visitor.visit_some(self) } else { // Treat missing values as `None`. diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index dd3f6ae933e..fddd47acb90 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -682,25 +682,25 @@ impl Config { } } - fn has_key(&self, key: &ConfigKey, env_prefix_ok: bool) -> bool { + /// Check if the [`Config`] contains a given [`ConfigKey`]. + /// + /// See `ConfigMapAccess` for a description of `env_prefix_ok`. + fn has_key(&self, key: &ConfigKey, env_prefix_ok: bool) -> CargoResult { if self.env.contains_key(key.as_env_key()) { - return true; + return Ok(true); } - // See ConfigMapAccess for a description of this. if env_prefix_ok { let env_prefix = format!("{}_", key.as_env_key()); if self.env.keys().any(|k| k.starts_with(&env_prefix)) { - return true; + return Ok(true); } } - if let Ok(o_cv) = self.get_cv(key) { - if o_cv.is_some() { - return true; - } + if self.get_cv(key)?.is_some() { + return Ok(true); } self.check_environment_key_case_mismatch(key); - false + Ok(false) } fn check_environment_key_case_mismatch(&self, key: &ConfigKey) { diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index 4e1d483e4c3..295255c566d 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -19,8 +19,8 @@ fn bad1() { .with_status(101) .with_stderr( "\ -[ERROR] invalid configuration for key `target.nonexistent-target` -expected a table, but found a string for `[..]` in [..]config +[ERROR] expected table for configuration key `target.nonexistent-target`, \ +but found string in [..]config ", ) .run(); diff --git a/tests/testsuite/cargo_alias_config.rs b/tests/testsuite/cargo_alias_config.rs index 7d09557e0ad..13cea01474e 100644 --- a/tests/testsuite/cargo_alias_config.rs +++ b/tests/testsuite/cargo_alias_config.rs @@ -21,7 +21,7 @@ fn alias_incorrect_config_type() { p.cargo("b-cargo-test -v") .with_status(101) - .with_stderr_contains( + .with_stderr( "\ [ERROR] invalid configuration for key `alias.b-cargo-test` expected a list, but found a integer for [..]", @@ -29,6 +29,80 @@ expected a list, but found a integer for [..]", .run(); } +#[cargo_test] +fn alias_malformed_config_string() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/main.rs", "fn main() {}") + .file( + ".cargo/config", + r#" + [alias] + b-cargo-test = ` + "#, + ) + .build(); + + p.cargo("b-cargo-test -v") + .with_status(101) + .with_stderr( + "\ +[ERROR] could not load Cargo configuration + +Caused by: + could not parse TOML configuration in `[..]/config` + +Caused by: + [..] + +Caused by: + TOML parse error at line [..] + | + 3 | b-cargo-test = ` + | ^ + Unexpected ``` + Expected quoted string +", + ) + .run(); +} + +#[cargo_test] +fn alias_malformed_config_list() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/main.rs", "fn main() {}") + .file( + ".cargo/config", + r#" + [alias] + b-cargo-test = [1, 2] + "#, + ) + .build(); + + p.cargo("b-cargo-test -v") + .with_status(101) + .with_stderr( + "\ +[ERROR] could not load Cargo configuration + +Caused by: + failed to load TOML configuration from `[..]/config` + +Caused by: + [..] `alias` + +Caused by: + [..] `b-cargo-test` + +Caused by: + expected string but found integer in list +", + ) + .run(); +} + #[cargo_test] fn alias_config() { let p = project() diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 601d58af6ae..b1d07bb4050 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -1074,10 +1074,6 @@ Dotted key `ssl-version` attempted to extend non-table type (string) ", ); - assert!(config - .get::>("http.ssl-version") - .unwrap() - .is_none()); } #[cargo_test]