Auto merge of #13046 - linyihai:cargo-add-public, r=epage

Add `--public` for `cargo add`

## What does this PR try to resolve?
Complete #13037

This PR want to add `--public/--no public` flag for `cargo add`

Note: this assumes we'll remove workspace inheritance support for `public` as it sounds like we'll be reverting it rust-lang/rust#44663 (comment).  If we decide to keep workspace inheritance, we'll need to come back and update this.

## How should we test and review this PR?
Most of Code were reference `cargo add --optional`, So can reviewed the new code based on the part of  `optional` code.

The new testcases were origin from the `cargo add --optional` part.
- `public` testcase：there is no dependencies and will be add `public` dependencies.
- `no_public` testcase:  there is no dependencies  and will be add `no_public` dependencies.
- `overwrite_public` testcase: the dependencies already exists but will be overwrite with `public`.
- `overwrite_no_public` testcase: the dependencies already exists but will be overwrite with `no_public`.
- `overwrite_public_with_no_public` testcase: the dependencies already marked as `no_public` and will be overwrite with `public`.
- `overwrite_no_public_with_public` testcase: the dependencies already marked as `public` and will be overwrite with `no_public`.

Author: bors

src/bin/cargo/commands/add.rs

@@ -62,6 +62,19 @@ The package name will be exposed as feature of your crate.")
 The package will be removed from your features.")
                 .conflicts_with("dev")
                 .overrides_with("optional"),
+            flag("public", "Mark the dependency as public")
+                .conflicts_with("dev")
+                .conflicts_with("build")
+                .long_help("Mark the dependency as public
+
+The dependency can be referenced in your library's public API."),
+            flag("no-public", "Mark the dependency as private")
+                .conflicts_with("dev")
+                .conflicts_with("build")
+                .overrides_with("public")
+                .long_help("Mark the dependency as private
+
+While you can use the crate in your implementation, it cannot be referenced in your public API."),
             clap::Arg::new("rename")
                 .long("rename")
                 .action(ArgAction::Set)
@@ -235,6 +248,7 @@ fn parse_dependencies(config: &Config, matches: &ArgMatches) -> CargoResult<Vec<
     };
     let default_features = default_features(matches);
     let optional = optional(matches);
+    let public = public(matches);
 
     let mut crates = matches
         .get_many::<String>("crates")
@@ -325,6 +339,7 @@ fn parse_dependencies(config: &Config, matches: &ArgMatches) -> CargoResult<Vec<
             features,
             default_features,
             optional,
+            public,
             registry: registry.clone(),
             path: path.map(String::from),
             git: git.map(String::from),
@@ -353,6 +368,10 @@ fn optional(matches: &ArgMatches) -> Option<bool> {
     resolve_bool_arg(matches.flag("optional"), matches.flag("no-optional"))
 }
 
+fn public(matches: &ArgMatches) -> Option<bool> {
+    resolve_bool_arg(matches.flag("public"), matches.flag("no-public"))
+}
+
 fn resolve_bool_arg(yes: bool, no: bool) -> Option<bool> {
     match (yes, no) {
         (true, false) => Some(true),

src/cargo/ops/cargo_add/mod.rs

@@ -244,6 +244,9 @@ pub struct DepOp {
     /// Whether dependency is optional
     pub optional: Option<bool>,
 
+    /// Whether dependency is public
+    pub public: Option<bool>,
+
     /// Registry for looking up dependency version
     pub registry: Option<String>,
 
@@ -758,6 +761,13 @@ fn populate_dependency(mut dependency: Dependency, arg: &DepOp) -> Dependency {
             dependency.optional = None;
         }
     }
+    if let Some(value) = arg.public {
+        if value {
+            dependency.public = Some(true);
+        } else {
+            dependency.public = None;
+        }
+    }
     if let Some(value) = arg.default_features {
         if value {
             dependency.default_features = None;
@@ -945,6 +955,9 @@ fn print_action_msg(shell: &mut Shell, dep: &DependencyUI, section: &[String]) -
     if dep.optional().unwrap_or(false) {
         write!(message, " optional")?;
     }
+    if dep.public().unwrap_or(false) {
+        write!(message, " public")?;
+    }
     let section = if section.len() == 1 {
         section[0].clone()
     } else {

src/cargo/util/toml_mut/dependency.rs

@@ -25,6 +25,9 @@ pub struct Dependency {
     /// Whether the dependency is opted-in with a feature flag.
     pub optional: Option<bool>,
 
+    /// Whether the dependency is marked as public.
+    pub public: Option<bool>,
+
     /// List of features to add (or None to keep features unchanged).
     pub features: Option<IndexSet<String>>,
     /// Whether default features are enabled.
@@ -48,6 +51,7 @@ impl Dependency {
         Self {
             name: name.into(),
             optional: None,
+            public: None,
             features: None,
             default_features: None,
             inherited_features: None,
@@ -163,6 +167,11 @@ impl Dependency {
         self.optional
     }
 
+    /// Get whether the dep is public.
+    pub fn public(&self) -> Option<bool> {
+        self.public
+    }
+
     /// Get the SourceID for this dependency.
     pub fn source_id(&self, config: &Config) -> CargoResult<MaybeWorkspace<SourceId>> {
         match &self.source.as_ref() {
@@ -325,16 +334,18 @@ impl Dependency {
             };
 
             let optional = table.get("optional").and_then(|v| v.as_bool());
+            let public = table.get("public").and_then(|v| v.as_bool());
 
             let dep = Self {
                 name,
-                rename,
-                source: Some(source),
-                registry,
-                default_features,
-                features,
                 optional,
+                public,
+                features,
+                default_features,
                 inherited_features: None,
+                source: Some(source),
+                registry,
+                rename,
             };
             Ok(dep)
         } else {
@@ -366,6 +377,7 @@ impl Dependency {
             crate_root.display()
         );
         let table: toml_edit::Item = match (
+            self.public.unwrap_or(false),
             self.optional.unwrap_or(false),
             self.features.as_ref(),
             self.default_features.unwrap_or(true),
@@ -375,21 +387,22 @@ impl Dependency {
         ) {
             // Extra short when version flag only
             (
+                false,
                 false,
                 None,
                 true,
                 Some(Source::Registry(RegistrySource { version: v })),
                 None,
                 None,
             ) => toml_edit::value(v),
-            (false, None, true, Some(Source::Workspace(WorkspaceSource {})), None, None) => {
+            (false, false, None, true, Some(Source::Workspace(WorkspaceSource {})), None, None) => {
                 let mut table = toml_edit::InlineTable::default();
                 table.set_dotted(true);
                 table.insert("workspace", true.into());
                 toml_edit::value(toml_edit::Value::InlineTable(table))
             }
             // Other cases are represented as an inline table
-            (_, _, _, _, _, _) => {
+            (_, _, _, _, _, _, _) => {
                 let mut table = toml_edit::InlineTable::default();
 
                 match &self.source {
@@ -442,6 +455,9 @@ impl Dependency {
                 if let Some(v) = self.optional {
                     table.insert("optional", v.into());
                 }
+                if let Some(v) = self.public {
+                    table.insert("public", v.into());
+                }
 
                 toml_edit::value(toml_edit::Value::InlineTable(table))
             }
@@ -579,6 +595,15 @@ impl Dependency {
                     table.remove("optional");
                 }
             }
+            match self.public {
+                Some(v) => {
+                    table.set_dotted(false);
+                    overwrite_value(table, "public", v);
+                }
+                None => {
+                    table.remove("public");
+                }
+            }
         } else {
             unreachable!("Invalid dependency type: {}", item.type_name());
         }

tests/testsuite/cargo_add/detect_workspace_inherit_public/in/Cargo.toml

@@ -0,0 +1,5 @@
+[workspace]
+members = ["primary", "dependency"]
+
+[workspace.dependencies]
+foo = { version = "0.0.0", path = "./dependency"}

tests/testsuite/cargo_add/detect_workspace_inherit_public/in/dependency/Cargo.toml

@@ -0,0 +1,3 @@
+[package]
+name = "foo"
+version = "0.0.0"

tests/testsuite/cargo_add/detect_workspace_inherit_public/in/dependency/src/lib.rs

@@ -0,0 +1,4 @@
+cargo-features = ["public-dependency"]
+[package]
+name = "bar"
+version = "0.0.0"

tests/testsuite/cargo_add/detect_workspace_inherit_public/in/primary/Cargo.toml

@@ -0,0 +1,26 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::prelude::*;
+use cargo_test_support::Project;
+
+use cargo_test_support::curr_dir;
+
+#[cargo_test]
+fn case() {
+    cargo_test_support::registry::init();
+
+    let project = Project::from_template(curr_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    snapbox::cmd::Command::cargo_ui()
+        .arg("add")
+        .args(["foo", "-p", "bar", "--public"])
+        .masquerade_as_nightly_cargo(&["public-dependency"])
+        .current_dir(cwd)
+        .assert()
+        .success()
+        .stdout_matches_path(curr_dir!().join("stdout.log"))
+        .stderr_matches_path(curr_dir!().join("stderr.log"));
+
+    assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
+}

tests/testsuite/cargo_add/detect_workspace_inherit_public/in/primary/src/lib.rs

@@ -0,0 +1,5 @@
+[workspace]
+members = ["primary", "dependency"]
+
+[workspace.dependencies]
+foo = { version = "0.0.0", path = "./dependency"}

tests/testsuite/cargo_add/detect_workspace_inherit_public/mod.rs

@@ -0,0 +1,3 @@
+[package]
+name = "foo"
+version = "0.0.0"

tests/testsuite/cargo_add/detect_workspace_inherit_public/out/Cargo.toml

@@ -0,0 +1,7 @@
+cargo-features = ["public-dependency"]
+[package]
+name = "bar"
+version = "0.0.0"
+
+[dependencies]
+foo = { workspace = true, public = true }

tests/testsuite/cargo_add/detect_workspace_inherit_public/out/dependency/Cargo.toml

@@ -0,0 +1 @@
+      Adding foo (workspace) to public dependencies.

tests/testsuite/cargo_add/detect_workspace_inherit_public/out/primary/Cargo.toml

@@ -32,6 +32,17 @@ Options:
 
           The package will be removed from your features.
 
+      --public
+          Mark the dependency as public
+          
+          The dependency can be referenced in your library's public API.
+
+      --no-public
+          Mark the dependency as private
+          
+          While you can use the crate in your implementation, it cannot be referenced in your public
+          API.
+
       --rename <NAME>
           Rename the dependency
 

tests/testsuite/cargo_add/detect_workspace_inherit_public/stderr.log

@@ -12,6 +12,7 @@ mod deprecated_section;
 mod detect_workspace_inherit;
 mod detect_workspace_inherit_features;
 mod detect_workspace_inherit_optional;
+mod detect_workspace_inherit_public;
 mod dev;
 mod dev_build_conflict;
 mod dev_prefer_existing_version;
@@ -65,6 +66,7 @@ mod namever;
 mod no_args;
 mod no_default_features;
 mod no_optional;
+mod no_public;
 mod offline_empty_cache;
 mod optional;
 mod overwrite_default_features;
@@ -81,11 +83,15 @@ mod overwrite_no_default_features;
 mod overwrite_no_default_features_with_default_features;
 mod overwrite_no_optional;
 mod overwrite_no_optional_with_optional;
+mod overwrite_no_public;
+mod overwrite_no_public_with_public;
 mod overwrite_optional;
 mod overwrite_optional_with_no_optional;
 mod overwrite_path_noop;
 mod overwrite_path_with_version;
 mod overwrite_preserves_inline_table;
+mod overwrite_public;
+mod overwrite_public_with_no_public;
 mod overwrite_rename_with_no_rename;
 mod overwrite_rename_with_rename;
 mod overwrite_rename_with_rename_noop;
@@ -103,6 +109,7 @@ mod preserve_dep_std_table;
 mod preserve_features_table;
 mod preserve_sorted;
 mod preserve_unsorted;
+mod public;
 mod quiet;
 mod registry;
 mod rename;

tests/testsuite/cargo_add/detect_workspace_inherit_public/stdout.log

@@ -0,0 +1,6 @@
+cargo-features = ["public-dependency"]
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"

tests/testsuite/cargo_add/help/stdout.log

@@ -0,0 +1,26 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::prelude::*;
+use cargo_test_support::Project;
+
+use cargo_test_support::curr_dir;
+
+#[cargo_test]
+fn case() {
+    cargo_test_support::registry::init();
+    cargo_test_support::registry::Package::new("my-package", "0.1.0").publish();
+    let project = Project::from_template(curr_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    snapbox::cmd::Command::cargo_ui()
+        .arg("add")
+        .arg_line("my-package --no-public")
+        .current_dir(cwd)
+        .masquerade_as_nightly_cargo(&["public-dependency"])
+        .assert()
+        .success()
+        .stdout_matches_path(curr_dir!().join("stdout.log"))
+        .stderr_matches_path(curr_dir!().join("stderr.log"));
+
+    assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
+}

tests/testsuite/cargo_add/mod.rs

@@ -0,0 +1,9 @@
+cargo-features = ["public-dependency"]
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+
+[dependencies]
+my-package = "0.1.0"

tests/testsuite/cargo_add/no_public/in/Cargo.toml

@@ -0,0 +1,2 @@
+    Updating `dummy-registry` index
+      Adding my-package v0.1.0 to dependencies.

tests/testsuite/cargo_add/no_public/in/src/lib.rs

@@ -0,0 +1,9 @@
+cargo-features = ["public-dependency"]
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+
+[dependencies]
+my-package = "0.1.0"