Skip to content

Commit

Permalink
Dont swallow errors when checking existence of a config key
Browse files Browse the repository at this point in the history
  • Loading branch information
weihanglo committed Sep 22, 2022
1 parent 2c6647d commit ef7a4ef
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/cargo/util/config/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
18 changes: 9 additions & 9 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
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) {
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
32 changes: 27 additions & 5 deletions tests/testsuite/cargo_alias_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 [..]",
Expand All @@ -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:
[..]
<tab>View all installed commands with `cargo --list`
Caused by:
TOML parse error at line [..]
|
3 | b-cargo-test = `
| ^
Unexpected ```
Expected quoted string
",
)
.run();
Expand All @@ -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`
<tab>View all installed commands with `cargo --list`
Caused by:
expected string but found integer in list
",
)
.run();
Expand Down
4 changes: 0 additions & 4 deletions tests/testsuite/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1074,10 +1074,6 @@ Dotted key `ssl-version` attempted to extend non-table type (string)
",
);
assert!(config
.get::<Option<SslVersionConfig>>("http.ssl-version")
.unwrap()
.is_none());
}

#[cargo_test]
Expand Down

0 comments on commit ef7a4ef

Please sign in to comment.