From ef7a4ef06280939853e6c9aae158b450ae41b4b7 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 21 Sep 2022 22:15:13 +0100 Subject: [PATCH] Dont swallow errors when checking existence of a config key --- src/cargo/util/config/de.rs | 2 +- src/cargo/util/config/mod.rs | 18 +++++++-------- tests/testsuite/bad_config.rs | 4 ++-- tests/testsuite/cargo_alias_config.rs | 32 ++++++++++++++++++++++----- tests/testsuite/config.rs | 4 ---- 5 files changed, 39 insertions(+), 21 deletions(-) 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 7efa27fa31b..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 [..]", @@ -47,9 +47,21 @@ fn alias_malformed_config_string() { .with_status(101) .with_stderr( "\ -[ERROR] no such subcommand: `b-cargo-test` +[ERROR] could not load Cargo configuration + +Caused by: + could not parse TOML configuration in `[..]/config` + +Caused by: + [..] -View all installed commands with `cargo --list` +Caused by: + TOML parse error at line [..] + | + 3 | b-cargo-test = ` + | ^ + Unexpected ``` + Expected quoted string ", ) .run(); @@ -73,9 +85,19 @@ fn alias_malformed_config_list() { .with_status(101) .with_stderr( "\ -[ERROR] no such subcommand: `b-cargo-test` +[ERROR] could not load Cargo configuration + +Caused by: + failed to load TOML configuration from `[..]/config` + +Caused by: + [..] `alias` + +Caused by: + [..] `b-cargo-test` -View all installed commands with `cargo --list` +Caused by: + expected string but found integer in list ", ) .run(); 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]