From b116864d7963a19c3dcea1131a13a966a0c1e839 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 13 Jun 2023 09:29:34 -0500 Subject: [PATCH 1/5] fix(embedded): Be consistent with existing style when sanitizing --- src/cargo/util/toml/embedded.rs | 16 ++++++++++++++-- tests/testsuite/script.rs | 4 ++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index c911161dd60..5be08bee2f6 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -79,7 +79,13 @@ fn write( .file_stem() .ok_or_else(|| anyhow::format_err!("no file name"))? .to_string_lossy(); - let separator = '_'; + let separator = if file_name.contains('_') { + '_' + } else { + // Since embedded manifests only support `[[bin]]`s, prefer arrow-case as that is the + // more common convention for CLIs + '-' + }; let name = sanitize_package_name(file_name.as_ref(), separator); let mut workspace_root = target_dir.to_owned(); @@ -140,7 +146,13 @@ fn expand_manifest_(script: &RawScript, config: &Config) -> CargoResult Date: Tue, 13 Jun 2023 10:54:07 -0500 Subject: [PATCH 2/5] refactor(embedded): Centralize package name rules --- src/cargo/util/restricted_names.rs | 24 ++++++++++++++++++++++++ src/cargo/util/toml/embedded.rs | 22 ++-------------------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/cargo/util/restricted_names.rs b/src/cargo/util/restricted_names.rs index 650ae233059..6a666db99e1 100644 --- a/src/cargo/util/restricted_names.rs +++ b/src/cargo/util/restricted_names.rs @@ -83,6 +83,30 @@ pub fn validate_package_name(name: &str, what: &str, help: &str) -> CargoResult< Ok(()) } +/// Ensure a package name is [valid][validate_package_name] +pub fn sanitize_package_name(name: &str, placeholder: char) -> String { + let mut slug = String::new(); + for (i, c) in name.chars().enumerate() { + match (i, c) { + (0, '0'..='9') => { + slug.push(placeholder); + slug.push(c); + } + (_, '0'..='9') | (_, 'a'..='z') | (_, '_') | (_, '-') => { + slug.push(c); + } + (_, 'A'..='Z') => { + // Convert uppercase characters to lowercase to avoid `non_snake_case` warnings. + slug.push(c.to_ascii_lowercase()); + } + (_, _) => { + slug.push(placeholder); + } + } + } + slug +} + /// Check the entire path for names reserved in Windows. pub fn is_windows_reserved_path(path: &Path) -> bool { path.iter() diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 5be08bee2f6..62da341f684 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -1,6 +1,7 @@ use anyhow::Context as _; use crate::core::Workspace; +use crate::util::restricted_names; use crate::CargoResult; use crate::Config; @@ -206,26 +207,7 @@ fn expand_manifest_(script: &RawScript, config: &Config) -> CargoResult String { - let mut slug = String::new(); - for (i, c) in name.chars().enumerate() { - match (i, c) { - (0, '0'..='9') => { - slug.push(placeholder); - slug.push(c); - } - (_, '0'..='9') | (_, 'a'..='z') | (_, '_') | (_, '-') => { - slug.push(c); - } - (_, 'A'..='Z') => { - // Convert uppercase characters to lowercase to avoid `non_snake_case` warnings. - slug.push(c.to_ascii_lowercase()); - } - (_, _) => { - slug.push(placeholder); - } - } - } - slug + restricted_names::sanitize_package_name(name, placeholder) } fn hash(script: &RawScript) -> blake3::Hash { From 8186282f28e7888fd142949d16a42dba0e934c48 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 12 Jun 2023 14:31:44 -0500 Subject: [PATCH 3/5] fix(embedded): Sanitize like we validate package names --- src/cargo/util/restricted_names.rs | 32 +++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/cargo/util/restricted_names.rs b/src/cargo/util/restricted_names.rs index 6a666db99e1..be1811a8878 100644 --- a/src/cargo/util/restricted_names.rs +++ b/src/cargo/util/restricted_names.rs @@ -86,22 +86,22 @@ pub fn validate_package_name(name: &str, what: &str, help: &str) -> CargoResult< /// Ensure a package name is [valid][validate_package_name] pub fn sanitize_package_name(name: &str, placeholder: char) -> String { let mut slug = String::new(); - for (i, c) in name.chars().enumerate() { - match (i, c) { - (0, '0'..='9') => { - slug.push(placeholder); - slug.push(c); - } - (_, '0'..='9') | (_, 'a'..='z') | (_, '_') | (_, '-') => { - slug.push(c); - } - (_, 'A'..='Z') => { - // Convert uppercase characters to lowercase to avoid `non_snake_case` warnings. - slug.push(c.to_ascii_lowercase()); - } - (_, _) => { - slug.push(placeholder); - } + let mut chars = name.chars(); + if let Some(ch) = chars.next() { + if ch.is_digit(10) { + slug.push(placeholder); + slug.push(ch); + } else if unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_' { + slug.push(ch); + } else { + slug.push(placeholder); + } + } + for ch in chars { + if unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-' { + slug.push(ch); + } else { + slug.push(placeholder); } } slug From f4abdbe5f5df70590b879319789a21136ec8b95c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 13 Jun 2023 11:37:07 -0500 Subject: [PATCH 4/5] fix(embedded): Extend sanitization rules to cover cargo-new validation --- src/cargo/ops/cargo_new.rs | 1 + src/cargo/util/toml/embedded.rs | 35 +++++++++++++++++++++++++-------- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs index ca1339afa17..b113671b078 100644 --- a/src/cargo/ops/cargo_new.rs +++ b/src/cargo/ops/cargo_new.rs @@ -163,6 +163,7 @@ fn get_name<'a>(path: &'a Path, opts: &'a NewOptions) -> CargoResult<&'a str> { }) } +/// See also `util::toml::embedded::sanitize_name` fn check_name( name: &str, show_name_help: bool, diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 62da341f684..474a981cc27 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -87,7 +87,7 @@ fn write( // more common convention for CLIs '-' }; - let name = sanitize_package_name(file_name.as_ref(), separator); + let name = sanitize_name(file_name.as_ref(), separator); let mut workspace_root = target_dir.to_owned(); workspace_root.push("eval"); @@ -154,7 +154,7 @@ fn expand_manifest_(script: &RawScript, config: &Config) -> CargoResult CargoResult String { - restricted_names::sanitize_package_name(name, placeholder) +/// Ensure the package name matches the validation from `ops::cargo_new::check_name` +fn sanitize_name(name: &str, placeholder: char) -> String { + let mut name = restricted_names::sanitize_package_name(name, placeholder); + + loop { + if restricted_names::is_keyword(&name) { + name.push(placeholder); + } else if restricted_names::is_conflicting_artifact_name(&name) { + // Being an embedded manifest, we always assume it is a `[[bin]]` + name.push(placeholder); + } else if name == "test" { + name.push(placeholder); + } else if restricted_names::is_windows_reserved(&name) { + // Go ahead and be consistent across platforms + name.push(placeholder); + } else { + break; + } + } + + name } fn hash(script: &RawScript) -> blake3::Hash { @@ -442,12 +461,12 @@ mod test_expand { fn test_default() { snapbox::assert_eq( r#"[[bin]] -name = "test" +name = "test-" path = "/home/me/test.rs" [package] edition = "2021" -name = "test" +name = "test-" publish = false version = "0.0.0" @@ -464,7 +483,7 @@ strip = true fn test_dependencies() { snapbox::assert_eq( r#"[[bin]] -name = "test" +name = "test-" path = "/home/me/test.rs" [dependencies] @@ -472,7 +491,7 @@ time = "0.1.25" [package] edition = "2021" -name = "test" +name = "test-" publish = false version = "0.0.0" From 667ff78d69256fa1cbaef619a60a46457fe25177 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 16 Jun 2023 19:47:59 -0500 Subject: [PATCH 5/5] refactor(embedded): Centralize placeholder This was originally split out because before #12269, it was needed elsewhere. --- src/cargo/util/toml/embedded.rs | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 474a981cc27..2200c53f5da 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -80,14 +80,7 @@ fn write( .file_stem() .ok_or_else(|| anyhow::format_err!("no file name"))? .to_string_lossy(); - let separator = if file_name.contains('_') { - '_' - } else { - // Since embedded manifests only support `[[bin]]`s, prefer arrow-case as that is the - // more common convention for CLIs - '-' - }; - let name = sanitize_name(file_name.as_ref(), separator); + let name = sanitize_name(file_name.as_ref()); let mut workspace_root = target_dir.to_owned(); workspace_root.push("eval"); @@ -147,14 +140,7 @@ fn expand_manifest_(script: &RawScript, config: &Config) -> CargoResult CargoResult String { +fn sanitize_name(name: &str) -> String { + let placeholder = if name.contains('_') { + '_' + } else { + // Since embedded manifests only support `[[bin]]`s, prefer arrow-case as that is the + // more common convention for CLIs + '-' + }; + let mut name = restricted_names::sanitize_package_name(name, placeholder); loop {