diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-06-05 01:50:16 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-06-05 01:50:16 +0900 |
| commit | bbb2f9454b6580fcbd337ced61b5a1888d52ce8d (patch) | |
| tree | 45563449df57f63ae4a4d010a3e8e6007518c35b /crates | |
| parent | adf14510b00929aaee85ccb8dedf9164878a0164 (diff) | |
| download | php-shirabe-bbb2f9454b6580fcbd337ced61b5a1888d52ce8d.tar.gz php-shirabe-bbb2f9454b6580fcbd337ced61b5a1888d52ce8d.tar.zst php-shirabe-bbb2f9454b6580fcbd337ced61b5a1888d52ce8d.zip | |
refactor(io): model ask_and_validate validators with anyhow::Result
PHP's askAndValidate throws when a validator rejects input. Change the
IOInterface validator callback and return type to anyhow::Result so the
call sites can return Err instead of panic, faithfully modeling the throw
semantics already supported by the Question layer.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Diffstat (limited to 'crates')
| -rw-r--r-- | crates/shirabe/src/command/init_command.rs | 72 | ||||
| -rw-r--r-- | crates/shirabe/src/command/package_discovery_trait.rs | 60 | ||||
| -rw-r--r-- | crates/shirabe/src/command/self_update_command.rs | 32 | ||||
| -rw-r--r-- | crates/shirabe/src/io/buffer_io.rs | 4 | ||||
| -rw-r--r-- | crates/shirabe/src/io/console_io.rs | 13 | ||||
| -rw-r--r-- | crates/shirabe/src/io/io_interface.rs | 8 | ||||
| -rw-r--r-- | crates/shirabe/src/io/null_io.rs | 6 | ||||
| -rw-r--r-- | crates/shirabe/src/util/auth_helper.rs | 20 |
8 files changed, 113 insertions, 102 deletions
diff --git a/crates/shirabe/src/command/init_command.rs b/crates/shirabe/src/command/init_command.rs index 678e76d..053df36 100644 --- a/crates/shirabe/src/command/init_command.rs +++ b/crates/shirabe/src/command/init_command.rs @@ -539,9 +539,9 @@ impl InitCommand { "Package name (<vendor>/<name>) [<comment>{}</comment>]: ", name_default ), - Box::new(move |value: PhpMixed| -> PhpMixed { + Box::new(move |value: PhpMixed| -> anyhow::Result<PhpMixed> { if value.is_null() { - return PhpMixed::String(name_for_validate.clone()); + return Ok(PhpMixed::String(name_for_validate.clone())); } if !Preg::is_match( @@ -550,21 +550,21 @@ impl InitCommand { ) .unwrap_or(false) { - // TODO(phase-b): closure returning PhpMixed cannot throw — needs Result type - panic!( - "{}", - format!( + return Err(InvalidArgumentException { + message: format!( "The package name {} is invalid, it should be lowercase and have a vendor name, a forward slash, and a package name, matching: [a-z0-9_.-]+/[a-z0-9_.-]+", value.as_string().unwrap_or("") - ) - ); + ), + code: 0, + } + .into()); } - value + Ok(value) }), None, PhpMixed::String(name.clone()), - ) + )? .as_string() .unwrap_or("") .to_string(); @@ -605,10 +605,10 @@ impl InitCommand { } ), // TODO(phase-b): closure cannot call &self.parse_author_string; needs &self capture - Box::new(move |value: PhpMixed| -> PhpMixed { + Box::new(move |value: PhpMixed| -> anyhow::Result<PhpMixed> { let value_str = value.as_string().unwrap_or("").to_string(); if value_str == "n" || value_str == "no" { - return PhpMixed::Null; + return Ok(PhpMixed::Null); } let value_or_default = if value_str.is_empty() { author_for_validate.clone() @@ -617,11 +617,11 @@ impl InitCommand { }; // TODO(phase-b): would call self.parse_author_string(value_or_default) let _ = value_or_default; - PhpMixed::Null + Ok(PhpMixed::Null) }), None, PhpMixed::String(author_default), - ); + )?; input.set_option("author", author_value); let minimum_stability = input @@ -635,19 +635,17 @@ impl InitCommand { "Minimum Stability [<comment>{}</comment>]: ", minimum_stability.clone().unwrap_or_default() ), - Box::new(move |value: PhpMixed| -> PhpMixed { + Box::new(move |value: PhpMixed| -> anyhow::Result<PhpMixed> { if value.is_null() { - return minimum_stability_for_validate + return Ok(minimum_stability_for_validate .clone() .map(PhpMixed::String) - .unwrap_or(PhpMixed::Null); + .unwrap_or(PhpMixed::Null)); } if !base_package::STABILITIES.contains_key(value.as_string().unwrap_or("")) { - // TODO(phase-b): closure cannot throw - panic!( - "{}", - format!( + return Err(InvalidArgumentException { + message: format!( "Invalid minimum stability \"{}\". Must be empty or one of: {}", value.as_string().unwrap_or(""), implode( @@ -657,17 +655,19 @@ impl InitCommand { .map(|k| k.to_string()) .collect::<Vec<_>>() ) - ) - ); + ), + code: 0, + } + .into()); } - value + Ok(value) }), None, minimum_stability_default .map(PhpMixed::String) .unwrap_or(PhpMixed::Null), - ); + )?; input.set_option("stability", minimum_stability_value); let type_val = input.get_option("type"); @@ -827,14 +827,14 @@ impl InitCommand { "Add PSR-4 autoload mapping? Maps namespace \"{}\" to the entered relative path. [<comment>{}</comment>, n to skip]: ", namespace, autoload ), - Box::new(move |value: PhpMixed| -> PhpMixed { + Box::new(move |value: PhpMixed| -> anyhow::Result<PhpMixed> { if value.is_null() { - return PhpMixed::String(autoload_for_validate.clone()); + return Ok(PhpMixed::String(autoload_for_validate.clone())); } let value_str = value.as_string().unwrap_or("").to_string(); if value_str == "n" || value_str == "no" { - return PhpMixed::Null; + return Ok(PhpMixed::Null); } let value_or_default = if value_str.is_empty() { @@ -845,21 +845,21 @@ impl InitCommand { if !Preg::is_match(r"{^[^/][A-Za-z0-9\-_/]+/$}", &value_or_default).unwrap_or(false) { - // TODO(phase-b): closure cannot throw - panic!( - "{}", - sprintf( + return Err(InvalidArgumentException { + message: sprintf( "The src folder name \"%s\" is invalid. Please add a relative path with tailing forward slash. [A-Za-z0-9_-/]+/", &[PhpMixed::String(value_or_default.clone())], - ) - ); + ), + code: 0, + } + .into()); } - PhpMixed::String(value_or_default) + Ok(PhpMixed::String(value_or_default)) }), None, PhpMixed::String(autoload_default), - ); + )?; input.set_option("autoload", autoload_value); Ok(()) diff --git a/crates/shirabe/src/command/package_discovery_trait.rs b/crates/shirabe/src/command/package_discovery_trait.rs index 7119b7f..197636c 100644 --- a/crates/shirabe/src/command/package_discovery_trait.rs +++ b/crates/shirabe/src/command/package_discovery_trait.rs @@ -9,10 +9,10 @@ use shirabe_external_packages::composer::pcre::{CaptureKey, Preg}; use shirabe_external_packages::symfony::component::console::input::InputInterface; use shirabe_external_packages::symfony::component::console::output::OutputInterface; use shirabe_php_shim::{ - InvalidArgumentException, LogicException, PHP_EOL, PhpMixed, array_keys, array_slice, - array_unshift, array_values, asort, count, explode, file_get_contents, implode, in_array, - is_array, is_file, is_numeric, is_string, json_decode, levenshtein, sprintf, strlen, strpos, - trim, + Exception, InvalidArgumentException, LogicException, PHP_EOL, PhpMixed, array_keys, + array_slice, array_unshift, array_values, asort, count, explode, file_get_contents, implode, + in_array, is_array, is_file, is_numeric, is_string, json_decode, levenshtein, sprintf, strlen, + strpos, trim, }; use crate::composer::PartialComposerHandle; @@ -353,17 +353,17 @@ pub trait PackageDiscoveryTrait { let matches_clone = matches.clone(); let version_parser_clone = version_parser.clone(); - let validator: Box<dyn Fn(PhpMixed) -> PhpMixed> = - Box::new(move |selection_mixed: PhpMixed| -> PhpMixed { + let validator: Box<dyn Fn(PhpMixed) -> anyhow::Result<PhpMixed>> = Box::new( + move |selection_mixed: PhpMixed| -> anyhow::Result<PhpMixed> { let selection = selection_mixed.as_string().unwrap_or("").to_string(); if "" == selection { - return PhpMixed::Bool(false); + return Ok(PhpMixed::Bool(false)); } if is_numeric(&PhpMixed::String(selection.clone())) { let idx: usize = selection.parse().unwrap_or(0); if let Some(p) = matches_clone.get(idx) { - return PhpMixed::String(p.name.clone()); + return Ok(PhpMixed::String(p.name.clone())); } } @@ -380,29 +380,32 @@ pub trait PackageDiscoveryTrait { { // parsing `acme/example ~2.3` // validate version constraint - // TODO(phase-b): parse_constraints returns Result - let _ = version_parser_clone.parse_constraints(&v); + version_parser_clone.parse_constraints(&v)?; - return PhpMixed::String(format!( + return Ok(PhpMixed::String(format!( "{} {}", m.get(&CaptureKey::ByName("name".to_string())) .cloned() .unwrap_or_default(), v, - )); + ))); } // parsing `acme/example` - return PhpMixed::String( + return Ok(PhpMixed::String( m.get(&CaptureKey::ByName("name".to_string())) .cloned() .unwrap_or_default(), - ); + )); } - // TODO(phase-b): throw new \Exception('Not a valid selection'); - panic!("Not a valid selection"); - }); + Err(Exception { + message: "Not a valid selection".to_string(), + code: 0, + } + .into()) + }, + ); package = io .ask_and_validate( @@ -410,7 +413,7 @@ pub trait PackageDiscoveryTrait { validator, Some(3), PhpMixed::String(String::new()), - ) + )? .as_string() .map(|s| s.to_string()) .unwrap_or_default(); @@ -418,13 +421,13 @@ pub trait PackageDiscoveryTrait { // no constraint yet, determine the best version automatically if !package.is_empty() && strpos(&package, " ").is_none() { - let validator: Box<dyn Fn(PhpMixed) -> PhpMixed> = - Box::new(|input_mixed: PhpMixed| -> PhpMixed { + let validator: Box<dyn Fn(PhpMixed) -> anyhow::Result<PhpMixed>> = + Box::new(|input_mixed: PhpMixed| -> anyhow::Result<PhpMixed> { let input = trim(input_mixed.as_string().unwrap_or(""), None); if strlen(&input) > 0 { - PhpMixed::String(input) + Ok(PhpMixed::String(input)) } else { - PhpMixed::Bool(false) + Ok(PhpMixed::Bool(false)) } }); @@ -433,7 +436,7 @@ pub trait PackageDiscoveryTrait { validator, Some(3), PhpMixed::String(String::new()), - ); + )?; let constraint: String = match &constraint_mixed { PhpMixed::Bool(false) => { @@ -537,14 +540,13 @@ pub trait PackageDiscoveryTrait { if input.is_interactive() { let providers_count = providers.len(); let name_owned = name.to_string(); - let validator: Box<dyn Fn(PhpMixed) -> PhpMixed> = - Box::new(move |value_mixed: PhpMixed| -> PhpMixed { + let validator: Box<dyn Fn(PhpMixed) -> anyhow::Result<PhpMixed>> = + Box::new(move |value_mixed: PhpMixed| -> anyhow::Result<PhpMixed> { let value = value_mixed.as_string().unwrap_or("").to_string(); let parser = VersionParser::new(); - // TODO(phase-b): parse_constraints returns Result - let _ = parser.parse_constraints(&value); + parser.parse_constraints(&value)?; - PhpMixed::String(value) + Ok(PhpMixed::String(value)) }); constraint = self .get_io() @@ -556,7 +558,7 @@ pub trait PackageDiscoveryTrait { validator, Some(3), PhpMixed::String("*".to_string()), - ) + )? .as_string() .map(|s| s.to_string()) .unwrap_or_default(); diff --git a/crates/shirabe/src/command/self_update_command.rs b/crates/shirabe/src/command/self_update_command.rs index 9b960f0..63e6a0c 100644 --- a/crates/shirabe/src/command/self_update_command.rs +++ b/crates/shirabe/src/command/self_update_command.rs @@ -672,9 +672,8 @@ RGv89BPD+2DLnJysngsvVaUCAwEAAQ==\n\ "Open <info>https://composer.github.io/pubkeys.html</info> to find the latest keys", ); - // TODO(phase-b): closure captures none; PHP throws inside the closure on bad input - let validator: Box<dyn Fn(PhpMixed) -> PhpMixed> = - Box::new(|value: PhpMixed| -> PhpMixed { + let validator: std::rc::Rc<dyn Fn(PhpMixed) -> anyhow::Result<PhpMixed>> = + std::rc::Rc::new(|value: PhpMixed| -> anyhow::Result<PhpMixed> { let value_str = value.as_string().unwrap_or("").to_string(); if !Preg::is_match( r"{^-----BEGIN PUBLIC KEY-----$}", @@ -682,11 +681,17 @@ RGv89BPD+2DLnJysngsvVaUCAwEAAQ==\n\ ) .unwrap_or(false) { - // TODO(phase-b): closure cannot throw - panic!("{}", "Invalid input"); + return Err(UnexpectedValueException { + message: "Invalid input".to_string(), + code: 0, + } + .into()); } - PhpMixed::String(format!("{}\n", shirabe_php_shim::trim(&value_str, None))) + Ok(PhpMixed::String(format!( + "{}\n", + shirabe_php_shim::trim(&value_str, None) + ))) }); let mut dev_key = String::new(); @@ -708,10 +713,13 @@ RGv89BPD+2DLnJysngsvVaUCAwEAAQ==\n\ dev_key = io .ask_and_validate( "Enter Dev / Snapshot Public Key (including lines with -----): ".to_string(), - Box::new(|v: PhpMixed| v), + { + let validator = validator.clone(); + Box::new(move |v: PhpMixed| validator(v)) + }, None, PhpMixed::Null, - ) + )? .as_string() .unwrap_or("") .to_string(); @@ -727,7 +735,6 @@ RGv89BPD+2DLnJysngsvVaUCAwEAAQ==\n\ } } } - let _ = &validator; let key_path = format!( "{}/keys.dev.pub", config.get("home").as_string().unwrap_or("") @@ -757,10 +764,13 @@ RGv89BPD+2DLnJysngsvVaUCAwEAAQ==\n\ tags_key = io .ask_and_validate( "Enter Tags Public Key (including lines with -----): ".to_string(), - Box::new(|v: PhpMixed| v), + { + let validator = validator.clone(); + Box::new(move |v: PhpMixed| validator(v)) + }, None, PhpMixed::Null, - ) + )? .as_string() .unwrap_or("") .to_string(); diff --git a/crates/shirabe/src/io/buffer_io.rs b/crates/shirabe/src/io/buffer_io.rs index 291b64b..836fd95 100644 --- a/crates/shirabe/src/io/buffer_io.rs +++ b/crates/shirabe/src/io/buffer_io.rs @@ -175,10 +175,10 @@ impl crate::io::IOInterfaceImmutable for BufferIO { fn ask_and_validate( &self, question: String, - validator: Box<dyn Fn(PhpMixed) -> PhpMixed>, + validator: Box<dyn Fn(PhpMixed) -> anyhow::Result<PhpMixed>>, attempts: Option<i64>, default: PhpMixed, - ) -> PhpMixed { + ) -> anyhow::Result<PhpMixed> { self.inner .ask_and_validate(question, validator, attempts, default) } diff --git a/crates/shirabe/src/io/console_io.rs b/crates/shirabe/src/io/console_io.rs index b69b68b..44e5609 100644 --- a/crates/shirabe/src/io/console_io.rs +++ b/crates/shirabe/src/io/console_io.rs @@ -487,10 +487,10 @@ impl IOInterfaceImmutable for ConsoleIO { fn ask_and_validate( &self, question: String, - validator: Box<dyn Fn(PhpMixed) -> PhpMixed>, + validator: Box<dyn Fn(PhpMixed) -> anyhow::Result<PhpMixed>>, attempts: Option<i64>, default: PhpMixed, - ) -> PhpMixed { + ) -> anyhow::Result<PhpMixed> { let _helper = self.helper_set.get("question"); let sanitized_question = Self::sanitize(PhpMixed::String(question), true) .as_string() @@ -502,13 +502,10 @@ impl IOInterfaceImmutable for ConsoleIO { Some(default) }; let mut question = Question::new(&sanitized_question, sanitized_default); - // TODO(phase-b): IOInterface validator type is Box<dyn Fn(PhpMixed) -> PhpMixed> - // but Question::set_validator expects Option<Box<dyn Fn(Option<PhpMixed>) -> Result<PhpMixed>>>. - // Bridge the signatures by adapting the input/output types. + // Question::set_validator takes Fn(Option<PhpMixed>) -> Result<PhpMixed>; adapt the + // None answer to PHP's null and forward the validator's Result unchanged. let adapted: Box<dyn Fn(Option<PhpMixed>) -> anyhow::Result<PhpMixed>> = - Box::new(move |answer: Option<PhpMixed>| { - Ok(validator(answer.unwrap_or(PhpMixed::Null))) - }); + Box::new(move |answer: Option<PhpMixed>| validator(answer.unwrap_or(PhpMixed::Null))); question.set_validator(Some(adapted)); question.set_max_attempts(attempts); diff --git a/crates/shirabe/src/io/io_interface.rs b/crates/shirabe/src/io/io_interface.rs index e48f9ce..e33cea2 100644 --- a/crates/shirabe/src/io/io_interface.rs +++ b/crates/shirabe/src/io/io_interface.rs @@ -101,10 +101,10 @@ pub trait IOInterfaceImmutable: std::fmt::Debug { fn ask_and_validate( &self, question: String, - validator: Box<dyn Fn(PhpMixed) -> PhpMixed>, + validator: Box<dyn Fn(PhpMixed) -> anyhow::Result<PhpMixed>>, attempts: Option<i64>, default: PhpMixed, - ) -> PhpMixed; + ) -> anyhow::Result<PhpMixed>; fn ask_and_hide_answer(&self, question: String) -> Option<String>; @@ -279,10 +279,10 @@ impl IOInterfaceImmutable for Rc<RefCell<dyn IOInterface>> { fn ask_and_validate( &self, question: String, - validator: Box<dyn Fn(PhpMixed) -> PhpMixed>, + validator: Box<dyn Fn(PhpMixed) -> anyhow::Result<PhpMixed>>, attempts: Option<i64>, default: PhpMixed, - ) -> PhpMixed { + ) -> anyhow::Result<PhpMixed> { self.borrow() .ask_and_validate(question, validator, attempts, default) } diff --git a/crates/shirabe/src/io/null_io.rs b/crates/shirabe/src/io/null_io.rs index 85a5fa5..0a83bff 100644 --- a/crates/shirabe/src/io/null_io.rs +++ b/crates/shirabe/src/io/null_io.rs @@ -70,11 +70,11 @@ impl IOInterfaceImmutable for NullIO { fn ask_and_validate( &self, _question: String, - _validator: Box<dyn Fn(PhpMixed) -> PhpMixed>, + _validator: Box<dyn Fn(PhpMixed) -> anyhow::Result<PhpMixed>>, _attempts: Option<i64>, default: PhpMixed, - ) -> PhpMixed { - default + ) -> anyhow::Result<PhpMixed> { + Ok(default) } fn ask_and_hide_answer(&self, _question: String) -> Option<String> { diff --git a/crates/shirabe/src/util/auth_helper.rs b/crates/shirabe/src/util/auth_helper.rs index 3471a54..0465b10 100644 --- a/crates/shirabe/src/util/auth_helper.rs +++ b/crates/shirabe/src/util/auth_helper.rs @@ -5,9 +5,9 @@ use anyhow::Result; use indexmap::IndexMap; use shirabe_external_packages::composer::pcre::Preg; use shirabe_php_shim::{ - E_USER_DEPRECATED, PHP_URL_HOST, PHP_URL_PATH, PHP_URL_SCHEME, PhpMixed, base64_encode, - explode, in_array, is_array, is_string, json_decode, parse_url, sprintf, str_replace, strpos, - strtolower, substr, trigger_error, trim, + E_USER_DEPRECATED, PHP_URL_HOST, PHP_URL_PATH, PHP_URL_SCHEME, PhpMixed, RuntimeException, + base64_encode, explode, in_array, is_array, is_string, json_decode, parse_url, sprintf, + str_replace, strpos, strtolower, substr, trigger_error, trim, }; use crate::config::Config; @@ -69,7 +69,7 @@ impl AuthHelper { origin, config_source.get_name(), ), - Box::new(|value: PhpMixed| -> PhpMixed { + Box::new(|value: PhpMixed| -> anyhow::Result<PhpMixed> { let input = strtolower(&substr( &trim(value.as_string().unwrap_or(""), None), 0, @@ -83,15 +83,17 @@ impl AuthHelper { ]), false, ) { - return PhpMixed::String(input); + return Ok(PhpMixed::String(input)); } - // PHP: throw new \RuntimeException('Please answer (y)es or (n)o'); - // TODO(phase-b): validator should return a recoverable error rather than panic - panic!("Please answer (y)es or (n)o"); + Err(RuntimeException { + message: "Please answer (y)es or (n)o".to_string(), + code: 0, + } + .into()) }), None, PhpMixed::String("y".to_string()), - ); + )?; if answer.as_string() == Some("y") { store = Some(()); |
