Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validation for crate/package name in new/init #1943

Merged
merged 5 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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.3.6", features = ["serde"] }
pep508_rs = { version = "0.2.1", features = ["serde"] }
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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code bailed if the platform is actually windows, I think this should be good enough for now

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_digit(10) {
// 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)
}