Auto merge of #11997 - hi-rustin:rustin-patch-dep-message, r=weihanglo

Better error message when getting an empty dep table
rust-lang · Apr 20, 2023 · 738c699 · 738c699
2 parents 87871cc + e0276ca
commit 738c699
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 45 deletions.
diff --git a/src/cargo/util/toml_mut/dependency.rs b/src/cargo/util/toml_mut/dependency.rs
@@ -225,62 +225,72 @@ 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 path = crate_root
+                }
+                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 path =
+                    crate_root
                         .join(path.as_str().ok_or_else(|| {
                             invalid_type(key, "path", path.type_name(), "string")
                         })?);
-                    let mut src = PathSource::new(path);
-                    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);
+                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 {
-                    anyhow::bail!("Unrecognized dependency source for `{key}`");
-                };
+                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",
+                    );
+                }
+                anyhow::bail!(msg);
+            };
             let registry = if let Some(value) = table.get("registry") {
                 Some(
                     value

diff --git a/tests/testsuite/cargo_add/empty_dep_table/in/Cargo.toml b/tests/testsuite/cargo_add/empty_dep_table/in/Cargo.toml
@@ -0,0 +1,8 @@
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+
+[dependencies]
+your-face = { }
diff --git a/tests/testsuite/cargo_add/empty_dep_table/in/src/lib.rs b/tests/testsuite/cargo_add/empty_dep_table/in/src/lib.rs
@@ -0,0 +1 @@
+
diff --git a/tests/testsuite/cargo_add/empty_dep_table/mod.rs b/tests/testsuite/cargo_add/empty_dep_table/mod.rs
@@ -0,0 +1,25 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::prelude::*;
+use cargo_test_support::Project;
+
+use crate::cargo_add::init_registry;
+use cargo_test_support::curr_dir;
+
+#[cargo_test]
+fn case() {
+    init_registry();
+    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("your-face --features eyes")
+        .current_dir(cwd)
+        .assert()
+        .code(101)
+        .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);
+}
diff --git a/tests/testsuite/cargo_add/empty_dep_table/out/Cargo.toml b/tests/testsuite/cargo_add/empty_dep_table/out/Cargo.toml
@@ -0,0 +1,8 @@
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+
+[dependencies]
+your-face = { }
diff --git a/tests/testsuite/cargo_add/empty_dep_table/stderr.log b/tests/testsuite/cargo_add/empty_dep_table/stderr.log
@@ -0,0 +1 @@
+error: unrecognized dependency source for `your-face`, expected a local path, Git repository, version, or workspace dependency to be specified
diff --git a/tests/testsuite/cargo_add/empty_dep_table/stdout.log b/tests/testsuite/cargo_add/empty_dep_table/stdout.log
diff --git a/tests/testsuite/cargo_add/mod.rs b/tests/testsuite/cargo_add/mod.rs
@@ -15,6 +15,7 @@ mod dev;
 mod dev_build_conflict;
 mod dev_prefer_existing_version;
 mod dry_run;
+mod empty_dep_table;
 mod features;
 mod features_empty;
 mod features_multiple_occurrences;