From 421d98d9a52dd036acd5632f9049f4503c9add18 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Tue, 23 Jun 2026 01:41:33 +0900 Subject: refactor(io): hold QuestionHelper directly in ConsoleIO instead of HelperSet Every ConsoleIO construction site only ever registers a single QuestionHelper (production) or none (tests), and routing asks through HelperSet::get('question') loses the concrete type, forcing a downcast back to QuestionHelper. Receive the QuestionHelper in the constructor and hold it directly (in a RefCell, since QuestionHelper::ask takes &mut self while the IOInterfaceImmutable ask methods are &self), dropping the HelperSet field and the throwaway HelperSet built at each call site. Co-Authored-By: Claude Opus 4.8 --- crates/shirabe/src/console/application.rs | 12 +----- crates/shirabe/src/io/buffer_io.rs | 16 +------ crates/shirabe/src/io/console_io.rs | 50 +++++++--------------- crates/shirabe/tests/io/console_io_test.rs | 7 ++- crates/shirabe/tests/util/process_executor_test.rs | 4 +- 5 files changed, 23 insertions(+), 66 deletions(-) diff --git a/crates/shirabe/src/console/application.rs b/crates/shirabe/src/console/application.rs index 5dd2eeb..c3287e7 100644 --- a/crates/shirabe/src/console/application.rs +++ b/crates/shirabe/src/console/application.rs @@ -1973,20 +1973,10 @@ impl ApplicationHandle { input.borrow_mut().set_interactive(false); } - let mut helpers: IndexMap< - HelperSetKey, - std::rc::Rc>, - > = IndexMap::new(); - helpers.insert( - HelperSetKey::Int(0), - std::rc::Rc::new(std::cell::RefCell::new(QuestionHelper::default())), - ); - let helper_set = std::rc::Rc::new(std::cell::RefCell::new(HelperSet::default())); - HelperSet::new(&helper_set, helpers); application.borrow_mut().io = std::rc::Rc::new(std::cell::RefCell::new(ConsoleIO::new( input.clone(), output.clone(), - helper_set.borrow().clone(), + QuestionHelper::default(), ))); // Cache the IO so the rest of the flow does not re-borrow the application; command callbacks // (e.g. mergeApplicationDefinition) need the application's RefCell free. diff --git a/crates/shirabe/src/io/buffer_io.rs b/crates/shirabe/src/io/buffer_io.rs index 91d22d7..933deab 100644 --- a/crates/shirabe/src/io/buffer_io.rs +++ b/crates/shirabe/src/io/buffer_io.rs @@ -4,9 +4,6 @@ use crate::io::ConsoleIO; use anyhow::Result; use shirabe_external_packages::composer::pcre::Preg; use shirabe_external_packages::symfony::console::formatter::OutputFormatterInterface; -use shirabe_external_packages::symfony::console::helper::HelperInterface; -use shirabe_external_packages::symfony::console::helper::HelperSet; -use shirabe_external_packages::symfony::console::helper::HelperSetKey; use shirabe_external_packages::symfony::console::helper::QuestionHelper; use shirabe_external_packages::symfony::console::input::InputInterface; use shirabe_external_packages::symfony::console::input::StringInput; @@ -53,22 +50,11 @@ impl BufferIO { let output: std::rc::Rc> = todo!("wire StreamOutput as the ConsoleIO output (needs PhpResource stream)"); - let question_helper: std::rc::Rc> = - std::rc::Rc::new(std::cell::RefCell::new(QuestionHelper::default())); - let mut helpers: indexmap::IndexMap< - HelperSetKey, - std::rc::Rc>, - > = indexmap::IndexMap::new(); - helpers.insert(HelperSetKey::Int(0), question_helper); - let helper_set = std::rc::Rc::new(std::cell::RefCell::new(HelperSet::default())); - HelperSet::new(&helper_set, helpers); - let helper_set = helper_set.borrow().clone(); - let inner = ConsoleIO::new( std::rc::Rc::new(std::cell::RefCell::new(input_obj)) as std::rc::Rc>, output, - helper_set, + QuestionHelper::default(), ); Ok(Self { inner }) diff --git a/crates/shirabe/src/io/console_io.rs b/crates/shirabe/src/io/console_io.rs index e6261c7..d6e10da 100644 --- a/crates/shirabe/src/io/console_io.rs +++ b/crates/shirabe/src/io/console_io.rs @@ -5,7 +5,6 @@ use crate::io::io_interface; use indexmap::IndexMap; use indexmap::indexmap; use shirabe_external_packages::composer::pcre::Preg; -use shirabe_external_packages::symfony::console::helper::HelperSet; use shirabe_external_packages::symfony::console::helper::ProgressBar; use shirabe_external_packages::symfony::console::helper::QuestionHelper; use shirabe_external_packages::symfony::console::helper::Table; @@ -37,26 +36,25 @@ pub struct ConsoleIO { pub(crate) input: std::rc::Rc>, pub(crate) output: std::rc::Rc>, - pub(crate) helper_set: HelperSet, + + /// `$helperSet` is flattened to `question_helper`. PHP stores a `HelperSet` and calls + /// `$this->helperSet->get('question')` on every ask. The Application class, which constructs + /// this class, registers a single `QuestionHelper`. So this port holds the `QuestionHelper` + /// directly instead of a `HelperSet`. + question_helper: RefCell, + pub(crate) last_message: RefCell, pub(crate) last_message_err: RefCell, - /// @var float start_time: Option, - /// @var array verbosity_map: IndexMap, } impl ConsoleIO { - /// Constructor. - /// - /// @param InputInterface $input The input instance - /// @param OutputInterface $output The output instance - /// @param HelperSet $helperSet The helperSet instance pub fn new( input: std::rc::Rc>, output: std::rc::Rc>, - helper_set: HelperSet, + question_helper: QuestionHelper, ) -> Self { let mut verbosity_map = IndexMap::new(); verbosity_map.insert(io_interface::QUIET, output_interface::VERBOSITY_QUIET); @@ -71,7 +69,7 @@ impl ConsoleIO { authentications: indexmap![], input, output, - helper_set, + question_helper: RefCell::new(question_helper), last_message: RefCell::new(String::new()), last_message_err: RefCell::new(String::new()), start_time: None, @@ -83,7 +81,6 @@ impl ConsoleIO { self.start_time = Some(start_time); } - /// @param string[]|string $messages fn do_write(&self, messages: PhpMixed, newline: bool, stderr: bool, verbosity: i64, raw: bool) { let mut sf_verbosity = *self.verbosity_map.get(&verbosity).unwrap_or(&0); if sf_verbosity > self.output.borrow().get_verbosity() { @@ -157,7 +154,6 @@ impl ConsoleIO { ); } - /// @param string[]|string $messages fn do_overwrite( &self, messages: PhpMixed, @@ -241,7 +237,6 @@ impl ConsoleIO { } pub fn get_progress_bar(&self, max: i64) -> ProgressBar { - // PHP ProgressBar::__construct default $minSecondsBetweenRedraws = 1 / 25. ProgressBar::new(self.get_error_output(), max, 1.0 / 25.0) } @@ -366,24 +361,16 @@ impl ConsoleIO { } } - /// Resolves the "question" helper and delegates to `QuestionHelper::ask`. + /// Delegates to `QuestionHelper::ask`. /// - /// PHP: `$this->helperSet->get('question')->ask($this->input, $this->getErrorOutput(), $question)`. - /// `HelperSet::get` and `QuestionHelper::ask` both surface PHP exceptions; ConsoleIO does not - /// catch them, so an unrecoverable error here is a PHP fatal. The double `Result` is collapsed: - /// the outer `anyhow::Result` (fatal) and the inner `MissingInputException` (unhandled, hence - /// also fatal in PHP) both abort. + /// PHP: `$helper->ask($this->input, $this->getErrorOutput(), $question)`. + /// `QuestionHelper::ask` surfaces PHP exceptions; ConsoleIO does not catch them, so an + /// unrecoverable error here is a PHP fatal. The double `Result` is collapsed: the outer + /// `anyhow::Result` (fatal) and the inner `MissingInputException` (unhandled, hence also fatal + /// in PHP) both abort. fn ask_question(&self, question: &Question) -> PhpMixed { - let helper_rc = self - .helper_set - .get("question") - .expect("the \"question\" helper is always registered"); let error_output = self.get_error_output(); - let mut helper = helper_rc.borrow_mut(); - let question_helper = (*helper) - .as_any_mut() - .downcast_mut::() - .expect("the \"question\" helper is a QuestionHelper"); + let mut question_helper = self.question_helper.borrow_mut(); let mut input = self.input.borrow_mut(); question_helper .ask(&mut *input, error_output, question) @@ -561,16 +548,11 @@ impl IOInterfaceImmutable for ConsoleIO { question: String, choices: Vec, default: PhpMixed, - // PHP: int|false attempts attempts: PhpMixed, error_message: String, multiselect: bool, ) -> PhpMixed { let choices: PhpMixed = PhpMixed::List(choices.into_iter().map(PhpMixed::String).collect()); - let _helper = self - .helper_set - .get("question") - .expect("the \"question\" helper is always registered"); let sanitized_question = Self::sanitize(PhpMixed::String(question), true) .as_string() .unwrap_or("") diff --git a/crates/shirabe/tests/io/console_io_test.rs b/crates/shirabe/tests/io/console_io_test.rs index a79b7ca..3c8242a 100644 --- a/crates/shirabe/tests/io/console_io_test.rs +++ b/crates/shirabe/tests/io/console_io_test.rs @@ -7,7 +7,7 @@ use indexmap::IndexMap; use shirabe::io::ConsoleIO; use shirabe::io::IOInterfaceImmutable; use shirabe::io::IOInterfaceMutable; -use shirabe_external_packages::symfony::console::helper::HelperSet; +use shirabe_external_packages::symfony::console::helper::QuestionHelper; use shirabe_external_packages::symfony::console::input::array_input::ArrayInput; use shirabe_external_packages::symfony::console::input::input_interface::InputInterface; use shirabe_external_packages::symfony::console::output::buffered_output::BufferedOutput; @@ -15,7 +15,7 @@ use shirabe_external_packages::symfony::console::output::output_interface::Outpu use std::cell::RefCell; use std::rc::Rc; -/// Builds a ConsoleIO backed by real, side-effect-free Input/Output/HelperSet. PHP uses +/// Builds a ConsoleIO backed by real, side-effect-free Input/Output/QuestionHelper. PHP uses /// PHPUnit mocks here, but for tests that exercise only the authentication map they carry no /// expectations, so concrete implementations are an exact substitute. fn make_console_io() -> ConsoleIO { @@ -23,8 +23,7 @@ fn make_console_io() -> ConsoleIO { Rc::new(RefCell::new(ArrayInput::new(vec![], None).unwrap())); let output: Rc> = Rc::new(RefCell::new(BufferedOutput::new(None, false, None))); - let helper_set = HelperSet::default(); - ConsoleIO::new(input, output, helper_set) + ConsoleIO::new(input, output, QuestionHelper::default()) } #[ignore = "requires a PHPUnit mock of InputInterface::isInteractive with willReturnOnConsecutiveCalls; no mocking framework"] diff --git a/crates/shirabe/tests/util/process_executor_test.rs b/crates/shirabe/tests/util/process_executor_test.rs index e5ee7c2..0416a34 100644 --- a/crates/shirabe/tests/util/process_executor_test.rs +++ b/crates/shirabe/tests/util/process_executor_test.rs @@ -11,7 +11,7 @@ use shirabe::io::ConsoleIO; use shirabe::io::IOInterface; use shirabe::io::buffer_io::BufferIO; use shirabe::util::process_executor::ProcessExecutor; -use shirabe_external_packages::symfony::console::helper::HelperSet; +use shirabe_external_packages::symfony::console::helper::QuestionHelper; use shirabe_external_packages::symfony::console::input::array_input::ArrayInput; use shirabe_external_packages::symfony::console::input::input_interface::InputInterface; use shirabe_external_packages::symfony::console::output::buffered_output::BufferedOutput; @@ -143,7 +143,7 @@ fn test_console_io_does_not_format_symfony_console_style() { let console_io = ConsoleIO::new( input, output.clone() as Rc>, - HelperSet::default(), + QuestionHelper::default(), ); let mut process = ProcessExecutor::new(Some( Rc::new(RefCell::new(console_io)) as Rc> -- cgit v1.3.1