From fc1f5a43fb6bc20cc4894591bfd1af1ca87b46db Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Sun, 2 Oct 2022 18:03:01 -0700
Subject: [PATCH] Provide a better error message when mixing dep: with /

---
 src/cargo/core/summary.rs              |  30 ++++++
 tests/testsuite/features_namespaced.rs | 126 +++++++++++++++++++++++++
 2 files changed, 156 insertions(+)

diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs
index 9c5396eecf5..8a7238e4a56 100644
--- a/src/cargo/core/summary.rs
+++ b/src/cargo/core/summary.rs
@@ -277,6 +277,36 @@ fn build_feature_map(
                             feature
                         );
                     }
+
+                    // dep: cannot be combined with /
+                    if let Some(stripped_dep) = dep_name.strip_prefix("dep:") {
+                        let has_other_dep = explicitly_listed.contains(stripped_dep);
+                        let is_optional = dep_map
+                            .get(stripped_dep)
+                            .iter()
+                            .flat_map(|d| d.iter())
+                            .any(|d| d.is_optional());
+                        let extra_help = if *weak || has_other_dep || !is_optional {
+                            // In this case, the user should just remove dep:.
+                            // Note that "hiding" an optional dependency
+                            // wouldn't work with just a single `dep:foo?/bar`
+                            // because there would not be any way to enable
+                            // `foo`.
+                            String::new()
+                        } else {
+                            format!(
+                                "\nIf the intent is to avoid creating an implicit feature \
+                                 `{stripped_dep}` for an optional dependency, \
+                                 then consider replacing this with two values:\n    \
+                                 \"dep:{stripped_dep}\", \"{stripped_dep}/{dep_feature}\""
+                            )
+                        };
+                        bail!(
+                            "feature `{feature}` includes `{fv}` with both `dep:` and `/`\n\
+                            To fix this, remove the `dep:` prefix.{extra_help}"
+                        )
+                    }
+
                     // Validation of the feature name will be performed in the resolver.
                     if !is_any_dep {
                         bail!(
diff --git a/tests/testsuite/features_namespaced.rs b/tests/testsuite/features_namespaced.rs
index 0fe1c964bc1..e93a15e8f9f 100644
--- a/tests/testsuite/features_namespaced.rs
+++ b/tests/testsuite/features_namespaced.rs
@@ -1073,3 +1073,129 @@ feat3 = ["feat2"]
         )],
     );
 }
+
+#[cargo_test]
+fn namespaced_feature_together() {
+    // Check for an error when `dep:` is used with `/`
+    Package::new("bar", "1.0.0")
+        .feature("bar-feat", &[])
+        .publish();
+
+    // Non-optional shouldn't have extra err.
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+
+                [dependencies]
+                bar = "1.0"
+
+                [features]
+                f1 = ["dep:bar/bar-feat"]
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
+
+Caused by:
+  feature `f1` includes `dep:bar/bar-feat` with both `dep:` and `/`
+  To fix this, remove the `dep:` prefix.
+",
+        )
+        .run();
+
+    // Weak dependency shouldn't have extra err.
+    p.change_file(
+        "Cargo.toml",
+        r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            [dependencies]
+            bar = {version = "1.0", optional = true }
+
+            [features]
+            f1 = ["dep:bar?/bar-feat"]
+        "#,
+    );
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
+
+Caused by:
+  feature `f1` includes `dep:bar?/bar-feat` with both `dep:` and `/`
+  To fix this, remove the `dep:` prefix.
+",
+        )
+        .run();
+
+    // If dep: is already specified, shouldn't have extra err.
+    p.change_file(
+        "Cargo.toml",
+        r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            [dependencies]
+            bar = {version = "1.0", optional = true }
+
+            [features]
+            f1 = ["dep:bar", "dep:bar/bar-feat"]
+        "#,
+    );
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
+
+Caused by:
+  feature `f1` includes `dep:bar/bar-feat` with both `dep:` and `/`
+  To fix this, remove the `dep:` prefix.
+",
+        )
+        .run();
+
+    // Only when the other 3 cases aren't true should it give some extra help.
+    p.change_file(
+        "Cargo.toml",
+        r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            [dependencies]
+            bar = {version = "1.0", optional = true }
+
+            [features]
+            f1 = ["dep:bar/bar-feat"]
+        "#,
+    );
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
+
+Caused by:
+  feature `f1` includes `dep:bar/bar-feat` with both `dep:` and `/`
+  To fix this, remove the `dep:` prefix.
+  If the intent is to avoid creating an implicit feature `bar` for an optional \
+  dependency, then consider replacing this with two values:
+      \"dep:bar\", \"bar/bar-feat\"
+",
+        )
+        .run();
+}