Skip to content

Commit

Permalink
Error if any invalid keys are defined when inheriting a workspace dep…
Browse files Browse the repository at this point in the history
…endency
  • Loading branch information
Muscraft committed Apr 27, 2022
1 parent 7914967 commit 74d7f69
Show file tree
Hide file tree
Showing 35 changed files with 199 additions and 0 deletions.
38 changes: 38 additions & 0 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,10 @@ fn resolve_dependency(
}
}

if let Some(Source::Workspace(_)) = dependency.source() {
check_invalid_ws_keys(dependency.toml_key(), arg)?;
}

let version_required = dependency.source().and_then(|s| s.as_registry()).is_some();
let version_optional_in_section = section.kind() == DepKind::Development;
let preserve_existing_version = old_dep
Expand All @@ -328,6 +332,40 @@ fn resolve_dependency(
Ok(dependency)
}

/// When { workspace = true } you cannot define other keys that configure
/// the source of the dependency such as `version`, `registry`, `registry-index`,
/// `path`, `git`, `branch`, `tag`, `rev`, or `package`. You can also not define
/// `default-features`.
///
/// Only `default-features`, `registry` and `rename` need to be checked
/// for currently. This is because `git` and its associated keys, `path`, and
/// `version` should all bee checked before this is called. `rename` is checked
/// for as it turns into `package`
fn check_invalid_ws_keys(toml_key: &str, arg: &DepOp) -> CargoResult<()> {
fn err_msg(toml_key: &str, flag: &str, field: &str) -> String {
format!(
"cannot override workspace dependency with `{flag}`, \
either change `workspace.dependencies.{toml_key}.{field}` \
or define the dependency exclusively in the package's manifest"
)
}

if arg.default_features.is_some() {
anyhow::bail!(
"{}",
err_msg(toml_key, "--default-features", "default-features")
)
}
if arg.registry.is_some() {
anyhow::bail!("{}", err_msg(toml_key, "--registry", "registry"))
}
// rename is `package`
if arg.rename.is_some() {
anyhow::bail!("{}", err_msg(toml_key, "--rename", "package"))
}
Ok(())
}

/// Provide the existing dependency for the target table
///
/// If it doesn't exist but exists in another table, let's use that as most likely users
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary", "dependency"]

[workspace.dependencies]
foo = { version = "0.0.0", path = "./dependency"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "foo"
version = "0.0.0"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
cargo-features = ["workspace-inheritance"]

[package]
name = "bar"
version = "0.0.0"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary", "dependency"]

[workspace.dependencies]
foo = { version = "0.0.0", path = "./dependency"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "foo"
version = "0.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
cargo-features = ["workspace-inheritance"]

[package]
name = "bar"
version = "0.0.0"
1 change: 1 addition & 0 deletions tests/snapshots/add/invalid_key_inherit_dependency.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error: cannot override workspace dependency with `--default-features`, either change `workspace.dependencies.foo.default-features` or define the dependency exclusively in the package's manifest
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary", "dependency"]

[workspace.dependencies]
foo = { version = "0.0.0", path = "./dependency"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "foo"
version = "0.0.0"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
cargo-features = ["workspace-inheritance"]

[package]
name = "bar"
version = "0.0.0"

[dependencies]
foo.workspace = true
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary", "dependency"]

[workspace.dependencies]
foo = { version = "0.0.0", path = "./dependency"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "foo"
version = "0.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
cargo-features = ["workspace-inheritance"]

[package]
name = "bar"
version = "0.0.0"

[dependencies]
foo.workspace = true
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error: cannot override workspace dependency with `--default-features`, either change `workspace.dependencies.foo.default-features` or define the dependency exclusively in the package's manifest
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary", "dependency", "dependency-alt"]

[workspace.dependencies]
foo = { version = "0.0.0", path = "./dependency"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "foo-alt"
version = "0.0.0"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "foo"
version = "0.0.0"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
cargo-features = ["workspace-inheritance"]

[package]
name = "bar"
version = "0.0.0"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary", "dependency", "dependency-alt"]

[workspace.dependencies]
foo = { version = "0.0.0", path = "./dependency"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "foo-alt"
version = "0.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "foo"
version = "0.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
cargo-features = ["workspace-inheritance"]

[package]
name = "bar"
version = "0.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error: cannot override workspace dependency with `--rename`, either change `workspace.dependencies.foo.package` or define the dependency exclusively in the package's manifest
Empty file.
68 changes: 68 additions & 0 deletions tests/testsuite/cargo_add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,74 @@ fn invalid_git_external() {
);
}

#[cargo_test]
fn invalid_key_inherit_dependency() {
let project = Project::from_template("tests/snapshots/add/invalid_key_inherit_dependency.in");
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo()
.masquerade_as_nightly_cargo()
.arg("add")
.args(["foo", "--default-features", "-p", "bar"])
.current_dir(cwd)
.assert()
.failure()
.stdout_matches_path("tests/snapshots/add/invalid_key_inherit_dependency.stdout")
.stderr_matches_path("tests/snapshots/add/invalid_key_inherit_dependency.stderr");

assert().subset_matches(
"tests/snapshots/add/invalid_key_inherit_dependency.out",
&project_root,
);
}

#[cargo_test]
fn invalid_key_rename_inherit_dependency() {
let project =
Project::from_template("tests/snapshots/add/invalid_key_rename_inherit_dependency.in");
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo()
.masquerade_as_nightly_cargo()
.arg("add")
.args(["--rename", "foo", "foo-alt", "-p", "bar"])
.current_dir(cwd)
.assert()
.failure()
.stdout_matches_path("tests/snapshots/add/invalid_key_rename_inherit_dependency.stdout")
.stderr_matches_path("tests/snapshots/add/invalid_key_rename_inherit_dependency.stderr");

assert().subset_matches(
"tests/snapshots/add/invalid_key_rename_inherit_dependency.out",
&project_root,
);
}

#[cargo_test]
fn invalid_key_overwrite_inherit_dependency() {
let project =
Project::from_template("tests/snapshots/add/invalid_key_overwrite_inherit_dependency.in");
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo()
.masquerade_as_nightly_cargo()
.arg("add")
.args(["foo", "--default-features", "-p", "bar"])
.current_dir(cwd)
.assert()
.failure()
.stdout_matches_path("tests/snapshots/add/invalid_key_overwrite_inherit_dependency.stdout")
.stderr_matches_path("tests/snapshots/add/invalid_key_overwrite_inherit_dependency.stderr");

assert().subset_matches(
"tests/snapshots/add/invalid_key_overwrite_inherit_dependency.out",
&project_root,
);
}

#[cargo_test]
fn invalid_path() {
init_registry();
Expand Down

0 comments on commit 74d7f69

Please sign in to comment.