Try to better handle restricted crate names.

Author: ehuss

Cargo.toml

@@ -64,6 +64,7 @@ tar = { version = "0.4.26", default-features = false }
 tempfile = "3.0"
 termcolor = "1.0"
 toml = "0.5.3"
+unicode-xid = "0.2.0"
 url = "2.0"
 walkdir = "2.2"
 clap = "2.31.2"

src/cargo/core/compiler/layout.rs

@@ -129,12 +129,6 @@ pub struct Layout {
     _lock: FileLock,
 }
 
-pub fn is_bad_artifact_name(name: &str) -> bool {
-    ["deps", "examples", "build", "incremental"]
-        .iter()
-        .any(|&reserved| reserved == name)
-}
-
 impl Layout {
     /// Calculate the paths for build output, lock the build directory, and return as a Layout.
     ///

src/cargo/core/compiler/mod.rs

@@ -37,7 +37,6 @@ pub use self::custom_build::{BuildOutput, BuildScriptOutputs, BuildScripts};
 pub use self::job::Freshness;
 use self::job::{Job, Work};
 use self::job_queue::{JobQueue, JobState};
-pub use self::layout::is_bad_artifact_name;
 use self::output_depinfo::output_depinfo;
 use self::unit_dependencies::UnitDep;
 pub use crate::core::compiler::unit::{Unit, UnitInterner};

src/cargo/ops/cargo_new.rs

@@ -1,7 +1,7 @@
-use crate::core::{compiler, Workspace};
+use crate::core::{Shell, Workspace};
 use crate::util::errors::{CargoResult, CargoResultExt};
 use crate::util::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo};
-use crate::util::{paths, validate_package_name, Config};
+use crate::util::{paths, restricted_names, Config};
 use git2::Config as GitConfig;
 use git2::Repository as GitRepository;
 use serde::de;
@@ -155,41 +155,71 @@ fn get_name<'a>(path: &'a Path, opts: &'a NewOptions) -> CargoResult<&'a str> {
     })
 }
 
-fn check_name(name: &str, opts: &NewOptions) -> CargoResult<()> {
-    // If --name is already used to override, no point in suggesting it
-    // again as a fix.
-    let name_help = match opts.name {
-        Some(_) => "",
-        None => "\nuse --name to override crate name",
-    };
+fn check_name(name: &str, name_help: &str, has_bin: bool, shell: &mut Shell) -> CargoResult<()> {
+    restricted_names::validate_package_name(name, "crate name", name_help)?;
 
-    // Ban keywords + test list found at
-    // https://doc.rust-lang.org/reference/keywords.html
-    let blacklist = [
-        "abstract", "alignof", "as", "become", "box", "break", "const", "continue", "crate", "do",
-        "else", "enum", "extern", "false", "final", "fn", "for", "if", "impl", "in", "let", "loop",
-        "macro", "match", "mod", "move", "mut", "offsetof", "override", "priv", "proc", "pub",
-        "pure", "ref", "return", "self", "sizeof", "static", "struct", "super", "test", "trait",
-        "true", "type", "typeof", "unsafe", "unsized", "use", "virtual", "where", "while", "yield",
-    ];
-    if blacklist.contains(&name) || (opts.kind.is_bin() && compiler::is_bad_artifact_name(name)) {
+    if restricted_names::is_keyword(name) {
         anyhow::bail!(
-            "The name `{}` cannot be used as a crate name{}",
+            "the name `{}` cannot be used as a crate name, it is a Rust keyword{}",
             name,
             name_help
-        )
+        );
     }
-
-    if let Some(ref c) = name.chars().next() {
-        if c.is_digit(10) {
+    if restricted_names::is_conflicting_artifact_name(name) {
+        if has_bin {
             anyhow::bail!(
-                "Package names starting with a digit cannot be used as a crate name{}",
+                "the name `{}` cannot be used as a crate name, \
+                it conflicts with cargo's build directory names{}",
+                name,
                 name_help
-            )
+            );
+        } else {
+            shell.warn(format!(
+                "the name `{}` will not support binary \
+                executables with that name, \
+                it conflicts with cargo's build directory names",
+                name
+            ))?;
         }
     }
+    if name == "test" {
+        anyhow::bail!(
+            "the name `test` cannot be used as a crate name, \
+            it conflicts with Rust's built-in test library{}",
+            name_help
+        );
+    }
+    if ["core", "std", "alloc", "proc_macro", "proc-macro"].contains(&name) {
+        shell.warn(format!(
+            "the name `{}` is part of Rust's standard library\n\
+            It is recommended to use a different name to avoid problems.",
+            name
+        ))?;
+    }
+    if restricted_names::is_windows_reserved(name) {
+        if cfg!(windows) {
+            anyhow::bail!(
+                "cannot use name `{}`, it is a reserved Windows filename{}",
+                name,
+                name_help
+            );
+        } else {
+            shell.warn(format!(
+                "the name `{}` is a reserved Windows filename\n\
+                This package will not work on Windows platforms.",
+                name
+            ))?;
+        }
+    }
+    if restricted_names::is_non_ascii_name(name) {
+        shell.warn(format!(
+            "the name `{}` contains non-ASCII characters\n\
+            Support for non-ASCII crate names is experimental and only valid \
+            on the nightly toolchain.",
+            name
+        ))?;
+    }
 
-    validate_package_name(name, "crate name", name_help)?;
     Ok(())
 }
 
@@ -337,7 +367,7 @@ pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> {
     }
 
     let name = get_name(path, opts)?;
-    check_name(name, opts)?;
+    check_name(name, "", opts.kind.is_bin(), &mut config.shell())?;
 
     let mkopts = MkOptions {
         version_control: opts.version_control,
@@ -372,7 +402,6 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
     }
 
     let name = get_name(path, opts)?;
-    check_name(name, opts)?;
 
     let mut src_paths_types = vec![];
 
@@ -385,6 +414,14 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
         // Maybe when doing `cargo init --bin` inside a library package stub,
         // user may mean "initialize for library, but also add binary target"
     }
+    let has_bin = src_paths_types.iter().any(|x| x.bin);
+    // If --name is already used to override, no point in suggesting it
+    // again as a fix.
+    let name_help = match opts.name {
+        Some(_) => "",
+        None => "\nuse --name to override crate name",
+    };
+    check_name(name, name_help, has_bin, &mut config.shell())?;
 
     let mut version_control = opts.version_control;
 
@@ -426,7 +463,7 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
         version_control,
         path,
         name,
-        bin: src_paths_types.iter().any(|x| x.bin),
+        bin: has_bin,
         source_files: src_paths_types,
         edition: opts.edition.as_ref().map(|s| &**s),
         registry: opts.registry.as_ref().map(|s| &**s),

src/cargo/util/mod.rs

@@ -19,6 +19,7 @@ pub use self::paths::{dylib_path_envvar, normalize_path};
 pub use self::process_builder::{process, ProcessBuilder};
 pub use self::progress::{Progress, ProgressStyle};
 pub use self::read2::read2;
+pub use self::restricted_names::validate_package_name;
 pub use self::rustc::Rustc;
 pub use self::sha256::Sha256;
 pub use self::to_semver::ToSemver;
@@ -51,6 +52,7 @@ pub mod process_builder;
 pub mod profile;
 mod progress;
 mod read2;
+pub mod restricted_names;
 pub mod rustc;
 mod sha256;
 pub mod to_semver;
@@ -68,22 +70,6 @@ pub fn elapsed(duration: Duration) -> String {
     }
 }
 
-/// Check the base requirements for a package name.
-///
-/// This can be used for other things than package names, to enforce some
-/// level of sanity. Note that package names have other restrictions
-/// elsewhere. `cargo new` has a few restrictions, such as checking for
-/// reserved names. crates.io has even more restrictions.
-pub fn validate_package_name(name: &str, what: &str, help: &str) -> CargoResult<()> {
-    if let Some(ch) = name
-        .chars()
-        .find(|ch| !ch.is_alphanumeric() && *ch != '_' && *ch != '-')
-    {
-        anyhow::bail!("Invalid character `{}` in {}: `{}`{}", ch, what, name, help);
-    }
-    Ok(())
-}
-
 /// Whether or not this running in a Continuous Integration environment.
 pub fn is_ci() -> bool {
     std::env::var("CI").is_ok() || std::env::var("TF_BUILD").is_ok()

src/cargo/util/restricted_names.rs

@@ -0,0 +1,83 @@
+//! Helpers for validating and checking names like package and crate names.
+
+use crate::util::CargoResult;
+use anyhow::bail;
+
+/// Returns `true` if the name contains non-ASCII characters.
+pub fn is_non_ascii_name(name: &str) -> bool {
+    name.chars().any(|ch| ch > '\x7f')
+}
+
+/// A Rust keyword.
+pub fn is_keyword(name: &str) -> bool {
+    // See https://doc.rust-lang.org/reference/keywords.html
+    [
+        "Self", "abstract", "as", "async", "await", "become", "box", "break", "const", "continue",
+        "crate", "do", "dyn", "else", "enum", "extern", "false", "final", "fn", "for", "if",
+        "impl", "in", "let", "loop", "macro", "match", "mod", "move", "mut", "override", "priv",
+        "pub", "ref", "return", "self", "static", "struct", "super", "trait", "true", "try",
+        "type", "typeof", "unsafe", "unsized", "use", "virtual", "where", "while", "yield",
+    ]
+    .contains(&name)
+}
+
+/// These names cannot be used on Windows, even with an extension.
+pub fn is_windows_reserved(name: &str) -> bool {
+    [
+        "con", "prn", "aux", "nul", "com1", "com2", "com3", "com4", "com5", "com6", "com7", "com8",
+        "com9", "lpt1", "lpt2", "lpt3", "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9",
+    ]
+    .contains(&name.to_ascii_lowercase().as_str())
+}
+
+/// An artifact with this name will conflict with one of Cargo's build directories.
+pub fn is_conflicting_artifact_name(name: &str) -> bool {
+    ["deps", "examples", "build", "incremental"].contains(&name)
+}
+
+/// Check the base requirements for a package name.
+///
+/// This can be used for other things than package names, to enforce some
+/// level of sanity. Note that package names have other restrictions
+/// elsewhere. `cargo new` has a few restrictions, such as checking for
+/// reserved names. crates.io has even more restrictions.
+pub fn validate_package_name(name: &str, what: &str, help: &str) -> CargoResult<()> {
+    let mut chars = name.chars();
+    if let Some(ch) = chars.next() {
+        if ch.is_digit(10) {
+            // A specific error for a potentially common case.
+            bail!(
+                "the name `{}` cannot be used as a {}, \
+                the name cannot start with a digit{}",
+                name,
+                what,
+                help
+            );
+        }
+        if !(unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_') {
+            bail!(
+                "invalid character `{}` in {}: `{}`, \
+                the first character must be a Unicode XID start character \
+                (most letters or `_`){}",
+                ch,
+                what,
+                name,
+                help
+            );
+        }
+    }
+    for ch in chars {
+        if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-') {
+            bail!(
+                "invalid character `{}` in {}: `{}`, \
+                characters must be Unicode XID characters \
+                (numbers, `-`, `_`, or most letters){}",
+                ch,
+                what,
+                name,
+                help
+            );
+        }
+    }
+    Ok(())
+}

src/cargo/util/toml/targets.rs

@@ -18,8 +18,9 @@ use super::{
     LibKind, PathValue, StringOrBool, StringOrVec, TomlBenchTarget, TomlBinTarget,
     TomlExampleTarget, TomlLibTarget, TomlManifest, TomlTarget, TomlTestTarget,
 };
-use crate::core::{compiler, Edition, Feature, Features, Target};
+use crate::core::{Edition, Feature, Features, Target};
 use crate::util::errors::{CargoResult, CargoResultExt};
+use crate::util::restricted_names;
 
 pub fn targets(
     features: &Features,
@@ -286,7 +287,7 @@ fn clean_bins(
             ));
         }
 
-        if compiler::is_bad_artifact_name(&name) {
+        if restricted_names::is_conflicting_artifact_name(&name) {
             anyhow::bail!("the binary target name `{}` is forbidden", name)
         }
     }

tests/testsuite/alt_registry.rs

@@ -644,7 +644,7 @@ fn bad_registry_name() {
 [ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
 
 Caused by:
-  Invalid character ` ` in registry name: `bad name`",
+  invalid character ` ` in registry name: `bad name`, [..]",
         )
         .run();
 
@@ -661,7 +661,7 @@ Caused by:
             .arg("--registry")
             .arg("bad name")
             .with_status(101)
-            .with_stderr("[ERROR] Invalid character ` ` in registry name: `bad name`")
+            .with_stderr("[ERROR] invalid character ` ` in registry name: `bad name`, [..]")
             .run();
     }
 }

tests/testsuite/build.rs

@@ -299,7 +299,7 @@ fn cargo_compile_with_invalid_package_name() {
 [ERROR] failed to parse manifest at `[..]`
 
 Caused by:
-  Invalid character `:` in package name: `foo::bar`
+  invalid character `:` in package name: `foo::bar`, [..]
 ",
         )
         .run();

tests/testsuite/init.rs

@@ -342,9 +342,8 @@ fn invalid_dir_name() {
         .with_status(101)
         .with_stderr(
             "\
-[ERROR] Invalid character `.` in crate name: `foo.bar`
-use --name to override crate name
-",
+[ERROR] invalid character `.` in crate name: `foo.bar`, [..]
+use --name to override crate name",
         )
         .run();
 
@@ -361,7 +360,7 @@ fn reserved_name() {
         .with_status(101)
         .with_stderr(
             "\
-[ERROR] The name `test` cannot be used as a crate name\n\
+[ERROR] the name `test` cannot be used as a crate name, it conflicts [..]\n\
 use --name to override crate name
 ",
         )