From 5006491c45a902fa47da596f81d541d20d74300b Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Tue, 29 Oct 2024 08:54:38 +0100 Subject: [PATCH 01/12] Initial #107 work --- src/problem/mod.rs | 5 +++++ src/problem/npv_170.rs | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 src/problem/npv_170.rs diff --git a/src/problem/mod.rs b/src/problem/mod.rs index 981c0ea..0550903 100644 --- a/src/problem/mod.rs +++ b/src/problem/mod.rs @@ -34,6 +34,8 @@ pub mod npv_161; pub mod npv_162; pub mod npv_163; +pub mod npv_170; + #[derive(Clone, Display, EnumFrom)] pub enum Problem { /// NPV-100: attribute is not defined but it should be defined automatically @@ -123,6 +125,9 @@ pub enum Problem { NewTopLevelPackageShouldBeByNameWithCustomArgument( npv_163::NewTopLevelPackageShouldBeByNameWithCustomArgument, ), + + /// NPV-170: by-name package cannot be number prefixed + ByNamePackegPrefixedWithNumber(npv_170::ByNamePackegPrefixedWithNumber), } fn indent_definition(column: usize, definition: &str) -> String { diff --git a/src/problem/npv_170.rs b/src/problem/npv_170.rs new file mode 100644 index 0000000..61d4dcb --- /dev/null +++ b/src/problem/npv_170.rs @@ -0,0 +1,19 @@ +use std::fmt; + +use derive_new::new; + +#[derive(Clone, new)] +pub struct ByNamePackegPrefixedWithNumber { + #[new(into)] + attribute_name: String, +} + +impl fmt::Display for ByNamePackegPrefixedWithNumber { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let Self { attribute_name } = self; + write!( + f, + r#"- pkgs.{attribute_name}: "Attribute names should not be number-prefixed. It is suggestet to `"`-wrap this name"# + ) + } +} From 85a1b53741cbc7c3663457891cb79033cca39053 Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Tue, 29 Oct 2024 09:20:58 +0100 Subject: [PATCH 02/12] Update shard/package name sanatisation regex --- src/structure.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/structure.rs b/src/structure.rs index 93b5975..4af427f 100644 --- a/src/structure.rs +++ b/src/structure.rs @@ -16,8 +16,8 @@ pub const BASE_SUBPATH: &str = "pkgs/by-name"; pub const PACKAGE_NIX_FILENAME: &str = "package.nix"; lazy_static! { - static ref SHARD_NAME_REGEX: Regex = Regex::new(r"^[a-z0-9_-]{1,2}$").unwrap(); - static ref PACKAGE_NAME_REGEX: Regex = Regex::new(r"^[a-zA-Z0-9_-]+$").unwrap(); + static ref SHARD_NAME_REGEX: Regex = Regex::new(r"^((_[0-9])|([a-z][a-z0-9_-]?))$").unwrap(); + static ref PACKAGE_NAME_REGEX: Regex = Regex::new(r"^((_[0-9])|[a-zA-Z])[a-zA-Z0-9_-]*$").unwrap(); } /// Deterministic file listing so that tests are reproducible. From af703c93c210abcee1b9a7da4b327378b40298f9 Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Tue, 29 Oct 2024 09:21:08 +0100 Subject: [PATCH 03/12] Add test --- tests/by-name-numprefix/default.nix | 1 + tests/by-name-numprefix/expected | 4 ++++ tests/by-name-numprefix/pkgs/by-name/10/10foo/package.nix | 3 +++ 3 files changed, 8 insertions(+) create mode 100644 tests/by-name-numprefix/default.nix create mode 100644 tests/by-name-numprefix/expected create mode 100644 tests/by-name-numprefix/pkgs/by-name/10/10foo/package.nix diff --git a/tests/by-name-numprefix/default.nix b/tests/by-name-numprefix/default.nix new file mode 100644 index 0000000..861260c --- /dev/null +++ b/tests/by-name-numprefix/default.nix @@ -0,0 +1 @@ +import { root = ./.; } diff --git a/tests/by-name-numprefix/expected b/tests/by-name-numprefix/expected new file mode 100644 index 0000000..e76e502 --- /dev/null +++ b/tests/by-name-numprefix/expected @@ -0,0 +1,4 @@ +trace: This should be on stderr! +@REDACTED@error: This is an error!@REDACTED@ +- Nix evaluation failed for some package in `pkgs/by-name`, see error above +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/by-name-numprefix/pkgs/by-name/10/10foo/package.nix b/tests/by-name-numprefix/pkgs/by-name/10/10foo/package.nix new file mode 100644 index 0000000..51081b0 --- /dev/null +++ b/tests/by-name-numprefix/pkgs/by-name/10/10foo/package.nix @@ -0,0 +1,3 @@ +{ }: +# If we caused an actual Nix failure +builtins.trace "This should be on stderr!" throw "This is an error!" From fd4a72fb1fa5974ec8f9e3b80709e12563057ebb Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Tue, 29 Oct 2024 09:23:13 +0100 Subject: [PATCH 04/12] cargo fmt --- src/structure.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/structure.rs b/src/structure.rs index 4af427f..9089597 100644 --- a/src/structure.rs +++ b/src/structure.rs @@ -17,7 +17,8 @@ pub const PACKAGE_NIX_FILENAME: &str = "package.nix"; lazy_static! { static ref SHARD_NAME_REGEX: Regex = Regex::new(r"^((_[0-9])|([a-z][a-z0-9_-]?))$").unwrap(); - static ref PACKAGE_NAME_REGEX: Regex = Regex::new(r"^((_[0-9])|[a-zA-Z])[a-zA-Z0-9_-]*$").unwrap(); + static ref PACKAGE_NAME_REGEX: Regex = + Regex::new(r"^((_[0-9])|[a-zA-Z])[a-zA-Z0-9_-]*$").unwrap(); } /// Deterministic file listing so that tests are reproducible. From e693e74ca442e97846e7882106ae0520f1689513 Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Tue, 29 Oct 2024 09:53:17 +0100 Subject: [PATCH 05/12] Rename dir: 10 -> _1 --- tests/by-name-numprefix/pkgs/by-name/{10 => _1}/10foo/package.nix | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/by-name-numprefix/pkgs/by-name/{10 => _1}/10foo/package.nix (100%) diff --git a/tests/by-name-numprefix/pkgs/by-name/10/10foo/package.nix b/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix similarity index 100% rename from tests/by-name-numprefix/pkgs/by-name/10/10foo/package.nix rename to tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix From 884c807876b0b12186a5c0beeb0603217db08a6a Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Tue, 29 Oct 2024 09:53:29 +0100 Subject: [PATCH 06/12] add leading digit check --- src/structure.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/structure.rs b/src/structure.rs index 9089597..c7996f1 100644 --- a/src/structure.rs +++ b/src/structure.rs @@ -7,7 +7,9 @@ use lazy_static::lazy_static; use regex::Regex; use relative_path::RelativePathBuf; -use crate::problem::{npv_109, npv_110, npv_111, npv_140, npv_141, npv_142, npv_143, npv_144}; +use crate::problem::{ + npv_109, npv_110, npv_111, npv_140, npv_141, npv_142, npv_143, npv_144, npv_170, +}; use crate::references; use crate::validation::{self, ResultIteratorExt, Validation::Success}; use crate::NixFileStore; @@ -138,11 +140,18 @@ fn check_package( } else { let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); let result = if !package_name_valid { - npv_141::InvalidPackageDirectoryName::new( - package_name.clone(), - relative_package_dir.clone(), - ) - .into() + if package_name + .chars() + .next() + .is_some_and(|c| c.is_ascii_digit()) + { + npv_170::ByNamePackegPrefixedWithNumber::new(package_name.clone()).into() + } else { + npv_141::InvalidPackageDirectoryName::new( + package_name.clone(), + relative_package_dir.clone(), + ).into() + } } else { Success(()) }; From 05bafef5a630ce3b9b4c536d7ec80dbbc2832ec0 Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Tue, 29 Oct 2024 10:12:55 +0100 Subject: [PATCH 07/12] Cleanup err-msg and match(ish) it with expected --- src/problem/npv_170.rs | 12 +++++++++--- src/structure.rs | 9 +++++++-- tests/by-name-numprefix/expected | 2 +- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/problem/npv_170.rs b/src/problem/npv_170.rs index 61d4dcb..6614951 100644 --- a/src/problem/npv_170.rs +++ b/src/problem/npv_170.rs @@ -1,19 +1,25 @@ use std::fmt; use derive_new::new; +use relative_path::RelativePathBuf; #[derive(Clone, new)] pub struct ByNamePackegPrefixedWithNumber { #[new(into)] - attribute_name: String, + package_name: String, + #[new(into)] + relative_package_dir: RelativePathBuf, } impl fmt::Display for ByNamePackegPrefixedWithNumber { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let Self { attribute_name } = self; + let Self { + package_name, + relative_package_dir, + } = self; write!( f, - r#"- pkgs.{attribute_name}: "Attribute names should not be number-prefixed. It is suggestet to `"`-wrap this name"# + r#"- {relative_package_dir}: Attribute `{package_name}` should not be number-prefixed. It is suggestet to `"`-wrap this name"# ) } } diff --git a/src/structure.rs b/src/structure.rs index c7996f1..5e19da8 100644 --- a/src/structure.rs +++ b/src/structure.rs @@ -145,12 +145,17 @@ fn check_package( .next() .is_some_and(|c| c.is_ascii_digit()) { - npv_170::ByNamePackegPrefixedWithNumber::new(package_name.clone()).into() + npv_170::ByNamePackegPrefixedWithNumber::new( + package_name.clone(), + relative_package_dir.clone(), + ) + .into() } else { npv_141::InvalidPackageDirectoryName::new( package_name.clone(), relative_package_dir.clone(), - ).into() + ) + .into() } } else { Success(()) diff --git a/tests/by-name-numprefix/expected b/tests/by-name-numprefix/expected index e76e502..1e4daf6 100644 --- a/tests/by-name-numprefix/expected +++ b/tests/by-name-numprefix/expected @@ -1,4 +1,4 @@ trace: This should be on stderr! @REDACTED@error: This is an error!@REDACTED@ -- Nix evaluation failed for some package in `pkgs/by-name`, see error above +- pkgs/by-name/_1/10foo: Attribute `10foo` should not be number-prefixed. It is suggestet to `"`-wrap this name This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. From c20b06e6046fc82040d68eba5834139bb45dfb6c Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Tue, 29 Oct 2024 10:36:17 +0100 Subject: [PATCH 08/12] Remodel test based on invalid name test --- tests/by-name-numprefix/expected | 2 -- tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/by-name-numprefix/expected b/tests/by-name-numprefix/expected index 1e4daf6..5df5142 100644 --- a/tests/by-name-numprefix/expected +++ b/tests/by-name-numprefix/expected @@ -1,4 +1,2 @@ -trace: This should be on stderr! -@REDACTED@error: This is an error!@REDACTED@ - pkgs/by-name/_1/10foo: Attribute `10foo` should not be number-prefixed. It is suggestet to `"`-wrap this name This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix b/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix index 51081b0..c431c8b 100644 --- a/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix +++ b/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix @@ -1,3 +1,3 @@ -{ }: +{ someDrv }: SomeDrv # If we caused an actual Nix failure -builtins.trace "This should be on stderr!" throw "This is an error!" +# builtins.trace "This should be on stderr!" throw "This is an error!" From 7f3e6ce7b32c6aa1635059863fb31e702e61c229 Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Tue, 29 Oct 2024 10:38:45 +0100 Subject: [PATCH 09/12] treefmt --- tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix b/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix index c431c8b..9fb8b5d 100644 --- a/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix +++ b/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix @@ -1,3 +1,4 @@ -{ someDrv }: SomeDrv +{ someDrv }: +SomeDrv # If we caused an actual Nix failure # builtins.trace "This should be on stderr!" throw "This is an error!" From 932f6ef2aa82a7d9179340d5effeffbb6bcc2dbf Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Wed, 30 Oct 2024 09:24:07 +0100 Subject: [PATCH 10/12] Review followup --- src/problem/npv_170.rs | 2 +- src/structure.rs | 6 +----- tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix | 5 +---- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/problem/npv_170.rs b/src/problem/npv_170.rs index 6614951..c540051 100644 --- a/src/problem/npv_170.rs +++ b/src/problem/npv_170.rs @@ -19,7 +19,7 @@ impl fmt::Display for ByNamePackegPrefixedWithNumber { } = self; write!( f, - r#"- {relative_package_dir}: Attribute `{package_name}` should not be number-prefixed. It is suggestet to `"`-wrap this name"# + "- {relative_package_dir}: Attribute `{package_name}` should not be number-prefixed. Prefix with `_`, or wrap in quotes" ) } } diff --git a/src/structure.rs b/src/structure.rs index 5e19da8..966326d 100644 --- a/src/structure.rs +++ b/src/structure.rs @@ -140,11 +140,7 @@ fn check_package( } else { let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); let result = if !package_name_valid { - if package_name - .chars() - .next() - .is_some_and(|c| c.is_ascii_digit()) - { + if package_name.starts_with(|c: char| c.is_ascii_digit()) { npv_170::ByNamePackegPrefixedWithNumber::new( package_name.clone(), relative_package_dir.clone(), diff --git a/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix b/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix index 9fb8b5d..ce29e51 100644 --- a/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix +++ b/tests/by-name-numprefix/pkgs/by-name/_1/10foo/package.nix @@ -1,4 +1 @@ -{ someDrv }: -SomeDrv -# If we caused an actual Nix failure -# builtins.trace "This should be on stderr!" throw "This is an error!" +{ someDrv }: SomeDrv From 837cba723d3f6d7b467898d8d6f06cb015feb01a Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Wed, 30 Oct 2024 10:17:34 +0100 Subject: [PATCH 11/12] Fix CI error --- tests/by-name-numprefix/expected | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-name-numprefix/expected b/tests/by-name-numprefix/expected index 5df5142..38342a6 100644 --- a/tests/by-name-numprefix/expected +++ b/tests/by-name-numprefix/expected @@ -1,2 +1,2 @@ -- pkgs/by-name/_1/10foo: Attribute `10foo` should not be number-prefixed. It is suggestet to `"`-wrap this name +- pkgs/by-name/_1/10foo: Attribute `10foo` should not be number-prefixed. Prefix with `_`, or wrap in quotes This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. From 2e544898980a7d3acc20107228d60733cd30b7b1 Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Wed, 30 Oct 2024 10:21:22 +0100 Subject: [PATCH 12/12] Update README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 3e1e9fd..807b457 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,7 @@ The following checks are performed when calling the binary: - `pkgs/by-name` must only contain subdirectories of the form `${shard}/${name}`, called _package directories_. - The `name`'s of package directories must be unique when lowercased. - `name` is a string only consisting of the ASCII characters `a-z`, `A-Z`, `0-9`, `-` or `_`. +- `name` only starts with an ASCII character `a-z`, `A-Z`, `-` or `_`. - `shard` is the lowercased first two letters of `name`, expressed in Nix: `shard = toLower (substring 0 2 name)`. - Each package directory must contain a `package.nix` file and may contain arbitrary other files.