vhaudiquet/pkh
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

new: type the scaffold validator errors

Browse Source 
The leaf validators of the wizard (source name, command, upstream
version, maintainer, homepage, Depends, orig path, template id, file
collisions) now return a shared options::ValidationError carrying the
structured pieces (the rejected string, the wrapped DebianVersion or
Deps parse failure) instead of pre-formatted Strings; the wizard
resolve flow keeps its aggregate String errors and renders at the
boundary, and the Prompter validator closures adapt with to_string()
since the port's Validator deliberately exchanges re-ask explanations
as plain strings. Display texts are unchanged, as are the tests that
assert on them.
This commit is contained in:
vhaudiquet
2026-10-03 02:17:22 +02:00
parent 82a15c7426
commit a59e99b107
2 changed files with 193 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/new/options.rs
+167 -69
View File
@@ -63,22 +63,12 @@ impl TemplateId {
}
/// Parse a CLI identifier, accepting exactly the registered spellings.
pub fn parse(s: &str) -> Result<TemplateId, String> {
pub fn parse(s: &str) -> Result<TemplateId, ValidationError> {
Self::all()
.iter()
.copied()
.find(|id| id.as_str() == s)
.ok_or_else(|| {
format!(
"Unknown language/template '{}'. Supported values are: {}.",
s,
Self::all()
.iter()
.map(|id| id.as_str())
.collect::<Vec<_>>()
.join(", ")
)
})
.ok_or_else(|| ValidationError::UnknownTemplate { id: s.to_string() })
}
/// Human-readable menu label of this template, as offered by the wizard
@@ -407,21 +397,132 @@ pub struct NewCli {
pub defaults: bool,
}
/// Failure of one of the scaffold option validators ([`validate_source_name`],
/// [`validate_command`], [`validate_upstream_version`], [`parse_maintainer`],
/// [`validate_homepage`], [`validate_depends`], [`validate_orig_path`],
/// [`check_file_collisions`], [`TemplateId::parse`]): what the wizard re-asks
/// about, carrying the pieces callers may want to match on.
#[derive(Debug, thiserror::Error)]
pub enum ValidationError {
/// The source package name is shorter than dpkg's 2-character minimum.
#[error("'{name}' is too short: package names need at least 2 characters")]
SourceNameTooShort {
/// The rejected name.
name: String,
},
/// The source package name is outside dpkg's charset or shape.
#[error(
"'{}' is not a valid package name: names must be lowercase, start with a letter or digit and may only contain [a-z0-9+.-]",
name
)]
InvalidSourceName {
/// The rejected name.
name: String,
},
/// No installed command was given.
#[error("the command name must not be empty")]
EmptyCommand,
/// The command name is not a lowercase identifier.
#[error(
"'{}' is not a valid command name: commands must be a lowercase identifier (letters, digits, + - . _), e.g. 'mytool'",
command
)]
InvalidCommand {
/// The rejected command name.
command: String,
},
/// Upstream versions should start with a digit (dpkg recommendation,
/// enforced here).
#[error("upstream versions should start with a digit")]
UpstreamNotDigit,
/// `-` is the Debian revision separator.
#[error(
"upstream versions must not contain '-': the Debian revision is appended automatically"
)]
UpstreamContainsDash,
/// The upstream version composed with the Debian revision does not
/// parse as a Debian version.
#[error("'{version}' is not a valid Debian version: {source}")]
InvalidVersion {
/// The composed `upstream-revision` string.
version: String,
/// The version parse failure.
source: crate::debian::version::VersionParseError,
},
/// The maintainer is not of the form `Name <email>`.
#[error("'{s}' is not a valid maintainer: expected the form 'Name <email>'")]
MaintainerShape {
/// The rejected maintainer string.
s: String,
},
/// The maintainer name (before the angle brackets) is empty.
#[error("'{s}': the maintainer name must not be empty")]
MaintainerNameEmpty {
/// The rejected maintainer string.
s: String,
},
/// The maintainer email does not carry exactly one `@`.
#[error("'{s}': the maintainer email must contain exactly one '@'")]
MaintainerEmailAt {
/// The rejected maintainer string.
s: String,
},
/// The maintainer email carries whitespace.
#[error("'{s}': the maintainer email must not contain whitespace")]
MaintainerEmailWhitespace {
/// The rejected maintainer string.
s: String,
},
/// The homepage is not an http(s) URL.
#[error("'{url}' is not a valid homepage: expected an http:// or https:// URL")]
InvalidHomepage {
/// The rejected URL.
url: String,
},
/// The Depends list does not parse as package relations.
#[error("Invalid Depends list '{list}': {source}")]
InvalidDepends {
/// The rejected list.
list: String,
/// The relation parse failure.
source: crate::debian::deps::ParseError,
},
/// The orig tarball is neither an http(s) URL nor an existing file.
#[error(
"'{path}' is not an http(s) URL and not an existing file: --orig-path must point at a tarball to repack"
)]
InvalidOrigPath {
/// The rejected path or URL.
path: String,
},
/// The language/template id is not one of the registered spellings.
#[error("Unknown language/template '{}'. Supported values are: {}.", id, TemplateId::all().iter().map(|id| id.as_str()).collect::<Vec<_>>().join(", "))]
UnknownTemplate {
/// The unknown id.
id: String,
},
/// A template generated the same file twice.
#[error("internal error: file '{path}' was generated more than once")]
GeneratedTwice {
/// The duplicated path.
path: String,
},
}
/// Validate a Debian source package name: `^[a-z0-9][a-z0-9+.\-]+$` with a
/// minimum length of 2 (dpkg rules).
pub fn validate_source_name(name: &str) -> Result<(), String> {
pub fn validate_source_name(name: &str) -> Result<(), ValidationError> {
static NAME_REGEX: std::sync::OnceLock<Regex> = std::sync::OnceLock::new();
let regex = NAME_REGEX.get_or_init(|| Regex::new(r"^[a-z0-9][a-z0-9+.\-]+$").unwrap());
if name.len() < 2 {
return Err(format!(
"'{name}' is too short: package names need at least 2 characters"
));
return Err(ValidationError::SourceNameTooShort {
name: name.to_string(),
});
}
if !regex.is_match(name) {
return Err(format!(
"'{name}' is not a valid package name: names must be lowercase, \
start with a letter or digit and may only contain [a-z0-9+.-]"
));
return Err(ValidationError::InvalidSourceName {
name: name.to_string(),
});
}
Ok(())
}
@@ -473,17 +574,16 @@ pub fn sanitize_name(input: &str) -> Option<String> {
/// `meson.build` and the `[project.scripts]` table, so uppercase letters,
/// whitespace, quotes and shell metacharacters are rejected here instead of
/// generating broken install lines and build files.
pub fn validate_command(command: &str) -> Result<(), String> {
pub fn validate_command(command: &str) -> Result<(), ValidationError> {
static COMMAND_REGEX: std::sync::OnceLock<Regex> = std::sync::OnceLock::new();
let regex = COMMAND_REGEX.get_or_init(|| Regex::new(r"^[a-z0-9][a-z0-9+.\-_]*$").unwrap());
if command.is_empty() {
return Err("the command name must not be empty".to_string());
return Err(ValidationError::EmptyCommand);
}
if !regex.is_match(command) {
return Err(format!(
"'{command}' is not a valid command name: commands must be a \
lowercase identifier (letters, digits, + - . _), e.g. 'mytool'"
));
return Err(ValidationError::InvalidCommand {
command: command.to_string(),
});
}
Ok(())
}
@@ -492,65 +592,60 @@ pub fn validate_command(command: &str) -> Result<(), String> {
/// recommendation, enforced here) and survive [`DebianVersion::parse`] once
/// composed with the Debian revision. It must not contain `-` (the revision
/// separator).
pub fn validate_upstream_version(upstream: &str, revision: u32) -> Result<(), String> {
pub fn validate_upstream_version(upstream: &str, revision: u32) -> Result<(), ValidationError> {
if !upstream.starts_with(|c: char| c.is_ascii_digit()) {
return Err("upstream versions should start with a digit".to_string());
return Err(ValidationError::UpstreamNotDigit);
}
if upstream.contains('-') {
return Err(
"upstream versions must not contain '-': the Debian revision is \
appended automatically"
.to_string(),
);
return Err(ValidationError::UpstreamContainsDash);
}
let composed = format!("{}-{}", upstream, revision);
DebianVersion::parse(&composed)
.map(|_| ())
.map_err(|e| format!("'{composed}' is not a valid Debian version: {e}"))
.map_err(|source| ValidationError::InvalidVersion {
version: composed.clone(),
source,
})
}
/// Parse a `Name <email>` maintainer string: the name must be non-empty and
/// the email must contain exactly one `@` with no whitespace inside the
/// angle brackets.
pub fn parse_maintainer(s: &str) -> Result<(String, String), String> {
pub fn parse_maintainer(s: &str) -> Result<(String, String), ValidationError> {
static MAINTAINER_REGEX: std::sync::OnceLock<Regex> = std::sync::OnceLock::new();
let regex = MAINTAINER_REGEX.get_or_init(|| Regex::new(r"^(.+?)\s*<([^<>]*)>\s*$").unwrap());
let caps = regex.captures(s.trim()).ok_or_else(|| {
format!("'{s}' is not a valid maintainer: expected the form 'Name <email>'")
})?;
let caps = regex
.captures(s.trim())
.ok_or_else(|| ValidationError::MaintainerShape { s: s.to_string() })?;
let name = caps[1].trim().to_string();
let email = caps[2].trim().to_string();
if name.is_empty() {
return Err(format!("'{s}': the maintainer name must not be empty"));
return Err(ValidationError::MaintainerNameEmpty { s: s.to_string() });
}
if !email.contains('@') || email.matches('@').count() > 1 {
return Err(format!(
"'{s}': the maintainer email must contain exactly one '@'"
));
return Err(ValidationError::MaintainerEmailAt { s: s.to_string() });
}
if email.chars().any(char::is_whitespace) {
return Err(format!(
"'{s}': the maintainer email must not contain whitespace"
));
return Err(ValidationError::MaintainerEmailWhitespace { s: s.to_string() });
}
Ok((name, email))
}
/// Validate a homepage URL: it must carry an `http(s)://` prefix.
pub fn validate_homepage(url: &str) -> Result<(), String> {
pub fn validate_homepage(url: &str) -> Result<(), ValidationError> {
if url.starts_with("http://") || url.starts_with("https://") {
Ok(())
} else {
Err(format!(
"'{url}' is not a valid homepage: expected an http:// or https:// URL"
))
Err(ValidationError::InvalidHomepage {
url: url.to_string(),
})
}
}
/// Validate a comma-separated runtime Depends list (the metapackage flavor)
/// by parsing it with [`Deps::parse`] and re-rendering every clause
/// canonically.
pub fn validate_depends(list: &str) -> Result<Vec<String>, String> {
pub fn validate_depends(list: &str) -> Result<Vec<String>, ValidationError> {
let opts = ParseOpts {
host_arch: crate::get_current_arch(),
build_arch: crate::get_current_arch(),
@@ -559,8 +654,10 @@ pub fn validate_depends(list: &str) -> Result<Vec<String>, String> {
union: false,
build_dep: false,
};
let deps =
Deps::parse(list, &opts).map_err(|e| format!("Invalid Depends list '{list}': {e}"))?;
let deps = Deps::parse(list, &opts).map_err(|source| ValidationError::InvalidDepends {
list: list.to_string(),
source,
})?;
Ok(deps
.clauses()
.map(|clause| {
@@ -620,7 +717,7 @@ pub async fn resolve(cli: NewCli) -> Result<NewOptions, String> {
// (nothing to detect for a fresh skeleton), else --defaults picks the
// empty template.
let template = match &cli.lang {
Some(lang) => Some(TemplateId::parse(lang)?),
Some(lang) => Some(TemplateId::parse(lang).map_err(|e| e.to_string())?),
None => match &source_dir {
SourceDir::Skeleton => {
if cli.defaults {
@@ -706,7 +803,7 @@ pub async fn resolve(cli: NewCli) -> Result<NewOptions, String> {
}
// Everything below has a default and is validated as it is resolved.
validate_source_name(&name)?;
validate_source_name(&name).map_err(|e| e.to_string())?;
// Source format: explicit flags win (--native / --quilt, mutually
// exclusive), otherwise the mode decides — a fresh, self-authored
@@ -769,7 +866,7 @@ pub async fn resolve(cli: NewCli) -> Result<NewOptions, String> {
let homepage = match &cli.homepage {
Some(h) => {
validate_homepage(h)?;
validate_homepage(h).map_err(|e| e.to_string())?;
Some(h.clone())
}
None => None,
@@ -780,10 +877,10 @@ pub async fn resolve(cli: NewCli) -> Result<NewOptions, String> {
let command = cli.command.unwrap_or_else(|| name.clone());
// The default is the already-validated package name (a subset of the
// command charset), so only an explicit --command/wizard answer can fail.
validate_command(&command)?;
validate_command(&command).map_err(|e| e.to_string())?;
let maintainer = match &cli.maintainer {
Some(m) => parse_maintainer(m)?,
Some(m) => parse_maintainer(m).map_err(|e| e.to_string())?,
None => crate::changelog::get_maintainer_info().map_err(|e| {
format!(
"Could not determine the maintainer ({e}). \
@@ -815,7 +912,7 @@ pub async fn resolve(cli: NewCli) -> Result<NewOptions, String> {
let depends = if joined.trim().is_empty() {
Vec::new()
} else {
validate_depends(&joined)?
validate_depends(&joined).map_err(|e| e.to_string())?
};
Ok(NewOptions {
@@ -874,7 +971,7 @@ fn resolve_orig_origin(
let source = cli.orig_path.clone().ok_or_else(|| {
"--orig-from path requires --orig-path <tarball file or URL>".to_string()
})?;
validate_orig_path(&source)?;
validate_orig_path(&source).map_err(|e| e.to_string())?;
OrigOrigin::Provided { source }
}
Some("git") => {
@@ -935,27 +1032,26 @@ fn resolve_orig_origin(
/// Validate a `--orig-path` value (and the wizard's tarball answer): either
/// an http(s) URL (downloaded at scaffold time) or an existing local
/// tarball.
pub fn validate_orig_path(source: &str) -> Result<(), String> {
pub fn validate_orig_path(source: &str) -> Result<(), ValidationError> {
if source.starts_with("http://") || source.starts_with("https://") {
return Ok(());
}
if Path::new(source).is_file() {
return Ok(());
}
Err(format!(
"'{source}' is not an http(s) URL and not an existing file: \
--orig-path must point at a tarball to repack"
))
Err(ValidationError::InvalidOrigPath {
path: source.to_string(),
})
}
/// Set of names no template may produce twice (collision check).
pub(crate) fn check_file_collisions(paths: &[String]) -> Result<(), String> {
pub(crate) fn check_file_collisions(paths: &[String]) -> Result<(), ValidationError> {
let mut seen: HashSet<&String> = HashSet::new();
for path in paths {
if !seen.insert(path) {
return Err(format!(
"internal error: file '{path}' was generated more than once"
));
return Err(ValidationError::GeneratedTwice {
path: path.to_string(),
});
}
}
Ok(())
@@ -1222,7 +1318,9 @@ mod tests {
#[test]
fn collision_detection() {
assert!(check_file_collisions(&["a".into(), "b".into()]).is_ok());
let err = check_file_collisions(&["a".into(), "b".into(), "a".into()]).unwrap_err();
let err = check_file_collisions(&["a".into(), "b".into(), "a".into()])
.unwrap_err()
.to_string();
assert!(err.contains('a'));
}
src/new/questions.rs
+26 -19
View File
@@ -96,12 +96,9 @@ async fn run_wizard(
// basename of the current directory.
if cli.name.is_none() {
let default = default_package_name(&cwd, probe.as_ref());
let answer = ask_text(
prompter,
"Package name",
&default,
options::validate_source_name,
)?;
let answer = ask_text(prompter, "Package name", &default, |name: &str| {
options::validate_source_name(name).map_err(|e| e.to_string())
})?;
cli.name = Some(answer);
}
@@ -265,7 +262,9 @@ async fn run_wizard(
.or_else(|| origin.as_ref().and_then(GitOrigin::git_version))
.unwrap_or_else(|| "0.1.0".to_string());
let revision = cli.revision.unwrap_or(1);
let validate = move |version: &str| options::validate_upstream_version(version, revision);
let validate = move |version: &str| {
options::validate_upstream_version(version, revision).map_err(|e| e.to_string())
};
let answer = ask_text(prompter, "Upstream version", &default, validate)?;
cli.upstream_version = Some(answer.clone());
@@ -321,7 +320,8 @@ async fn run_wizard(
.expect("answer comes from the choice list");
cli.orig_from = Some(chosen.to_string());
if chosen == "path" {
let validator = |path: &str| options::validate_orig_path(path);
let validator =
|path: &str| options::validate_orig_path(path).map_err(|e| e.to_string());
let path = prompter.text("Tarball path or URL", "", Some(&validator))?;
cli.orig_path = Some(path);
}
@@ -354,7 +354,7 @@ async fn run_wizard(
if url.is_empty() {
Ok(())
} else {
options::validate_homepage(url)
options::validate_homepage(url).map_err(|e| e.to_string())
}
};
let answer = ask_text(prompter, "Homepage (blank to skip)", &default, validate)?;
@@ -400,12 +400,9 @@ async fn run_wizard(
.as_ref()
.and_then(|p| p.command.clone())
.unwrap_or_else(|| cli.name.clone().unwrap_or_default());
let command = ask_text(
prompter,
"Command name",
&default,
options::validate_command,
)?;
let command = ask_text(prompter, "Command name", &default, |command: &str| {
options::validate_command(command).map_err(|e| e.to_string())
})?;
cli.command = Some(command);
}
@@ -430,7 +427,11 @@ async fn run_wizard(
DEBEMAIL and no git user config): answer as 'Name <email>'"
);
}
let validate = |answer: &str| options::parse_maintainer(answer).map(|_| ());
let validate = |answer: &str| {
options::parse_maintainer(answer)
.map(|_| ())
.map_err(|e| e.to_string())
};
cli.maintainer = Some(ask_text(prompter, "Maintainer", &default, validate)?);
}
@@ -480,7 +481,11 @@ async fn run_wizard(
// 13. Metapackage Depends (empty template only).
if template == TemplateId::EMPTY && cli.depends.is_empty() {
let validate = |answer: &str| options::validate_depends(answer).map(|_| ());
let validate = |answer: &str| {
options::validate_depends(answer)
.map(|_| ())
.map_err(|e| e.to_string())
};
let answer = ask_text(
prompter,
"Depends (metapackage, comma-separated, blank for an empty base)",
@@ -1496,7 +1501,9 @@ mod tests {
assert!(options::validate_upstream_version("v1.0", 1).is_err());
assert!(options::validate_upstream_version("1.0.0", 1).is_ok());
let validate = |version: &str| options::validate_upstream_version(version, 1);
let validate = |version: &str| {
options::validate_upstream_version(version, 1).map_err(|e| e.to_string())
};
// Invalid probes are not offered as the default...
assert_eq!(offered_default("1.0-2", &validate), "");
assert_eq!(offered_default("v1.0", &validate), "");
@@ -1532,7 +1539,7 @@ mod tests {
// The upstream-version scenario end to end: Enter on an invalid
// probed default (offered as none) fails instead of taking it
// verbatim, while a valid default is taken.
let version = |v: &str| options::validate_upstream_version(v, 1);
let version = |v: &str| options::validate_upstream_version(v, 1).map_err(|e| e.to_string());
assert!(accept_answer("", "1.0-2", &version).is_err());
assert_eq!(
accept_answer("", "1.0.0", &version),