From dd81231c29213ebf422ded352897741a0f9af1cd Mon Sep 17 00:00:00 2001 From: alm Date: Tue, 27 Feb 2024 13:45:20 +0200 Subject: [PATCH] Add validation for crate/package name in new/init (#1943) * Start adding validation for crate/package name * Add cargo validations * Solve todos * Fix clippy --- Cargo.lock | 7 ++ Cargo.toml | 1 + src/lib.rs | 1 + src/new_project.rs | 17 ++++- src/package_name_validations.rs | 116 ++++++++++++++++++++++++++++++++ 5 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 src/package_name_validations.rs diff --git a/Cargo.lock b/Cargo.lock index c6dd1d30f..3ee286fb7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1131,6 +1131,7 @@ dependencies = [ "tracing", "tracing-subscriber", "trycmd", + "unicode-xid", "ureq", "url", "which 6.0.0", @@ -2344,6 +2345,12 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e51733f11c9c4f72aa0c160008246859e340b00807569a0da0e7a1079b27ba85" +[[package]] +name = "unicode-xid" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f962df74c8c05a667b5ee8bcf162993134c104e96440b663c8daa176dc772d8c" + [[package]] name = "unscanny" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 0e6806804..68ab918df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,6 +75,7 @@ path-slash = "0.2.1" pep440_rs = { version = "0.5.0", features = ["serde", "tracing"] } pep508_rs = { version = "0.4.2", features = ["serde", "tracing"] } time = "0.3.17" +unicode-xid = "0.2.4" # cli clap = { version = "4.0.0", features = ["derive", "env", "wrap_help", "unstable-styles"] } diff --git a/src/lib.rs b/src/lib.rs index dc89994e1..e6fcc644c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -55,6 +55,7 @@ mod metadata; mod module_writer; #[cfg(feature = "scaffolding")] mod new_project; +mod package_name_validations; mod project_layout; pub mod pyproject_toml; mod python_interpreter; diff --git a/src/new_project.rs b/src/new_project.rs index 9befebd2a..4ec938246 100644 --- a/src/new_project.rs +++ b/src/new_project.rs @@ -1,4 +1,5 @@ use crate::ci::GenerateCI; +use crate::package_name_validations::cargo_check_name; use crate::BridgeModel; use anyhow::{bail, Context, Result}; use console::style; @@ -146,11 +147,19 @@ impl<'a> ProjectGenerator<'a> { } } +fn validate_name(name: &str) -> anyhow::Result { + cargo_check_name(name)?; + + Ok(name.to_string()) +} /// Options common to `maturin new` and `maturin init`. #[derive(Debug, clap::Parser)] pub struct GenerateProjectOptions { /// Set the resulting package name, defaults to the directory name - #[arg(long)] + #[arg( + long, + value_parser=validate_name, + )] name: Option, /// Use mixed Rust/Python project layout #[arg(long)] @@ -212,10 +221,12 @@ fn generate_project( let file_name = project_path.file_name().with_context(|| { format!("Failed to get name from path '{}'", project_path.display()) })?; - file_name + let temp = file_name .to_str() .context("Filename isn't valid Unicode")? - .to_string() + .to_string(); + + validate_name(temp.as_str()).map_err(|e| anyhow::anyhow!(e))? }; let bindings_items = if options.mixed { vec!["pyo3", "rust-cpython", "cffi", "uniffi"] diff --git a/src/package_name_validations.rs b/src/package_name_validations.rs new file mode 100644 index 000000000..05db1f0e1 --- /dev/null +++ b/src/package_name_validations.rs @@ -0,0 +1,116 @@ +// Based on: https://github.com/rust-lang/cargo/blob/e975158c1b542aa3833fd8584746538c17a6ae55/src/cargo/ops/cargo_new.rs#L169 +pub fn cargo_check_name(name: &str) -> anyhow::Result<()> { + // Instead of `PackageName::new` which performs these checks in the original cargo code + validate_package_name(name)?; + + if is_keyword(name) { + anyhow::bail!( + "the name `{}` cannot be used as a package name, it is a Rust keyword", + name, + ); + } + if is_conflicting_artifact_name(name) { + anyhow::bail!( + "the name `{}` cannot be used as a package name, \ + it conflicts with cargo's build directory names", + name, + ); + } + if name == "test" { + anyhow::bail!( + "the name `test` cannot be used as a package name, \ + it conflicts with Rust's built-in test library", + ); + } + if ["core", "std", "alloc", "proc_macro", "proc-macro"].contains(&name) { + eprintln!( + "⚠️ Warning: the name `{}` is part of Rust's standard library\n\ + It is recommended to use a different name to avoid problems.", + name, + ); + } + if is_windows_reserved(name) { + eprintln!( + "⚠️ Warning: the name `{}` is a reserved Windows filename\n\ + This package will not work on Windows platforms.", + name + ); + } + if is_non_ascii_name(name) { + eprintln!( + "⚠️ Warning: the name `{}` contains non-ASCII characters\n\ + Non-ASCII crate names are not supported by Rust.", + name + ); + } + let name_in_lowercase = name.to_lowercase(); + if name != name_in_lowercase { + eprintln!( + "⚠️ Warning: the name `{name}` is not snake_case or kebab-case which is recommended for package names, consider `{name_in_lowercase}`" + ); + } + + Ok(()) +} + +// Based on: https://github.com/rust-lang/cargo/blob/7b7af3077bff8d60b7f124189bc9de227d3063a9/crates/cargo-util-schemas/src/restricted_names.rs#L42 +fn validate_package_name(name: &str) -> anyhow::Result<()> { + if name.is_empty() { + anyhow::bail!("Package names cannot be empty"); + } + + let mut chars = name.chars(); + if let Some(ch) = chars.next() { + if ch.is_ascii_digit() { + // A specific error for a potentially common case. + anyhow::bail!("Package names cannot start with a digit"); + } + if !(unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_') { + anyhow::bail!( + "the first character must be a Unicode XID start character (most letters or `_`)" + ); + } + } + for ch in chars { + if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-') { + anyhow::bail!( + "characters must be Unicode XID characters (numbers, `-`, `_`, or most letters)" + ); + } + } + Ok(()) +} + +// The following functions are based on https://github.com/rust-lang/cargo/blob/e975158c1b542aa3833fd8584746538c17a6ae55/src/cargo/util/restricted_names.rs + +/// 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) +}