Skip to content

Commit

Permalink
Add validation for crate/package name in new/init (#1943)
Browse files Browse the repository at this point in the history
* Start adding validation for crate/package name

* Add cargo validations

* Solve todos

* Fix clippy
  • Loading branch information
alonme authored Feb 27, 2024
1 parent f2a7b9d commit dd81231
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 3 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 14 additions & 3 deletions src/new_project.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -146,11 +147,19 @@ impl<'a> ProjectGenerator<'a> {
}
}

fn validate_name(name: &str) -> anyhow::Result<String> {
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<String>,
/// Use mixed Rust/Python project layout
#[arg(long)]
Expand Down Expand Up @@ -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"]
Expand Down
116 changes: 116 additions & 0 deletions src/package_name_validations.rs
Original file line number Diff line number Diff line change
@@ -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)
}

0 comments on commit dd81231

Please sign in to comment.