diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index ed8506d0915..251d4c539a1 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -516,7 +516,11 @@ fn resolve_dependency( let query = dep.query(gctx)?; match query { MaybeWorkspace::Workspace(_) => { - unreachable!("This should have been caught when parsing a workspace root") + anyhow::bail!( + "dependency ({}) specified without \ + providing a local path, Git repository, or version", + dependency.toml_key() + ); } MaybeWorkspace::Other(query) => query, } diff --git a/src/cargo/util/toml_mut/dependency.rs b/src/cargo/util/toml_mut/dependency.rs index c28e96f2c68..ca63947097e 100644 --- a/src/cargo/util/toml_mut/dependency.rs +++ b/src/cargo/util/toml_mut/dependency.rs @@ -245,91 +245,87 @@ impl Dependency { (key.to_owned(), None) }; - let source: Source = if let Some(git) = table.get("git") { - let mut src = GitSource::new( - git.as_str() - .ok_or_else(|| invalid_type(key, "git", git.type_name(), "string"))?, - ); - if let Some(value) = table.get("branch") { - src = - src.set_branch(value.as_str().ok_or_else(|| { + let source: Source = + if let Some(git) = table.get("git") { + let mut src = GitSource::new( + git.as_str() + .ok_or_else(|| invalid_type(key, "git", git.type_name(), "string"))?, + ); + if let Some(value) = table.get("branch") { + src = src.set_branch(value.as_str().ok_or_else(|| { invalid_type(key, "branch", value.type_name(), "string") })?); - } - if let Some(value) = table.get("tag") { - src = - src.set_tag(value.as_str().ok_or_else(|| { + } + if let Some(value) = table.get("tag") { + src = src.set_tag(value.as_str().ok_or_else(|| { invalid_type(key, "tag", value.type_name(), "string") })?); - } - if let Some(value) = table.get("rev") { - src = - src.set_rev(value.as_str().ok_or_else(|| { + } + if let Some(value) = table.get("rev") { + src = src.set_rev(value.as_str().ok_or_else(|| { invalid_type(key, "rev", value.type_name(), "string") })?); - } - if let Some(value) = table.get("version") { - src = src.set_version(value.as_str().ok_or_else(|| { - invalid_type(key, "version", value.type_name(), "string") - })?); - } - src.into() - } else if let Some(path) = table.get("path") { - let base = table - .get("base") - .map(|base| { - base.as_str() - .ok_or_else(|| invalid_type(key, "base", base.type_name(), "string")) - .map(|s| s.to_owned()) - }) - .transpose()?; - let relative_to = if let Some(base) = &base { - Cow::Owned(lookup_path_base( - &PathBaseName::new(base.clone())?, - gctx, - &|| Ok(workspace_root), - unstable_features, - )?) - } else { - Cow::Borrowed(crate_root) - }; - let path = - relative_to + } + if let Some(value) = table.get("version") { + src = src.set_version(value.as_str().ok_or_else(|| { + invalid_type(key, "version", value.type_name(), "string") + })?); + } + src.into() + } else if let Some(path) = table.get("path") { + let base = table + .get("base") + .map(|base| { + base.as_str() + .ok_or_else(|| { + invalid_type(key, "base", base.type_name(), "string") + }) + .map(|s| s.to_owned()) + }) + .transpose()?; + let relative_to = if let Some(base) = &base { + Cow::Owned(lookup_path_base( + &PathBaseName::new(base.clone())?, + gctx, + &|| Ok(workspace_root), + unstable_features, + )?) + } else { + Cow::Borrowed(crate_root) + }; + let path = relative_to .join(path.as_str().ok_or_else(|| { invalid_type(key, "path", path.type_name(), "string") })?); - let mut src = PathSource::new(path); - src.base = base; - if let Some(value) = table.get("version") { - src = src.set_version(value.as_str().ok_or_else(|| { - invalid_type(key, "version", value.type_name(), "string") - })?); - } - src.into() - } else if let Some(version) = table.get("version") { - let src = - RegistrySource::new(version.as_str().ok_or_else(|| { + let mut src = PathSource::new(path); + src.base = base; + if let Some(value) = table.get("version") { + src = src.set_version(value.as_str().ok_or_else(|| { + invalid_type(key, "version", value.type_name(), "string") + })?); + } + src.into() + } else if let Some(version) = table.get("version") { + let src = RegistrySource::new(version.as_str().ok_or_else(|| { invalid_type(key, "version", version.type_name(), "string") })?); - src.into() - } else if let Some(workspace) = table.get("workspace") { - let workspace_bool = workspace - .as_bool() - .ok_or_else(|| invalid_type(key, "workspace", workspace.type_name(), "bool"))?; - if !workspace_bool { - anyhow::bail!("`{key}.workspace = false` is unsupported") - } - let src = WorkspaceSource::new(); - src.into() - } else { - let mut msg = format!("unrecognized dependency source for `{key}`"); - if table.is_empty() { - msg.push_str( - ", expected a local path, Git repository, version, or workspace dependency to be specified", + src.into() + } else if let Some(workspace) = table.get("workspace") { + let workspace_bool = workspace.as_bool().ok_or_else(|| { + invalid_type(key, "workspace", workspace.type_name(), "bool") + })?; + if !workspace_bool { + anyhow::bail!("`{key}.workspace = false` is unsupported") + } + let src = WorkspaceSource::new(); + src.into() + } else { + anyhow::bail!( + "dependency ({key}) specified without \ + providing a local path, Git repository, version, or \ + workspace dependency to use" ); - } - anyhow::bail!(msg); - }; + }; let registry = if let Some(value) = table.get("registry") { Some( value diff --git a/tests/testsuite/cargo_add/invalid_inherited_dependency/in/Cargo.toml b/tests/testsuite/cargo_add/invalid_inherited_dependency/in/Cargo.toml new file mode 100644 index 00000000000..f8aaaaba98a --- /dev/null +++ b/tests/testsuite/cargo_add/invalid_inherited_dependency/in/Cargo.toml @@ -0,0 +1,5 @@ +[workspace] +members = ["primary", "dependency"] + +[workspace.dependencies] +foo.workspace = true diff --git a/tests/testsuite/cargo_add/invalid_inherited_dependency/in/dependency/Cargo.toml b/tests/testsuite/cargo_add/invalid_inherited_dependency/in/dependency/Cargo.toml new file mode 100644 index 00000000000..0de6f9b15a8 --- /dev/null +++ b/tests/testsuite/cargo_add/invalid_inherited_dependency/in/dependency/Cargo.toml @@ -0,0 +1,4 @@ +[package] +name = "foo" +version = "0.0.0" +edition = "2015" diff --git a/tests/testsuite/cargo_add/invalid_inherited_dependency/in/dependency/src/lib.rs b/tests/testsuite/cargo_add/invalid_inherited_dependency/in/dependency/src/lib.rs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/testsuite/cargo_add/invalid_inherited_dependency/in/primary/Cargo.toml b/tests/testsuite/cargo_add/invalid_inherited_dependency/in/primary/Cargo.toml new file mode 100644 index 00000000000..fb520246281 --- /dev/null +++ b/tests/testsuite/cargo_add/invalid_inherited_dependency/in/primary/Cargo.toml @@ -0,0 +1,4 @@ +[package] +name = "bar" +version = "0.0.0" +edition = "2015" diff --git a/tests/testsuite/cargo_add/invalid_inherited_dependency/in/primary/src/lib.rs b/tests/testsuite/cargo_add/invalid_inherited_dependency/in/primary/src/lib.rs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/testsuite/cargo_add/invalid_inherited_dependency/mod.rs b/tests/testsuite/cargo_add/invalid_inherited_dependency/mod.rs new file mode 100644 index 00000000000..5aab4e21f9d --- /dev/null +++ b/tests/testsuite/cargo_add/invalid_inherited_dependency/mod.rs @@ -0,0 +1,25 @@ +use crate::prelude::*; +use cargo_test_support::Project; +use cargo_test_support::compare::assert_ui; +use cargo_test_support::current_dir; +use cargo_test_support::file; +use cargo_test_support::str; + +#[cargo_test] +fn case() { + cargo_test_support::registry::init(); + let project = Project::from_template(current_dir!().join("in")); + let project_root = project.root(); + let cwd = &project_root; + + snapbox::cmd::Command::cargo_ui() + .arg("add") + .args(["foo", "-p", "bar"]) + .current_dir(cwd) + .assert() + .failure() + .stdout_eq(str![""]) + .stderr_eq(file!["stderr.term.svg"]); + + assert_ui().subset_matches(current_dir!().join("out"), &project_root); +} diff --git a/tests/testsuite/cargo_add/invalid_inherited_dependency/out/Cargo.toml b/tests/testsuite/cargo_add/invalid_inherited_dependency/out/Cargo.toml new file mode 100644 index 00000000000..f8aaaaba98a --- /dev/null +++ b/tests/testsuite/cargo_add/invalid_inherited_dependency/out/Cargo.toml @@ -0,0 +1,5 @@ +[workspace] +members = ["primary", "dependency"] + +[workspace.dependencies] +foo.workspace = true diff --git a/tests/testsuite/cargo_add/invalid_inherited_dependency/out/dependency/Cargo.toml b/tests/testsuite/cargo_add/invalid_inherited_dependency/out/dependency/Cargo.toml new file mode 100644 index 00000000000..0de6f9b15a8 --- /dev/null +++ b/tests/testsuite/cargo_add/invalid_inherited_dependency/out/dependency/Cargo.toml @@ -0,0 +1,4 @@ +[package] +name = "foo" +version = "0.0.0" +edition = "2015" diff --git a/tests/testsuite/cargo_add/invalid_inherited_dependency/out/primary/Cargo.toml b/tests/testsuite/cargo_add/invalid_inherited_dependency/out/primary/Cargo.toml new file mode 100644 index 00000000000..fb520246281 --- /dev/null +++ b/tests/testsuite/cargo_add/invalid_inherited_dependency/out/primary/Cargo.toml @@ -0,0 +1,4 @@ +[package] +name = "bar" +version = "0.0.0" +edition = "2015" diff --git a/tests/testsuite/cargo_add/invalid_inherited_dependency/stderr.term.svg b/tests/testsuite/cargo_add/invalid_inherited_dependency/stderr.term.svg new file mode 100644 index 00000000000..59207de00a8 --- /dev/null +++ b/tests/testsuite/cargo_add/invalid_inherited_dependency/stderr.term.svg @@ -0,0 +1,27 @@ + + + + + + + error: dependency (foo) specified without providing a local path, Git repository, or version + + + + + + diff --git a/tests/testsuite/cargo_add/mod.rs b/tests/testsuite/cargo_add/mod.rs index 11e9f5fd2a4..5a8f814ad11 100644 --- a/tests/testsuite/cargo_add/mod.rs +++ b/tests/testsuite/cargo_add/mod.rs @@ -50,6 +50,7 @@ mod help; mod infer_prerelease; mod invalid_arg; mod invalid_git_name; +mod invalid_inherited_dependency; mod invalid_key_inherit_dependency; mod invalid_key_overwrite_inherit_dependency; mod invalid_key_rename_inherit_dependency;