diff --git a/src/cargo/ops/cargo_add/dependency.rs b/src/cargo/ops/cargo_add/dependency.rs index 530696fb333..5e7db7eaf87 100644 --- a/src/cargo/ops/cargo_add/dependency.rs +++ b/src/cargo/ops/cargo_add/dependency.rs @@ -1,3 +1,4 @@ +use anyhow::bail; use std::collections::BTreeMap; use std::fmt::{Display, Formatter}; use std::path::{Path, PathBuf}; @@ -61,8 +62,13 @@ impl Dependency { } } - /// Set dependency to a given version + /// Set dependency to a given source and clear any conflicting fields pub fn set_source(mut self, source: impl Into) -> Self { + let source = source.into(); + if let Source::Workspace(_) = source { + self.default_features = None; + self.rename = None; + } self.source = Some(source.into()); self } @@ -139,15 +145,26 @@ impl Dependency { /// Set the value of default-features for the dependency #[allow(dead_code)] - pub fn set_default_features(mut self, default_features: bool) -> Self { - self.default_features = Some(default_features); - self + pub fn set_default_features(mut self, default_features: bool) -> CargoResult { + if let Some(Source::Workspace(_)) = self.source { + bail!(invalid_source_for_set( + "default-features", + "WorkspaceSource" + )) + } else { + self.default_features = Some(default_features); + Ok(self) + } } /// Set the alias for the dependency - pub fn set_rename(mut self, rename: &str) -> Self { - self.rename = Some(rename.into()); - self + pub fn set_rename(mut self, rename: &str) -> CargoResult { + if let Some(Source::Workspace(_)) = self.source { + bail!(invalid_source_for_set("rename", "WorkspaceSource")) + } else { + self.rename = Some(rename.into()); + Ok(self) + } } /// Set the value of registry for the dependency @@ -486,9 +503,7 @@ impl Dependency { if str_or_1_len_table(item) { // Nothing to preserve *item = self.to_toml(crate_root); - if self.source != Some(Source::Workspace(WorkspaceSource)) { - key.fmt(); - } + key.fmt(); } else if let Some(table) = item.as_table_like_mut() { match &self.source { Some(Source::Registry(src)) => { @@ -592,9 +607,19 @@ impl Dependency { }) .unwrap_or_default(); features.extend(new_features.iter().map(|s| s.as_str())); - let features = toml_edit::value(features.into_iter().collect::()); - table.set_dotted(false); - table.insert("features", features); + if let Some(inherit_features) = &self.inherited_features { + inherit_features.iter().for_each(|f| { + features.remove(f.as_str()); + }); + } + if !features.is_empty() { + let features = + toml_edit::value(features.into_iter().collect::()); + table.set_dotted(false); + table.insert("features", features); + } else { + table.remove("features"); + } } else { table.remove("features"); } @@ -619,6 +644,12 @@ fn invalid_type(dep: &str, key: &str, actual: &str, expected: &str) -> anyhow::E anyhow::format_err!("Found {actual} for {key} when {expected} was expected for {dep}") } +fn invalid_source_for_set(field: &str, source: &str) -> anyhow::Error { + anyhow::format_err!( + "you cannot set `{field}` when the `Source` for the `Dependency` is a `{source}`" + ) +} + impl std::fmt::Display for Dependency { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if let Some(source) = self.source() { @@ -941,9 +972,24 @@ mod tests { use std::path::Path; use cargo_util::paths; + use toml_edit::{Document, Key}; use super::*; + #[test] + fn error_using_set_method() { + // default-features + Dependency::new("dep") + .set_source(WorkspaceSource::new()) + .set_default_features(false) + .expect_err("expected error, found"); + // rename + Dependency::new("dep") + .set_source(WorkspaceSource::new()) + .set_default_features(false) + .expect_err("expected error, found"); + } + #[test] fn to_toml_simple_dep() { let crate_root = @@ -991,12 +1037,12 @@ mod tests { } #[test] - fn to_toml_dep_without_default_features() { + fn to_toml_dep_without_default_features() -> CargoResult<()> { let crate_root = paths::normalize_path(&std::env::current_dir().unwrap().join(Path::new("/"))); let dep = Dependency::new("dep") .set_source(RegistrySource::new("1.0")) - .set_default_features(false); + .set_default_features(false)?; let key = dep.toml_key(); let item = dep.to_toml(&crate_root); @@ -1007,6 +1053,7 @@ mod tests { assert_eq!(dep.get("default-features").unwrap().as_bool(), Some(false)); verify_roundtrip(&crate_root, key, &item); + Ok(()) } #[test] @@ -1047,12 +1094,12 @@ mod tests { } #[test] - fn to_toml_renamed_dep() { + fn to_toml_renamed_dep() -> CargoResult<()> { let crate_root = paths::normalize_path(&std::env::current_dir().unwrap().join(Path::new("/"))); let dep = Dependency::new("dep") .set_source(RegistrySource::new("1.0")) - .set_rename("d"); + .set_rename("d")?; let key = dep.toml_key(); let item = dep.to_toml(&crate_root); @@ -1063,6 +1110,7 @@ mod tests { assert_eq!(dep.get("package").unwrap().as_str(), Some("dep")); verify_roundtrip(&crate_root, key, &item); + Ok(()) } #[test] @@ -1085,13 +1133,13 @@ mod tests { } #[test] - fn to_toml_complex_dep() { + fn to_toml_complex_dep() -> CargoResult<()> { let crate_root = paths::normalize_path(&std::env::current_dir().unwrap().join(Path::new("/"))); let dep = Dependency::new("dep") .set_source(RegistrySource::new("1.0")) - .set_default_features(false) - .set_rename("d"); + .set_default_features(false)? + .set_rename("d")?; let key = dep.toml_key(); let item = dep.to_toml(&crate_root); @@ -1104,6 +1152,51 @@ mod tests { assert_eq!(dep.get("default-features").unwrap().as_bool(), Some(false)); verify_roundtrip(&crate_root, key, &item); + Ok(()) + } + + #[test] + fn update_source_to_workspace_simple() -> CargoResult<()> { + let crate_root = + paths::normalize_path(&std::env::current_dir().unwrap().join(Path::new("/"))); + let dep = Dependency::new("dep") + .set_source(RegistrySource::new("1.0")) + .set_default_features(true)? + .set_rename("d")?; + let key = dep.toml_key(); + let item = dep.to_toml(&crate_root); + + assert_eq!(key, "d".to_owned()); + assert!(item.is_inline_table()); + + let dep_table = item.as_inline_table().unwrap(); + assert_eq!(dep_table.get("package").unwrap().as_str(), Some("dep")); + assert_eq!(dep_table.get("version").unwrap().as_str(), Some("1.0")); + assert_eq!( + dep_table.get("default-features").unwrap().as_bool(), + Some(true) + ); + + let dep = dep.clone().set_source(WorkspaceSource::new()); + let key = dep.toml_key(); + let mut key_mut = Key::new(key); + let mut dep_item = item.clone(); + dep.update_toml(&crate_root, &mut key_mut.as_mut(), &mut dep_item); + + assert_eq!(key, "dep".to_owned()); + assert!(str_or_1_len_table(&dep_item)); + let dep_table = dep_item.as_inline_table().unwrap(); + assert!(dep_table.get("package").is_none()); + assert!(dep_table.get("version").is_none()); + assert!(dep_table.get("default-features").is_none()); + assert_eq!(dep_table.get("workspace").unwrap().as_bool(), Some(true)); + assert!(dep_table.is_dotted()); + + let mut doc = Document::new(); + doc.insert(key_mut.get(), dep_item); + assert_eq!(doc.to_string(), "dep.workspace = true\n"); + verify_roundtrip(&crate_root, key, &item); + Ok(()) } #[test] diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index dff93e91b02..aa3bbc3bad8 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -284,6 +284,7 @@ fn resolve_dependency( // Checking for a workspace dependency happens first since a member could be specified // in the workspace dependencies table as a dependency if let Some(_dep) = find_workspace_dep(dependency.toml_key(), ws.root_manifest()).ok() { + check_invalid_ws_keys(dependency.toml_key(), arg)?; dependency = dependency.set_source(WorkspaceSource::new()); } else if let Some(package) = ws.members().find(|p| p.name().as_str() == dependency.name) { // Only special-case workspaces when the user doesn't provide any extra @@ -565,7 +566,7 @@ fn populate_dependency(mut dependency: Dependency, arg: &DepOp) -> Dependency { } if let Some(rename) = &arg.rename { - dependency = dependency.set_rename(rename); + dependency = dependency.set_rename(rename).unwrap(); } dependency