Reserve some names for named profiles.

Just to give a little flexibility in the future in case we want to use
these, or that they could cause confusion. Also updated the error text a
little.

Author: ehuss

src/cargo/util/command_prelude.rs

@@ -343,13 +343,7 @@ pub trait ArgMatchesExt {
         default: &str,
         profile_checking: ProfileChecking,
     ) -> CargoResult<InternedString> {
-        let specified_profile = match self._value_of("profile") {
-            None => None,
-            Some(name) => {
-                TomlProfile::validate_name(name, "profile name")?;
-                Some(InternedString::new(name))
-            }
-        };
+        let specified_profile = self._value_of("profile");
 
         match profile_checking {
             ProfileChecking::Unchecked => {}
@@ -360,31 +354,62 @@ pub trait ArgMatchesExt {
             }
         }
 
-        if self._is_present("release") {
-            if !config.cli_unstable().unstable_options {
-                Ok(InternedString::new("release"))
-            } else {
-                match specified_profile {
-                    Some(name) if name != "release" => {
-                        anyhow::bail!("Conflicting usage of --profile and --release")
-                    }
-                    _ => Ok(InternedString::new("release")),
+        let conflict = |flag: &str, equiv: &str, specified: &str| -> anyhow::Error {
+            anyhow::format_err!(
+                "conflicting usage of --profile={} and --{flag}\n\
+                 The `--{flag}` flag is the same as `--profile={equiv}`.\n\
+                 Remove one flag or the other to continue.",
+                specified,
+                flag = flag,
+                equiv = equiv
+            )
+        };
+
+        let name = match (
+            self._is_present("release"),
+            self._is_present("debug"),
+            specified_profile,
+        ) {
+            (false, false, None) => default,
+            // `check` is separate from all other reservations because
+            // `--profile=check` has been stable for the `cargo rustc` command
+            // for a long time. The Unchecked commands (check, fix, rustc)
+            // have their own validation.
+            //
+            // This is explicitly ordered before the --release conflict check
+            // since `cargo rustc --profile=check --release` means to check in
+            // release mode.
+            //
+            // When stabilizing, only `cargo rustc` should be allowed to use
+            // `check` with the old semantics.
+            (_, _, Some(name @ ("test" | "bench" | "check"))) => match profile_checking {
+                ProfileChecking::Unchecked => name,
+                ProfileChecking::Checked => {
+                    bail!(
+                        "profile `{}` is reserved and not allowed to be explicitly specified",
+                        name
+                    )
                 }
+            },
+            (true, _, None | Some("release")) => "release",
+            (true, _, Some(name)) => return Err(conflict("release", "release", name)),
+            (_, true, None | Some("dev")) => "dev",
+            (_, true, Some(name)) => return Err(conflict("debug", "dev", name)),
+            // `doc` is separate from all the other reservations because
+            // [profile.doc] was historically allowed, but is deprecated and
+            // has no effect. To avoid potentially breaking projects, it is a
+            // warning, but since `--profile` is new, we can reject it
+            // completely here.
+            (_, _, Some("doc")) => {
+                bail!("profile `doc` is reserved and not allowed to be explicitly specified")
             }
-        } else if self._is_present("debug") {
-            if !config.cli_unstable().unstable_options {
-                Ok(InternedString::new("dev"))
-            } else {
-                match specified_profile {
-                    Some(name) if name != "dev" => {
-                        anyhow::bail!("Conflicting usage of --profile and --debug")
-                    }
-                    _ => Ok(InternedString::new("dev")),
-                }
+            (_, _, Some(name)) => {
+                TomlProfile::validate_name(name)?;
+                name
             }
-        } else {
-            Ok(specified_profile.unwrap_or_else(|| InternedString::new(default)))
-        }
+        };
+
+        Ok(InternedString::new(name))
     }
 
     fn packages_from_flags(&self) -> CargoResult<Packages> {

src/cargo/util/toml/mod.rs

@@ -489,10 +489,6 @@ impl TomlProfile {
         features: &Features,
         warnings: &mut Vec<String>,
     ) -> CargoResult<()> {
-        if name == "debug" {
-            warnings.push("use `[profile.dev]` to configure debug builds".to_string());
-        }
-
         if let Some(ref profile) = self.build_override {
             features.require(Feature::profile_overrides())?;
             profile.validate_override("build-override")?;
@@ -513,7 +509,7 @@ impl TomlProfile {
         }
 
         // Profile name validation
-        Self::validate_name(name, "profile name")?;
+        Self::validate_name(name)?;
 
         // Feature gate on uses of keys related to named profiles
         if self.inherits.is_some() {
@@ -533,11 +529,12 @@ impl TomlProfile {
         }
 
         // `inherits` validation
-        match &self.inherits {
-            None => {}
-            Some(inherits) => {
-                Self::validate_name(inherits, "inherits")?;
-            }
+        if matches!(self.inherits.map(|s| s.as_str()), Some("debug")) {
+            bail!(
+                "profile.{}.inherits=\"debug\" should be profile.{}.inherits=\"dev\"",
+                name,
+                name
+            );
         }
 
         match name {
@@ -569,29 +566,77 @@ impl TomlProfile {
     }
 
     /// Validate dir-names and profile names according to RFC 2678.
-    pub fn validate_name(name: &str, what: &str) -> CargoResult<()> {
+    pub fn validate_name(name: &str) -> CargoResult<()> {
         if let Some(ch) = name
             .chars()
             .find(|ch| !ch.is_alphanumeric() && *ch != '_' && *ch != '-')
         {
-            bail!("Invalid character `{}` in {}: `{}`", ch, what, name);
+            bail!(
+                "invalid character `{}` in profile name `{}`\n\
+                Allowed characters are letters, numbers, underscore, and hyphen.",
+                ch,
+                name
+            );
         }
 
-        match name {
-            "package" | "build" => {
-                bail!("Invalid {}: `{}`", what, name);
-            }
-            "debug" if what == "profile" => {
-                if what == "profile name" {
-                    // Allowed, but will emit warnings
-                } else {
-                    bail!("Invalid {}: `{}`", what, name);
-                }
-            }
-            "doc" if what == "dir-name" => {
-                bail!("Invalid {}: `{}`", what, name);
-            }
-            _ => {}
+        const SEE_DOCS: &str = "See https://doc.rust-lang.org/cargo/reference/profiles.html \
+            for more on configuring profiles.";
+
+        let lower_name = name.to_lowercase();
+        if lower_name == "debug" {
+            bail!(
+                "profile name `{}` is reserved\n\
+                 To configure the default development profile, use the name `dev` \
+                 as in [profile.dev]\n\
+                {}",
+                name,
+                SEE_DOCS
+            );
+        }
+        if lower_name == "build-override" {
+            bail!(
+                "profile name `{}` is reserved\n\
+                 To configure build dependency settings, use [profile.dev.build-override] \
+                 and [profile.release.build-override]\n\
+                 {}",
+                name,
+                SEE_DOCS
+            );
+        }
+
+        // These are some arbitrary reservations. We have no plans to use
+        // these, but it seems safer to reserve a few just in case we want to
+        // add more built-in profiles in the future. We can also uses special
+        // syntax like cargo:foo if needed. But it is unlikely these will ever
+        // be used.
+        if matches!(
+            lower_name.as_str(),
+            "build"
+                | "check"
+                | "clean"
+                | "config"
+                | "fetch"
+                | "fix"
+                | "install"
+                | "metadata"
+                | "package"
+                | "publish"
+                | "report"
+                | "root"
+                | "run"
+                | "rust"
+                | "rustc"
+                | "rustdoc"
+                | "uninstall"
+        ) || lower_name.starts_with("cargo")
+        {
+            bail!(
+                "profile name `{}` is reserved\n\
+                 Please choose a different name.\n\
+                 {}",
+                name,
+                SEE_DOCS
+            );
         }
 
         Ok(())

tests/testsuite/bad_config.rs

@@ -665,39 +665,6 @@ warning: unused manifest key: target.foo.bar
         )
         .run();
 
-    let p = project()
-        .file(
-            "Cargo.toml",
-            r#"
-               cargo-features = ["named-profiles"]
-
-               [package]
-               name = "foo"
-               version = "0.1.0"
-               authors = []
-
-               [profile.debug]
-               debug = 1
-               inherits = "dev"
-            "#,
-        )
-        .file("src/lib.rs", "")
-        .build();
-
-    p.cargo("build -Z named-profiles")
-        .masquerade_as_nightly_cargo()
-        .with_stderr(
-            "\
-warning: use `[profile.dev]` to configure debug builds
-[..]
-[..]",
-        )
-        .run();
-
-    p.cargo("build -Z named-profiles")
-        .masquerade_as_nightly_cargo()
-        .run();
-
     let p = project()
         .file(
             "Cargo.toml",