diff options
20 files changed, 1189 insertions, 269 deletions
diff --git a/crates/shirabe-php-rpc/php/guards/Composer/Util/ProcessExecutor.php b/crates/shirabe-php-rpc/php/guards/Composer/Util/ProcessExecutor.php deleted file mode 100644 index 405905e8..00000000 --- a/crates/shirabe-php-rpc/php/guards/Composer/Util/ProcessExecutor.php +++ /dev/null @@ -1,114 +0,0 @@ -<?php - -// Generated by scripts/plugin-stub-generator; do not edit by hand. -// Guard for Composer\Util\ProcessExecutor. -// The Rust side owns this class and the worker has no proxy for it, so this -// declaration shadows the real one: the constants and the hierarchy stay, while -// constructing it or calling anything on it raises an explicit error. - -namespace Composer\Util; - -use Composer\IO\IOInterface; -use React\Promise\PromiseInterface; - -class ProcessExecutor -{ - private const STATUS_QUEUED = 1; - private const STATUS_STARTED = 2; - private const STATUS_COMPLETED = 3; - private const STATUS_FAILED = 4; - private const STATUS_ABORTED = 5; - private const BUILTIN_CMD_COMMANDS = [ - 'assoc', 'break', 'call', 'cd', 'chdir', 'cls', 'color', 'copy', 'date', - 'del', 'dir', 'echo', 'endlocal', 'erase', 'exit', 'for', 'ftype', 'goto', - 'help', 'if', 'label', 'md', 'mkdir', 'mklink', 'move', 'path', 'pause', - 'popd', 'prompt', 'pushd', 'rd', 'rem', 'ren', 'rename', 'rmdir', 'set', - 'setlocal', 'shift', 'start', 'time', 'title', 'type', 'ver', 'vol', - ]; - private const GIT_CMDS_NEED_GIT_DIR = [ - ['show'], - ['log'], - ['branch'], - ['remote', 'set-url'], - ]; - - public function __construct(?IOInterface $io = null) - { - \ShirabeUnsupportedClass::fail(self::class, '__construct'); - } - - public function execute($command, &$output = null, ?string $cwd = null): int - { - \ShirabeUnsupportedClass::fail(self::class, 'execute'); - } - - public function executeTty($command, ?string $cwd = null): int - { - \ShirabeUnsupportedClass::fail(self::class, 'executeTty'); - } - - public function executeAsync($command, ?string $cwd = null): PromiseInterface - { - \ShirabeUnsupportedClass::fail(self::class, 'executeAsync'); - } - - protected function outputHandler(string $type, string $buffer): void - { - \ShirabeUnsupportedClass::fail(self::class, 'outputHandler'); - } - - public function setMaxJobs(int $maxJobs): void - { - \ShirabeUnsupportedClass::fail(self::class, 'setMaxJobs'); - } - - public function resetMaxJobs(): void - { - \ShirabeUnsupportedClass::fail(self::class, 'resetMaxJobs'); - } - - public function wait($index = null): void - { - \ShirabeUnsupportedClass::fail(self::class, 'wait'); - } - - public function enableAsync(): void - { - \ShirabeUnsupportedClass::fail(self::class, 'enableAsync'); - } - - public function countActiveJobs($index = null): int - { - \ShirabeUnsupportedClass::fail(self::class, 'countActiveJobs'); - } - - public function splitLines(?string $output): array - { - \ShirabeUnsupportedClass::fail(self::class, 'splitLines'); - } - - public function getErrorOutput(): string - { - \ShirabeUnsupportedClass::fail(self::class, 'getErrorOutput'); - } - - public static function getTimeout(): int - { - \ShirabeUnsupportedClass::fail(self::class, 'getTimeout'); - } - - public static function setTimeout(int $timeout): void - { - \ShirabeUnsupportedClass::fail(self::class, 'setTimeout'); - } - - public static function escape($argument): string - { - \ShirabeUnsupportedClass::fail(self::class, 'escape'); - } - - public function requiresGitDirEnv($command): bool - { - \ShirabeUnsupportedClass::fail(self::class, 'requiresGitDirEnv'); - } -} diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/Util/Filesystem.php b/crates/shirabe-php-rpc/php/stubs/Composer/Util/Filesystem.php index e1532889..c66bcf56 100644 --- a/crates/shirabe-php-rpc/php/stubs/Composer/Util/Filesystem.php +++ b/crates/shirabe-php-rpc/php/stubs/Composer/Util/Filesystem.php @@ -83,26 +83,6 @@ class Filesystem implements \ShirabeRustStub return $path; } - public static function isLocalPath(string $path) - { - // on windows, \\foo indicates network paths so we exclude those from local paths, however it is unsafe - // on linux as file:////foo (which would be a network path \\foo on windows) will resolve to /foo which could be a local path - if (Platform::isWindows()) { - return Preg::isMatch('{^(file://(?!//)|/(?!/)|/?[a-z]:[\\\\/]|\.\.[\\\\/]|[a-z0-9_.-]+[\\\\/])}i', $path); - } - - return Preg::isMatch('{^(file://|/|/?[a-z]:[\\\\/]|\.\.[\\\\/]|[a-z0-9_.-]+[\\\\/])}i', $path); - } - - public static function getPlatformPath(string $path) - { - if (Platform::isWindows()) { - $path = Preg::replace('{^(?:file:///([a-z]):?/)}i', 'file://$1:/', $path); - } - - return Preg::replace('{^file://}i', '', $path); - } - public static function isReadable(string $path) { if (is_readable($path)) { @@ -121,6 +101,18 @@ class Filesystem implements \ShirabeRustStub return false; } + // Forwarded rather than materialized: it references Composer\Util\Platform, which a guard shadows in the worker. + public static function isLocalPath(string $path) + { + return \ShirabeRpcRuntime::callRust(0, '__shirabeCallStatic', [self::class, 'isLocalPath', [$path]]); + } + + // Forwarded rather than materialized: it references Composer\Util\Platform, which a guard shadows in the worker. + public static function getPlatformPath(string $path) + { + return \ShirabeRpcRuntime::callRust(0, '__shirabeCallStatic', [self::class, 'getPlatformPath', [$path]]); + } + public function remove(string $file) { return \ShirabeRpcRuntime::callRust($this->__rhandle, 'remove', [$file]); diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/Util/ProcessExecutor.php b/crates/shirabe-php-rpc/php/stubs/Composer/Util/ProcessExecutor.php new file mode 100644 index 00000000..dfa1f0d2 --- /dev/null +++ b/crates/shirabe-php-rpc/php/stubs/Composer/Util/ProcessExecutor.php @@ -0,0 +1,176 @@ +<?php + +// Generated by scripts/plugin-stub-generator; do not edit by hand. +// Proxy stub for Composer\Util\ProcessExecutor: the public surface forwards to the Rust-side entity over RPC. + +namespace Composer\Util; + +use Composer\IO\IOInterface; +use React\Promise\PromiseInterface; + +class ProcessExecutor implements \ShirabeRustStub +{ + /** @var int */ + protected $__rhandle; + /** @var int */ + protected $__epoch; + + /** + * Binds a stub the registry built for an existing entity. Proxy instantiation bypasses + * the constructor, which belongs to plugin code building a new entity instead. + */ + public function __shirabeBind(int $rhandle, int $epoch): void + { + $this->__rhandle = $rhandle; + $this->__epoch = $epoch; + } + + public function __destruct() + { + \ShirabeRustObjectRegistry::release($this->__rhandle); + } + + public function __shirabeRustHandleDescriptor(): array + { + return [ + '__rhandle' => $this->__rhandle, + '__class' => static::class, + '__epoch' => $this->__epoch, + ]; + } + + public function __clone() + { + // PHP has already shallow-copied this stub, so both copies would point at one + // entity and release it twice. The Rust side clones the entity instead, applying + // whatever __clone semantics the real class defines, and this copy rebinds to the + // fresh handle. Entities without clone semantics answer with an explicit error. + [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust($this->__rhandle, '__shirabeClone', []); + \ShirabeRustObjectRegistry::adopt($this->__rhandle, $this); + } + + public function __get($name) + { + return \ShirabeRpcRuntime::callRust($this->__rhandle, '__get', [$name]); + } + + public function __set($name, $value): void + { + \ShirabeRpcRuntime::callRust($this->__rhandle, '__set', [$name, $value]); + } + + public function __isset($name): bool + { + return \ShirabeRpcRuntime::callRust($this->__rhandle, '__isset', [$name]); + } + + public function __unset($name): void + { + \ShirabeRpcRuntime::callRust($this->__rhandle, '__unset', [$name]); + } + + public function __construct(?IOInterface $io = null) + { + [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust(0, '__shirabeConstruct', [static::class, [$io]]); + \ShirabeRustObjectRegistry::adopt($this->__rhandle, $this); + } + + private const STATUS_QUEUED = 1; + private const STATUS_STARTED = 2; + private const STATUS_COMPLETED = 3; + private const STATUS_FAILED = 4; + private const STATUS_ABORTED = 5; + private const BUILTIN_CMD_COMMANDS = [ + 'assoc', 'break', 'call', 'cd', 'chdir', 'cls', 'color', 'copy', 'date', + 'del', 'dir', 'echo', 'endlocal', 'erase', 'exit', 'for', 'ftype', 'goto', + 'help', 'if', 'label', 'md', 'mkdir', 'mklink', 'move', 'path', 'pause', + 'popd', 'prompt', 'pushd', 'rd', 'rem', 'ren', 'rename', 'rmdir', 'set', + 'setlocal', 'shift', 'start', 'time', 'title', 'type', 'ver', 'vol', + ]; + private const GIT_CMDS_NEED_GIT_DIR = [ + ['show'], + ['log'], + ['branch'], + ['remote', 'set-url'], + ]; + + // Forwarded rather than materialized: it reads the static property $timeout, whose value the Rust side owns. + public static function getTimeout(): int + { + return \ShirabeRpcRuntime::callRust(0, '__shirabeCallStatic', [self::class, 'getTimeout', []]); + } + + // Forwarded rather than materialized: it reads the static property $timeout, whose value the Rust side owns. + public static function setTimeout(int $timeout): void + { + \ShirabeRpcRuntime::callRust(0, '__shirabeCallStatic', [self::class, 'setTimeout', [$timeout]]); + } + + // Forwarded rather than materialized: it references Composer\Util\Platform, which a guard shadows in the worker. + public static function escape($argument): string + { + return \ShirabeRpcRuntime::callRust(0, '__shirabeCallStatic', [self::class, 'escape', [$argument]]); + } + + public function execute($command, &$output = null, ?string $cwd = null): int + { + $__args = [$command, $output, $cwd]; + array_splice($__args, func_num_args()); + $__out = []; + $__result = \ShirabeRpcRuntime::callRust($this->__rhandle, 'execute', $__args, [1], $__out); + if (array_key_exists(1, $__out)) { + $output = $__out[1]; + } + return $__result; + } + + public function executeTty($command, ?string $cwd = null): int + { + return \ShirabeRpcRuntime::callRust($this->__rhandle, 'executeTty', [$command, $cwd]); + } + + public function executeAsync($command, ?string $cwd = null): PromiseInterface + { + return \ShirabeRpcRuntime::callRust($this->__rhandle, 'executeAsync', [$command, $cwd]); + } + + public function setMaxJobs(int $maxJobs): void + { + \ShirabeRpcRuntime::callRust($this->__rhandle, 'setMaxJobs', [$maxJobs]); + } + + public function resetMaxJobs(): void + { + \ShirabeRpcRuntime::callRust($this->__rhandle, 'resetMaxJobs', []); + } + + public function wait($index = null): void + { + \ShirabeRpcRuntime::callRust($this->__rhandle, 'wait', [$index]); + } + + public function enableAsync(): void + { + \ShirabeRpcRuntime::callRust($this->__rhandle, 'enableAsync', []); + } + + public function countActiveJobs($index = null): int + { + return \ShirabeRpcRuntime::callRust($this->__rhandle, 'countActiveJobs', [$index]); + } + + public function splitLines(?string $output): array + { + return \ShirabeRpcRuntime::callRust($this->__rhandle, 'splitLines', [$output]); + } + + public function getErrorOutput(): string + { + return \ShirabeRpcRuntime::callRust($this->__rhandle, 'getErrorOutput', []); + } + + public function requiresGitDirEnv($command): bool + { + return \ShirabeRpcRuntime::callRust($this->__rhandle, 'requiresGitDirEnv', [$command]); + } +} diff --git a/crates/shirabe-php-rpc/php/worker.php b/crates/shirabe-php-rpc/php/worker.php index 6ed5df40..bc10c2de 100644 --- a/crates/shirabe-php-rpc/php/worker.php +++ b/crates/shirabe-php-rpc/php/worker.php @@ -295,15 +295,29 @@ final class ShirabeRpcRuntime return array_map([self::class, 'fromWire'], $value); } - /** Sends a CallRustMethod request and drives the cooperative loop until its Return. */ - public static function callRust(int $rhandle, string $method, array $args) - { + /** + * Sends a CallRustMethod request and drives the cooperative loop until its Return. + * + * `$outParamPositions` names the by-ref parameters of the called method; the Rust side + * answers with the value each of them holds afterwards, and the caller (a generated stub) + * assigns those back to its own by-ref parameters. + * + * @param list<int> $outParamPositions + * @param array<int, mixed>|null $outParams + */ + public static function callRust( + int $rhandle, + string $method, + array $args, + array $outParamPositions = [], + ?array &$outParams = null + ) { $corrId = self::$nextCorrId; self::$nextCorrId += 2; self::writeFrame( SHIRABE_TAG_CALL_RUST_METHOD, $corrId, - serialize([$rhandle, $method, self::toWire($args), []]) + serialize([$rhandle, $method, self::toWire($args), $outParamPositions]) ); while (true) { $frame = self::readFrame(); @@ -320,6 +334,7 @@ final class ShirabeRpcRuntime self::fail('protocol violation: unparseable response payload'); } if ($tag === SHIRABE_TAG_RETURN) { + $outParams = self::fromWire($fields[1] ?? []); return self::fromWire($fields[0]); } [$class, $message, $code] = $fields; diff --git a/crates/shirabe-php-rpc/src/lib.rs b/crates/shirabe-php-rpc/src/lib.rs index b2bc2c28..dfd8fa67 100644 --- a/crates/shirabe-php-rpc/src/lib.rs +++ b/crates/shirabe-php-rpc/src/lib.rs @@ -643,6 +643,12 @@ impl std::error::Error for PhpThrow {} /// Handles `CallRustMethod` requests arriving while a Rust-initiated call is waiting for its /// `Return` (the cooperative reentrancy loop). A handler may itself issue nested RPC calls. +/// +/// `out_param_positions` names the by-ref parameters of the called method, as the calling stub +/// declared them. A handler that assigns to one writes the resulting value into `out_params` under +/// the same position, and the stub copies it back into the caller's variable; leaving a position +/// out means the method never assigned to it, which is what PHP does with an untouched by-ref +/// parameter. pub trait RustMethodDispatcher { fn dispatch( &mut self, @@ -650,6 +656,7 @@ pub trait RustMethodDispatcher { method_name: &str, args: Vec<PluginValue>, out_param_positions: &[u32], + out_params: &mut IndexMap<u32, PluginValue>, ) -> Result<PluginValue, PhpThrow>; } @@ -812,10 +819,15 @@ fn rpc_call( args, out_param_positions, } => { + let mut out_params = IndexMap::new(); let outcome = match dispatcher.as_deref_mut() { - Some(dispatcher) => { - dispatcher.dispatch(rhandle, &method_name, args, &out_param_positions) - } + Some(dispatcher) => dispatcher.dispatch( + rhandle, + &method_name, + args, + &out_param_positions, + &mut out_params, + ), // Never fall back to a silent null: an unroutable callback is reported as an // explicit error on the PHP side. None => Err(PhpThrow::runtime(format!( @@ -827,7 +839,7 @@ fn rpc_call( Ok(value) => Frame::Return { corr_id, value, - out_params: IndexMap::new(), + out_params, }, Err(throw) => Frame::Throw { corr_id, @@ -1011,6 +1023,10 @@ const STUB_FILES: &[(&str, &str)] = &[ "Composer/Util/Filesystem.php", include_str!("../php/stubs/Composer/Util/Filesystem.php"), ), + ( + "Composer/Util/ProcessExecutor.php", + include_str!("../php/stubs/Composer/Util/ProcessExecutor.php"), + ), ]; /// Hand-written worker-side classes (two-world implementations with behavior of their own, not diff --git a/crates/shirabe/src/event_dispatcher/event_dispatcher.rs b/crates/shirabe/src/event_dispatcher/event_dispatcher.rs index a5a238bd..7b620e5b 100644 --- a/crates/shirabe/src/event_dispatcher/event_dispatcher.rs +++ b/crates/shirabe/src/event_dispatcher/event_dispatcher.rs @@ -1682,6 +1682,7 @@ impl RustMethodDispatcher for ScriptRpcDispatcher<'_> { method_name: &str, args: Vec<PluginValue>, _out_param_positions: &[u32], + out_params: &mut IndexMap<u32, PluginValue>, ) -> Result<PluginValue, PhpThrow> { if rhandle == 0 { if method_name == "__shirabe_find_file" { @@ -1707,6 +1708,9 @@ impl RustMethodDispatcher for ScriptRpcDispatcher<'_> { if method_name == "__shirabeConstruct" { return crate::plugin::php_plugin_proxy::construct_entity(&args); } + if method_name == "__shirabeCallStatic" { + return crate::plugin::php_plugin_proxy::call_static_entity(&args); + } return Err(runtime_throw(format!( "unknown runtime service method `{method_name}`" ))); @@ -1719,6 +1723,7 @@ impl RustMethodDispatcher for ScriptRpcDispatcher<'_> { rhandle, method_name, &args, + out_params, ), } } diff --git a/crates/shirabe/src/plugin/php_plugin_proxy.rs b/crates/shirabe/src/plugin/php_plugin_proxy.rs index 8469593f..84bf195f 100644 --- a/crates/shirabe/src/plugin/php_plugin_proxy.rs +++ b/crates/shirabe/src/plugin/php_plugin_proxy.rs @@ -57,6 +57,7 @@ enum RustEntity { ), Operation(std::rc::Rc<AnyOperation>), Plugin(std::rc::Rc<std::cell::RefCell<dyn PluginInterface>>), + ProcessExecutor(std::rc::Rc<std::cell::RefCell<crate::util::ProcessExecutor>>), } /// The pointer identity backing R-table interning: the same shared instance must always cross @@ -79,6 +80,7 @@ fn entity_ptr_id(entity: &RustEntity) -> usize { } RustEntity::Operation(operation) => std::rc::Rc::as_ptr(operation) as *const () as usize, RustEntity::Plugin(plugin) => std::rc::Rc::as_ptr(plugin) as *const () as usize, + RustEntity::ProcessExecutor(process) => std::rc::Rc::as_ptr(process) as *const () as usize, } } @@ -288,6 +290,7 @@ impl RustMethodDispatcher for PluginRpcDispatcher<'_> { method_name: &str, args: Vec<PluginValue>, _out_param_positions: &[u32], + out_params: &mut IndexMap<u32, PluginValue>, ) -> Result<PluginValue, PhpThrow> { if rhandle == 0 { if method_name == "__shirabe_find_file" { @@ -308,6 +311,9 @@ impl RustMethodDispatcher for PluginRpcDispatcher<'_> { if method_name == "__shirabeConstruct" { return construct_entity(&args); } + if method_name == "__shirabeCallStatic" { + return call_static_entity(&args); + } return Err(runtime_throw(format!( "unknown runtime service method `{method_name}`" ))); @@ -319,7 +325,7 @@ impl RustMethodDispatcher for PluginRpcDispatcher<'_> { return dispatch_event_method(event, method_name); } - dispatch_r_table_method(rhandle, method_name, &args) + dispatch_r_table_method(rhandle, method_name, &args, out_params) } } @@ -330,6 +336,7 @@ pub(crate) fn dispatch_r_table_method( rhandle: u64, method_name: &str, args: &[PluginValue], + out_params: &mut IndexMap<u32, PluginValue>, ) -> Result<PluginValue, PhpThrow> { // The entity is cloned out so no table borrow is held while the handler runs (a // handler that re-enters register_*_entity would otherwise panic on the RefCell). @@ -371,6 +378,9 @@ pub(crate) fn dispatch_r_table_method( dispatch_operation_method(&operation, method_name, args) } Some(RustEntity::Plugin(plugin)) => dispatch_plugin_method(&plugin, method_name), + Some(RustEntity::ProcessExecutor(process)) => { + dispatch_process_executor_method(&process, method_name, args, out_params) + } None => Err(runtime_throw(format!("unknown Rust handle {rhandle}"))), } } @@ -397,6 +407,14 @@ pub(crate) fn construct_entity(args: &[PluginValue]) -> Result<PluginValue, PhpT }; let string_arg = |position: usize| arg::<String>(&class, ctor_args, position); let package_arg = |position: usize| arg::<PackageInterfaceHandle>(&class, ctor_args, position); + let io_arg = |position: usize| { + arg::<std::rc::Rc<std::cell::RefCell<dyn IOInterface>>>(&class, ctor_args, position) + }; + let process_executor_arg = |position: usize| { + arg::<std::rc::Rc<std::cell::RefCell<crate::util::ProcessExecutor>>>( + &class, ctor_args, position, + ) + }; let alias_package_arg = |position: usize| -> Result<crate::package::AliasPackageHandle, PhpThrow> { package_arg(position)? @@ -486,18 +504,25 @@ pub(crate) fn construct_entity(args: &[PluginValue]) -> Result<PluginValue, PhpT // runs subprocesses through, so a plugin-built one is a complete instance rather than // a second view on a Rust-side service. "Composer\\Util\\Filesystem" => { - // TODO(plugin): `ProcessExecutor` has no proxy stub, so an executor argument could - // only be a second instance the Rust side never sees. - match ctor_args.first() { - None | Some(PluginValue::Null) => {} - other => { - return Err(runtime_throw(format!( - "{class} cannot take a ProcessExecutor over RPC yet, got {other:?}" - ))); - } - } + let executor = match ctor_args.first() { + None | Some(PluginValue::Null) => None, + _ => Some(process_executor_arg(0)?), + }; let rhandle = register_entity(RustEntity::Filesystem(std::rc::Rc::new( - std::cell::RefCell::new(crate::util::Filesystem::new(None)), + std::cell::RefCell::new(crate::util::Filesystem::new(executor)), + ))); + return Ok(construction_result(rhandle)); + } + // The job queue and the async permits of a process executor are its own state, and the + // timeout it runs children under is the Rust side's; a plugin-built one is a complete + // instance of the former holding the latter, not a second view on the graph's executor. + "Composer\\Util\\ProcessExecutor" => { + let io = match ctor_args.first() { + None | Some(PluginValue::Null) => None, + _ => Some(io_arg(0)?), + }; + let rhandle = register_entity(RustEntity::ProcessExecutor(std::rc::Rc::new( + std::cell::RefCell::new(crate::util::ProcessExecutor::new(io)), ))); return Ok(construction_result(rhandle)); } @@ -551,7 +576,8 @@ fn clone_entity(entity: &RustEntity) -> Result<PluginValue, PhpThrow> { | RustEntity::Repository(_) | RustEntity::EventDispatcher(_) | RustEntity::Operation(_) - | RustEntity::Plugin(_) => { + | RustEntity::Plugin(_) + | RustEntity::ProcessExecutor(_) => { return Err(runtime_throw( "cloning this Rust-side entity over RPC is not supported".to_string(), )); @@ -993,6 +1019,268 @@ fn dispatch_filesystem_method( } } +/// Serves the `ProcessExecutor` proxy stub, whether the executor behind it belongs to the object +/// graph or was built by plugin code writing `new ProcessExecutor(...)`. Both run their children in +/// the Rust process, which is what keeps the timeout, the executable path cache and the job budget +/// single-sourced across the boundary instead of forking a copy per world. +fn dispatch_process_executor_method( + process: &std::rc::Rc<std::cell::RefCell<crate::util::ProcessExecutor>>, + method_name: &str, + args: &[PluginValue], + out_params: &mut IndexMap<u32, PluginValue>, +) -> Result<PluginValue, PhpThrow> { + let failed = |error: anyhow::Error| runtime_throw(format!("{method_name} failed: {error:#}")); + let cwd_arg = |position: usize| -> Result<Option<String>, PhpThrow> { + match args.get(position) { + None | Some(PluginValue::Null) => Ok(None), + _ => arg::<String>(method_name, args, position).map(Some), + } + }; + match method_name { + "execute" => { + let command = exec_command_arg(method_name, args, 0)?; + let cwd = cwd_arg(2)?; + // PHP selects the output handling with `func_num_args() > 1` and `is_callable($output)`; + // the stub reproduces the caller's arity, so an absent argument is the forwarding case. + let Some(output) = args.get(1) else { + return process + .borrow_mut() + .execute( + command, + crate::util::ProcessExecutor::FORWARD_OUTPUT, + cwd.as_deref(), + ) + .map(|code| code.to_plugin_value()) + .map_err(failed); + }; + if is_php_callable(output).map_err(failed)? { + // TODO(plugin): the executor stays mutably borrowed while the callback runs, so a + // callback re-entering this same executor panics instead of nesting the way PHP + // would. + let callable = output.clone(); + let callback: Box<dyn FnMut(&str, &str) -> bool> = + Box::new(move |r#type: &str, buffer: &str| { + // TODO(error-model): `Process::run`'s callback cannot fail, so a throw + // raised by the plugin's handler is dropped here rather than unwinding + // out of the command the way PHP would. + let _ = call_php_callable( + &callable, + vec![PluginValue::string(r#type), PluginValue::string(buffer)], + ); + false + }); + return process + .borrow_mut() + .execute(command, callback, cwd.as_deref()) + .map(|code| code.to_plugin_value()) + .map_err(failed); + } + let mut captured: Option<String> = None; + let code = process + .borrow_mut() + .execute(command, &mut captured, cwd.as_deref()) + .map_err(failed)?; + // PHP leaves `$output` alone when the child was signaled before the assignment, which + // is what an absent out-param position reproduces. + if let Some(captured) = captured { + out_params.insert(1, PluginValue::string(captured)); + } + Ok(code.to_plugin_value()) + } + "executeTty" => Ok(process + .borrow_mut() + .execute_tty( + exec_command_arg(method_name, args, 0)?, + cwd_arg(1)?.as_deref(), + ) + .map_err(failed)? + .to_plugin_value()), + "splitLines" => { + let output = match args.first() { + None | Some(PluginValue::Null) => String::new(), + _ => arg::<String>(method_name, args, 0)?, + }; + Ok(PluginValue::List( + process + .borrow() + .split_lines(&output) + .into_iter() + .map(PluginValue::string) + .collect(), + )) + } + "getErrorOutput" => Ok(PluginValue::string(process.borrow().get_error_output())), + "requiresGitDirEnv" => Ok(process + .borrow() + .requires_git_dir_env(&exec_command_arg(method_name, args, 0)?) + .to_plugin_value()), + "setMaxJobs" => { + process + .borrow_mut() + .set_max_jobs(arg::<i64>(method_name, args, 0)?); + Ok(PluginValue::Null) + } + "resetMaxJobs" => { + process.borrow_mut().reset_max_jobs(); + Ok(PluginValue::Null) + } + // TODO(plugin,async): the async surface needs an execution model that can hand a plugin a + // live `Symfony\Component\Process\Process`. That object's state is the `proc_open()` + // resource and the OS pipes of whichever process called `start()`, so a Rust-side spawn + // has none to give; the real `start()` has to run in the worker, driven by a promise + // representation that crosses the boundary unresolved. Neither exists yet. + "executeAsync" | "wait" | "enableAsync" | "countActiveJobs" => Err(runtime_throw(format!( + "Shirabe does not support ProcessExecutor::{method_name}() from a plugin yet" + ))), + other => Err(runtime_throw(format!( + "unknown ProcessExecutor method `{other}`" + ))), + } +} + +/// Decodes the `string|non-empty-list<string>` a process executor takes as its command. +fn exec_command_arg( + method: &str, + args: &[PluginValue], + position: usize, +) -> Result<crate::util::process_executor::CommandLine, PhpThrow> { + match args.get(position) { + // TODO(bytes): lossy UTF-8; every PHP string crossing the boundary is bytes. + Some(PluginValue::String(bytes)) => Ok(crate::util::process_executor::CommandLine::Shell( + String::from_utf8_lossy(bytes).into_owned(), + )), + Some(PluginValue::List(items)) => { + let mut parts = Vec::with_capacity(items.len()); + for (index, item) in items.iter().enumerate() { + match item { + PluginValue::String(bytes) => { + parts.push(String::from_utf8_lossy(bytes).into_owned()); + } + other => { + return Err(runtime_throw(format!( + "{method} expects a list of strings at position {position}, \ + but element {index} is {other:?}" + ))); + } + } + } + Ok(crate::util::process_executor::CommandLine::Args(parts)) + } + other => Err(arg_throw( + method, + position, + "a command string or argument list", + other, + )), + } +} + +/// PHP's `is_callable($value)`, answered by the worker because only its own function and class +/// tables can say whether a string or a `[$object, 'method']` pair names something callable. +/// Values that cannot name a callable at all are answered here rather than over a round trip. +fn is_php_callable(value: &PluginValue) -> anyhow::Result<bool> { + match value { + PluginValue::String(_) + | PluginValue::List(_) + | PluginValue::Array(_) + | PluginValue::PhpHandle(_) + | PluginValue::PhpClass(_) => { + let answer = unwrap_php_result(call_function_with_dispatcher( + "is_callable", + vec![value.clone()], + Some(&mut PluginRpcDispatcher::default()), + ))?; + match answer { + PluginValue::Bool(answer) => Ok(answer), + other => Err(anyhow::anyhow!( + "is_callable did not return a bool over RPC: {other:?}" + )), + } + } + _ => Ok(false), + } +} + +/// Calls a PHP callable value in the worker, whatever form it takes: `call_user_func` resolves a +/// closure, a function name and an `[$object, 'method']` pair exactly as the original call site +/// would have. +fn call_php_callable( + callable: &PluginValue, + args: Vec<PluginValue>, +) -> anyhow::Result<PluginValue> { + let mut call_args = Vec::with_capacity(args.len() + 1); + call_args.push(callable.clone()); + call_args.extend(args); + unwrap_php_result(call_function_with_dispatcher( + "call_user_func", + call_args, + Some(&mut PluginRpcDispatcher::default()), + )) +} + +/// Serves the static methods a proxy stub forwards instead of running locally, because their real +/// bodies reach state the Rust side owns or classes the worker has no code for. +pub(crate) fn call_static_entity(args: &[PluginValue]) -> Result<PluginValue, PhpThrow> { + let (class, method, call_args) = match (args.first(), args.get(1), args.get(2)) { + // TODO(bytes): lossy UTF-8; class and method names are bytes in PHP. + ( + Some(PluginValue::String(class)), + Some(PluginValue::String(method)), + Some(PluginValue::List(call_args)), + ) => ( + String::from_utf8_lossy(class).into_owned(), + String::from_utf8_lossy(method).into_owned(), + call_args.clone(), + ), + ( + Some(PluginValue::String(class)), + Some(PluginValue::String(method)), + None | Some(PluginValue::Array(_)), + ) => ( + String::from_utf8_lossy(class).into_owned(), + String::from_utf8_lossy(method).into_owned(), + Vec::new(), + ), + other => { + return Err(runtime_throw(format!( + "__shirabeCallStatic expects a class name, a method name and an argument list, \ + got {other:?}" + ))); + } + }; + let name = format!("{class}::{method}"); + let string_arg = |position: usize| arg::<String>(&name, &call_args, position); + match (class.as_str(), method.as_str()) { + ("Composer\\Util\\Filesystem", "isLocalPath") => { + Ok(crate::util::Filesystem::is_local_path(&string_arg(0)?).to_plugin_value()) + } + ("Composer\\Util\\Filesystem", "getPlatformPath") => Ok(PluginValue::string( + crate::util::Filesystem::get_platform_path(&string_arg(0)?), + )), + ("Composer\\Util\\ProcessExecutor", "getTimeout") => { + Ok(crate::util::ProcessExecutor::get_timeout().to_plugin_value()) + } + ("Composer\\Util\\ProcessExecutor", "setTimeout") => { + crate::util::ProcessExecutor::set_timeout(arg::<i64>(&name, &call_args, 0)?); + Ok(PluginValue::Null) + } + // `escape(string|false|null $argument)` casts its argument to string first, which turns + // both of the non-string forms into the empty string. + ("Composer\\Util\\ProcessExecutor", "escape") => { + let argument = match call_args.first() { + None | Some(PluginValue::Null) | Some(PluginValue::Bool(false)) => String::new(), + _ => string_arg(0)?, + }; + Ok(PluginValue::string(crate::util::ProcessExecutor::escape( + &argument, + ))) + } + _ => Err(runtime_throw(format!( + "Shirabe does not support calling {name} from the plugin process yet" + ))), + } +} + fn dispatch_event_dispatcher_method( dispatcher: &std::rc::Rc< std::cell::RefCell<dyn crate::event_dispatcher::EventDispatcherInterface>, @@ -1326,6 +1614,50 @@ impl FromPluginArg for PackageInterfaceHandle { } } +/// Resolves an IO argument back to the Rust-side entity its proxy stub stands for. +impl FromPluginArg for std::rc::Rc<std::cell::RefCell<dyn IOInterface>> { + fn from_arg( + method: &str, + position: usize, + value: Option<&PluginValue>, + ) -> Result<Self, PhpThrow> { + match value { + Some(PluginValue::RustHandle(handle)) => { + match R_TABLE.with(|table| table.borrow().get(&handle.rhandle).cloned()) { + Some(RustEntity::Io(io)) => Ok(io), + _ => Err(runtime_throw(format!( + "{method} expects an IO handle, got Rust handle {}", + handle.rhandle + ))), + } + } + other => Err(arg_throw(method, position, "an IOInterface", other)), + } + } +} + +/// Resolves a process executor argument back to the Rust-side entity its proxy stub stands for. +impl FromPluginArg for std::rc::Rc<std::cell::RefCell<crate::util::ProcessExecutor>> { + fn from_arg( + method: &str, + position: usize, + value: Option<&PluginValue>, + ) -> Result<Self, PhpThrow> { + match value { + Some(PluginValue::RustHandle(handle)) => { + match R_TABLE.with(|table| table.borrow().get(&handle.rhandle).cloned()) { + Some(RustEntity::ProcessExecutor(process)) => Ok(process), + _ => Err(runtime_throw(format!( + "{method} expects a ProcessExecutor handle, got Rust handle {}", + handle.rhandle + ))), + } + } + other => Err(arg_throw(method, position, "a ProcessExecutor", other)), + } + } +} + /// Resolves a repository argument back to the Rust-side entity its proxy stub stands for. impl FromPluginArg for RepositoryInterfaceHandle { fn from_arg( diff --git a/crates/shirabe/src/util/process_executor.rs b/crates/shirabe/src/util/process_executor.rs index eae6692b..e1e7a8d8 100644 --- a/crates/shirabe/src/util/process_executor.rs +++ b/crates/shirabe/src/util/process_executor.rs @@ -618,37 +618,6 @@ impl ProcessExecutor { }) } - /// Plugin-facing counterpart of `execute_async`. Reached only when the RPC dispatcher - /// relays a plugin's `executeAsync()` call made on the `rust-proxy` `ProcessExecutor` stub - /// (a plugin obtained the handle via `Loop::getProcessExecutor()`) — see - /// docs/dev/plugin-class-classification.md, "Process: dual instantiation split by caller". - /// - /// A `Symfony\Component\Process\Process` cannot be reconstructed on the Rust side: its state - /// (the `proc_open()` resource, the OS pipes) belongs to whichever process calls `start()`, - /// and it refuses serialization outright. So unlike `execute_async`, this must not - /// spawn in Rust: the real `Process::start()` has to run in the PHP child, and the plugin's - /// `.then()` callback must receive that genuine PHP-side object. - // TODO(plugin): once the plugin RPC channel exists, acquire a permit from `self.semaphore` - // (shared with `execute_async`, so the combined job budget — including any shared cap - // with HttpDownloader — stays correct regardless of which path runs a given job), then send - // the spawn request to the PHP child over that channel instead of calling `Process::start()` - // here. Release the permit on the child's completion notification, not by polling a - // Rust-owned process handle. The return type below is provisional: the real deliverable is a - // handle to the live PHP-side Process object, not a `shirabe_symfony_process` `Process` - // value, so this signature will need to change once the RPC plumbing exists. - pub fn execute_async_php<C>( - &mut self, - _command: C, - _cwd: Option<&str>, - ) -> std::pin::Pin<Box<dyn std::future::Future<Output = anyhow::Result<()>>>> - where - C: IntoExecCommand, - { - todo!( - "forward the spawn to the PHP child over the plugin RPC channel and await its completion notification" - ) - } - fn output_handler( capture_output: bool, io: &mut Option<std::rc::Rc<std::cell::RefCell<dyn IOInterface>>>, @@ -855,6 +824,11 @@ impl ProcessExecutor { return false; } + // TODO(php-semantics): the shim's array_intersect returns a Vec and drops the keys PHP + // keeps from its first argument, so this compares values where PHP's `===` compares keys + // as well. PHP answers false for every pattern below, the early return above leaving `git` + // at index 0 so the intersection can never start where the pattern does; this answers + // true, which is what the patterns were written for. for git_cmd in Self::GIT_CMDS_NEED_GIT_DIR.iter() { let cmd_strs: Vec<String> = cmd.clone(); let git_cmd_strs: Vec<String> = git_cmd.iter().map(|s| s.to_string()).collect(); @@ -973,6 +947,7 @@ impl<const N: usize> IntoExecCommand for &[String; N] { /// | `execute($cmd)` | forward child output to STDOUT/STDERR (or the IO) | [`ProcessForwardOutput`] | `false` | /// | `execute($cmd, $out)` | assign captured output back to `$out` | `&mut String` / `&mut PhpMixed` | `true` | /// | `execute($cmd, $out)` where `$out` is unused | capture (suppress output) but discard it | `()` | `true` | +/// | `execute($cmd, $out)` from a plugin | capture, recording whether it was assigned | `&mut Option<String>` | `true` | /// | `execute($cmd, $cb)` | drive the child through the callback | `Box<dyn FnMut(&str, &str) -> bool>` | `false` | /// /// `capture_output` maps to PHP's `$this->captureOutput` (`func_num_args() > 3` in `doExecute`): when @@ -1058,6 +1033,23 @@ impl<'a> IntoExecOutput<'a> for &'a mut String { } } +/// `execute($cmd, $out)` reached over the plugin RPC boundary, where the caller has to know whether +/// the output was assigned at all rather than just what it is: PHP leaves `$output` untouched when +/// the child is signaled, and `None` reproduces that by sending no value back across the boundary. +impl<'a> IntoExecOutput<'a> for &'a mut Option<String> { + fn capture_output(&self) -> bool { + true + } + + fn to_callback(self) -> anyhow::Result<Box<dyn FnMut(&str, &str) -> bool>, Self> { + Err(self) + } + + fn write_back(&mut self, value: String) { + **self = Some(value); + } +} + /// `execute($cmd, $cb)` where `$cb` is callable: the callback is passed straight to `Process::run` /// as the output handler, so the caller drives the child's output itself (e.g. `Svn`'s streaming /// handler). The `bool` return mirrors Symfony's ignored callback return value. diff --git a/crates/shirabe/tests/plugin/e2e_process_executor_test.rs b/crates/shirabe/tests/plugin/e2e_process_executor_test.rs new file mode 100644 index 00000000..e1bb2aa6 --- /dev/null +++ b/crates/shirabe/tests/plugin/e2e_process_executor_test.rs @@ -0,0 +1,84 @@ +//! ProcessExecutor E2E compatibility check: upstream Composer and Shirabe each install a fixture +//! project whose plugin builds its own `ProcessExecutor` and writes what every call on it reports +//! to a trace file. Upstream has no test that drives a process executor from plugin code, so the +//! whole fixture is Shirabe-authored (`fixtures/e2e-process-executor/`) and nothing has to be +//! fetched; the test skips only while the PHP runtime or the Composer checkout is missing. + +use crate::e2e_extension_installer_test::{copy_dir, upstream_composer_bin}; +use crate::php_worker::{lock_php_worker, php_runtime_available}; +use std::path::{Path, PathBuf}; +use tempfile::TempDir; + +fn fixture_dir() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/plugin/fixtures/e2e-process-executor") +} + +struct Run { + exit_code: i32, + trace: String, +} + +/// Runs `install` in a fresh copy of the fixture and returns the exit code with the plugin's trace. +fn install(program: &str, prefix_args: &[&str]) -> Run { + let work = TempDir::new().unwrap(); + copy_dir(&fixture_dir(), work.path()); + let project = work.path().join("project"); + let output = std::process::Command::new(program) + .args(prefix_args) + .arg("install") + .current_dir(&project) + .env("COMPOSER_HOME", work.path().join("home")) + .env("COMPOSER_CACHE_DIR", work.path().join("cache")) + .env("COMPOSER_NO_INTERACTION", "1") + .env("COLUMNS", "120") + .env("LINES", "30") + .output() + .unwrap(); + Run { + exit_code: output.status.code().unwrap_or(-1), + trace: std::fs::read_to_string(project.join("process-executor-trace.txt")) + .unwrap_or_default(), + } +} + +#[test] +fn test_plugin_owned_process_executor_matches_upstream_composer() { + if !php_runtime_available() { + return; + } + let Some(composer_bin) = upstream_composer_bin() else { + return; + }; + let _worker = lock_php_worker(); + let composer_bin = composer_bin.to_str().unwrap().to_string(); + + let upstream = install("php", &[composer_bin.as_str()]); + let shirabe = install(env!("CARGO_BIN_EXE_shirabe"), &[]); + + assert_eq!(0, upstream.exit_code, "upstream install must succeed"); + assert_eq!(upstream.exit_code, shirabe.exit_code); + assert_eq!(upstream.trace, shirabe.trace); + + // Pinned as well as compared, so a run where neither side wrote a trace cannot pass. The + // timeout is the project's `process-timeout`, which is what makes it evidence that both + // worlds read one value rather than each holding its own default. + assert_eq!( + "\ +event=post-update-cmd +timeout=42 +timeout-after-set=7 +capture code=0 output=\"captured\\n\" error=\"\" +list code=0 output=\"from a list\\n\" +failing code=3 output=\"out\\n\" error=\"err\\n\" +forwarded code=0 file=\"forwarded\" +callback code=0 seen=[\"out:through-a-callback\"] argument=true +cwd code=0 basename=\"vendor\" +splitLines code=0 lines=[\"x\",\"y\"] empty=[] +escape=\"'a b'\\\\''c'\" +requiresGitDirEnv status=false +maxJobs=ok +filesystem normalizePath=\"\\/a\\/c\" isLocalPath=true getPlatformPath=\"\\/a\\/b\" +", + upstream.trace + ); +} diff --git a/crates/shirabe/tests/plugin/fixtures/e2e-process-executor/plugin/composer.json b/crates/shirabe/tests/plugin/fixtures/e2e-process-executor/plugin/composer.json new file mode 100644 index 00000000..5d5609c1 --- /dev/null +++ b/crates/shirabe/tests/plugin/fixtures/e2e-process-executor/plugin/composer.json @@ -0,0 +1,17 @@ +{ + "name": "shirabe-test/process-executor-probe", + "version": "1.0.0", + "type": "composer-plugin", + "description": "Fixture plugin driving a ProcessExecutor it constructs itself.", + "autoload": { + "psr-4": { + "ShirabeTest\\ProcessExecutor\\": "src/" + } + }, + "require": { + "composer-plugin-api": "^2.0" + }, + "extra": { + "class": "ShirabeTest\\ProcessExecutor\\Plugin" + } +} diff --git a/crates/shirabe/tests/plugin/fixtures/e2e-process-executor/plugin/src/Plugin.php b/crates/shirabe/tests/plugin/fixtures/e2e-process-executor/plugin/src/Plugin.php new file mode 100644 index 00000000..1f58a11d --- /dev/null +++ b/crates/shirabe/tests/plugin/fixtures/e2e-process-executor/plugin/src/Plugin.php @@ -0,0 +1,117 @@ +<?php + +namespace ShirabeTest\ProcessExecutor; + +use Composer\Composer; +use Composer\EventDispatcher\EventSubscriberInterface; +use Composer\IO\IOInterface; +use Composer\Plugin\PluginInterface; +use Composer\Script\Event; +use Composer\Script\ScriptEvents; +use Composer\Util\Filesystem; +use Composer\Util\ProcessExecutor; + +/** + * Drives a ProcessExecutor the plugin constructs itself and appends what every call reports to + * process-executor-trace.txt, so the whole surface can be compared line by line between + * implementations: the timeout shared with the rest of the run, both command forms, the three + * ways the second argument is treated, the error output, and a Filesystem built on the executor. + */ +class Plugin implements PluginInterface, EventSubscriberInterface +{ + /** @var IOInterface */ + private $io; + + public function activate(Composer $composer, IOInterface $io): void + { + $this->io = $io; + } + + public function deactivate(Composer $composer, IOInterface $io): void + { + } + + public function uninstall(Composer $composer, IOInterface $io): void + { + } + + public static function getSubscribedEvents() + { + // Whether an install resolves or replays a lock file decides which of the two fires, so + // both are subscribed and the trace records the one that ran. + return [ + ScriptEvents::POST_INSTALL_CMD => 'onPostCommand', + ScriptEvents::POST_UPDATE_CMD => 'onPostCommand', + ]; + } + + public function onPostCommand(Event $event): void + { + $process = new ProcessExecutor($this->io); + $lines = ['event=' . $event->getName()]; + + // The timeout is process-wide state Composer seeds from the config, so both worlds have + // to report the value this project asked for and to observe each other's writes. + $original = ProcessExecutor::getTimeout(); + $lines[] = 'timeout=' . $original; + ProcessExecutor::setTimeout(7); + $lines[] = 'timeout-after-set=' . ProcessExecutor::getTimeout(); + ProcessExecutor::setTimeout($original); + + $code = $process->execute('echo captured', $captured); + $lines[] = 'capture code=' . $code . ' output=' . json_encode($captured) + . ' error=' . json_encode($process->getErrorOutput()); + + $code = $process->execute(['echo', 'from', 'a', 'list'], $listed); + $lines[] = 'list code=' . $code . ' output=' . json_encode($listed); + + $code = $process->execute('echo out; echo err 1>&2; exit 3', $failed); + $lines[] = 'failing code=' . $code . ' output=' . json_encode($failed) + . ' error=' . json_encode($process->getErrorOutput()); + + // Without a second argument the child's output is forwarded rather than captured, which + // is a different branch of the same method; the redirection keeps it out of the terminal + // so the trace stays the only thing under comparison. + $code = $process->execute('echo forwarded > forwarded.txt'); + $lines[] = 'forwarded code=' . $code + . ' file=' . json_encode(trim((string) @file_get_contents('forwarded.txt'))); + + // A callable second argument drives the child's output itself and is never assigned to. + $seen = []; + $callback = static function (string $type, string $buffer) use (&$seen): void { + $seen[] = $type . ':' . trim($buffer); + }; + $code = $process->execute('echo through-a-callback', $callback); + $lines[] = 'callback code=' . $code . ' seen=' . json_encode($seen) + . ' argument=' . json_encode(\is_callable($callback)); + + $code = $process->execute('pwd', $cwdOutput, 'vendor'); + $lines[] = 'cwd code=' . $code . ' basename=' . json_encode(basename(trim((string) $cwdOutput))); + + $code = $process->execute('echo x; echo y', $multiline); + $lines[] = 'splitLines code=' . $code + . ' lines=' . json_encode($process->splitLines($multiline)) + . ' empty=' . json_encode($process->splitLines(null)); + + $lines[] = 'escape=' . json_encode(ProcessExecutor::escape("a b'c")); + // TODO(php-semantics): a command matching GIT_CMDS_NEED_GIT_DIR has no agreed value to + // compare. array_intersect() keeps its first argument's keys and `===` compares an + // array's keys too, so Composer answers false for those patterns as well; the shim's + // array_intersect drops the keys and Shirabe answers true. + $lines[] = 'requiresGitDirEnv status=' + . json_encode($process->requiresGitDirEnv('git status')); + + $process->setMaxJobs(4); + $process->resetMaxJobs(); + $lines[] = 'maxJobs=ok'; + + // The executor is a constructor argument of other Composer utilities, so a plugin-built + // one has to be accepted wherever the real class is. + $filesystem = new Filesystem($process); + $lines[] = 'filesystem normalizePath=' . json_encode($filesystem->normalizePath('/a/b/../c')) + . ' isLocalPath=' . json_encode(Filesystem::isLocalPath('/a/b')) + . ' getPlatformPath=' . json_encode(Filesystem::getPlatformPath('file:///a/b')); + + file_put_contents('process-executor-trace.txt', implode("\n", $lines) . "\n"); + } +} diff --git a/crates/shirabe/tests/plugin/fixtures/e2e-process-executor/project/composer.json b/crates/shirabe/tests/plugin/fixtures/e2e-process-executor/project/composer.json new file mode 100644 index 00000000..61ca9a5a --- /dev/null +++ b/crates/shirabe/tests/plugin/fixtures/e2e-process-executor/project/composer.json @@ -0,0 +1,25 @@ +{ + "name": "shirabe/e2e-process-executor", + "description": "E2E fixture project: record what a plugin's own ProcessExecutor reports.", + "repositories": [ + { + "type": "path", + "url": "../plugin", + "options": { + "symlink": false + } + }, + { + "packagist.org": false + } + ], + "require": { + "shirabe-test/process-executor-probe": "1.0.0" + }, + "config": { + "process-timeout": 42, + "allow-plugins": { + "shirabe-test/process-executor-probe": true + } + } +} diff --git a/crates/shirabe/tests/plugin/main.rs b/crates/shirabe/tests/plugin/main.rs index 000ca3a9..35f5beb5 100644 --- a/crates/shirabe/tests/plugin/main.rs +++ b/crates/shirabe/tests/plugin/main.rs @@ -12,6 +12,7 @@ mod e2e_installer_test; mod e2e_installers_test; mod e2e_normalize_test; mod e2e_package_event_test; +mod e2e_process_executor_test; mod e2e_script_command_test; mod e2e_script_event_test; mod plugin_installer_test; diff --git a/docs/dev/php-rpc.md b/docs/dev/php-rpc.md index eb93db19..aebf0097 100644 --- a/docs/dev/php-rpc.md +++ b/docs/dev/php-rpc.md @@ -184,10 +184,24 @@ Notable internal helpers: The runtime service endpoint (handle 0) answers `__shirabe_find_file` (autoload lookups), `__shirabe_run_rust_command` — the reverse half of the two-world command split: a `\Shirabe\RustCommandStub` forwards its stringified input here and the built-in command runs in -the Rust process, against the Rust-side application state — and `__shirabeConstruct`, which +the Rust process, against the Rust-side application state — `__shirabeConstruct`, which allocates the Rust entity behind a `new SomeProxiedClass(...)` written by plugin code and -answers with `[rhandle, epoch]`. Classes whose entity Rust cannot build are an explicit error -naming the class. +answers with `[rhandle, epoch]`, and `__shirabeCallStatic`, which takes `[class, method, args]` +and runs a static method no handle identifies a receiver for. Classes and static methods Rust +cannot answer are an explicit error naming them. + +### By-ref parameters + +`out_param_positions` names the by-ref parameters of the called method, as the caller declared +them; the answering `Return` carries `out_params`, a map from the same positions to the value +each parameter holds afterwards, and the caller assigns those back into its own variables. A +position the answer leaves out was never assigned to, which is what PHP does with an untouched +by-ref parameter — so an out-param is genuinely optional rather than defaulting to null. + +A generated stub fills both halves mechanically from the real signature +(`docs/dev/plugin-stub-generation.md`). `Composer\Util\ProcessExecutor::execute` is the method +this exists for: it is the only by-ref parameter on Composer's public surface that belongs to a +proxied class. ## Proxy stubs, runtime classes and guards diff --git a/docs/dev/plugin-class-classification.md b/docs/dev/plugin-class-classification.md index 1c7aec09..516cf226 100644 --- a/docs/dev/plugin-class-classification.md +++ b/docs/dev/plugin-class-classification.md @@ -139,11 +139,15 @@ design-sensitive. The class writes to `static` properties. Sub-classified by the disposition list (see exception lists): `memo-cache` (pure memoization, each world may compute its own: `Git::$version`, `Platform::$isDocker`, …), `seed-once` -(copied from the Rust side once at child startup: -`ProcessExecutor::$timeout`, which Composer seeds from config), -`needs-sync` (genuinely shared process state: `Platform`'s env table), or -`needs-review` (default for newly appearing ones — the run fails loudly -until a human files it). +(copied from the Rust side once at child startup; nothing is filed this way +today), `needs-sync` (genuinely shared process state: `Platform`'s env table, +`ProcessExecutor::$timeout`), or `needs-review` (default for newly appearing +ones — the run fails loudly until a human files it). + +A `needs-sync` static is not actually synced by copying it: the child holds no +copy at all, because the stub forwards the accessors that read and write it +(`__shirabeCallStatic`, see `docs/dev/plugin-stub-generation.md`). The +disposition records that the state is shared, not the mechanism. #### throwable @@ -369,23 +373,35 @@ Earlier design analysis assumed no public getter returns a `Composer::getLoop()` → `Loop::getProcessExecutor(): ?ProcessExecutor` / `Loop::getHttpDownloader(): HttpDownloader` make both reachable. The graph-owned `ProcessExecutor` instance must therefore be proxied (its job -queue is driven by the Rust loop), while the design intent — plugins using -it as a stateless utility — survives only for plugin-`new`ed instances. -This is exactly the dual-instantiation situation the -`plugin-constructible` attribute exists to surface. +queue is driven by the Rust loop), which is the dual-instantiation situation +the `plugin-constructible` attribute exists to surface. + +`ProcessExecutor` is one class in one category for both routes: it is a +`rust-proxy` stub, and a plugin-`new`ed instance allocates a Rust-side entity +of its own rather than a second, unconnected executor. That is what keeps +`$timeout` — process-wide state Composer seeds from `process-timeout` and +`RunScriptCommand` rewrites mid-run — readable and writable from both worlds +through one value, and it is what lets the executor stay a constructor +argument of the classes that take one (`new Filesystem($process)` is what +plugins actually write). Children are spawned in the Rust process either way, +which is also correct for stdio: the worker inherits the Rust process's +standard streams, so both worlds see the same terminal. -Both routes raise an explicit error today, and which one gets a story first -is still open. The graph-owned instances cannot be obtained at all: the -`Composer` proxy answers `getLoop()` with an explicit error, and `Loop`, -`ProcessExecutor` and `HttpDownloader` are guarded classes, so a -plugin-`new`ed instance is an explicit error too rather than a second -instance the Rust side never sees. +What stays an explicit error is the async surface — `executeAsync()`, +`wait()`, `enableAsync()`, `countActiveJobs()` — for two reasons that outlive +the choice of category. `executeAsync()` resolves its promise with a +`Symfony\Component\Process\Process`, whose state is the `proc_open()` +resource and OS pipes of whichever process called `start()`, so a Rust-side +spawn has no such object to hand back; and driving it needs a promise +representation that crosses the boundary unresolved, which the execution model +does not have. Neither is specific to how the executor is obtained: a plugin +can reach the async path on an instance of its own, since `enableAsync()`, +`wait()` and `countActiveJobs()` are all public (`@internal` is a docblock +note). -An earlier argument for leaving plugin-`new`ed instances alone does not -hold: `executeAsync()` does throw a `LogicException` unless `enableAsync()` -ran first, but `enableAsync()`, `wait()` and `countActiveJobs()` are all -public — `@internal` is a docblock note — so nothing stops a plugin from -driving its own instance through the async path. +`HttpDownloader` and `Loop` remain guarded, so `Composer::getLoop()` is still +an explicit error and the graph's own executor is not reachable yet. Serving +it needs only a `Loop` stub, since the executor's surface is already served. ### Dual instantiation @@ -406,39 +422,24 @@ stub constructor forwards to `__shirabeConstruct`, the Rust side allocates the entity and answers with its handle, and the plugin-`new`ed object is then the same entity the graph sees. It is filled in per class, driven by the explicit errors real plugins hit — today `Package`, `CompletePackage`, -the three alias packages, the five solver operations and -`Composer\Util\Filesystem` can be built this way, the last one only without -a `ProcessExecutor` argument (that class has no stub, so the argument could -only be an instance the Rust side never sees). Every other proxied class -answers with an explicit error naming it. +the three alias packages, the five solver operations, +`Composer\Util\Filesystem` (with or without its `ProcessExecutor` argument) +and `Composer\Util\ProcessExecutor` can be built this way. Every other +proxied class answers with an explicit error naming it. -`JsonFile`, `ArrayLoader` and `ProcessExecutor` are still undecided, so the -classes above stay `unsupported` — but a guard now shadows each of them in -the child, so touching one is an explicit error instead of real code -running against a second instance. +`JsonFile` and `ArrayLoader` are still undecided, so the classes above stay +`unsupported` — but a guard now shadows each of them in the child, so +touching one is an explicit error instead of real code running against a +second instance. The demotion rule has not been taught about the construction stories: it demotes for constructing any `rust-proxy` service, whether or not that service can now be built. `Composer\Package\Archiver\ArchivableFilesFinder` -is `unsupported` for `new Filesystem` alone, which no longer forks any -state; promoting such a class means feeding the per-class construction -stories back into the rule. - -### Process: dual instantiation split by caller - -`ProcessExecutor::executeAsync()` resolves its promise with a -`Symfony\Component\Process\Process` instance, so it may cross the language -boundary despite being a wholesale-`php-native` vendor class. A `Process` -can't be reconstructed PHP-side from Rust-generated data because its state -is stored in a `resource` created by `proc_open()`. - -Proposed resolution (not adopted): split `ProcessExecutor`'s Rust -implementation by caller. -Rust-ported Composer code (`VersionGuesser`, `Git`, …) calls `execute_async()` -directly and spawns in Rust. A plugin holding a `ProcessExecutor` handle -(`Loop::getProcessExecutor()`) instead hits the `rust-proxy` stub's RPC entry, -which forwards the spawn to the PHP child so the real `Process::start()` runs -there. The plugin gets the genuine object, never a fake one. +is `unsupported` for `new Filesystem` alone, and the VCS/auth belt is +`unsupported` for `new ProcessExecutor` alone; neither forks any state any +more. Promoting them means feeding the per-class construction stories back +into the rule — and, for the belt, deciding `HttpDownloader` too, since +those classes take one and no route to it exists yet. ### Package and CompletePackage diff --git a/docs/dev/plugin-stub-generation.md b/docs/dev/plugin-stub-generation.md index e2b6d848..a14d7ce1 100644 --- a/docs/dev/plugin-stub-generation.md +++ b/docs/dev/plugin-stub-generation.md @@ -88,11 +88,32 @@ the generator's vendor directory or the classifier report is unavailable. the class's own public methods already cover their surface. * Methods returning `self`/`static` perform the RPC and then `return $this;` to preserve identity instead of round-tripping the handle. +* **By-ref parameters** are declared `&$name` on the stub as well. The call + carries their positions, and the answer carries the value each of them holds + afterwards, which the stub assigns back (see "By-ref parameters" in + `docs/dev/php-rpc.md`). A position the answer omits is left alone, so a + parameter the callee never assigned to keeps the value it had. +* **Arity** is reproduced when — and only when — the real method body calls + `func_num_args()`, in which case the stub trims the argument list to what the + caller actually passed instead of sending the declared defaults. Sending them + regardless would make the Rust side answer a call the plugin never made: + `ProcessExecutor::execute($cmd)` forwards the child's output, while + `execute($cmd, $out)` captures it, and only the argument count separates them. + A body calling `func_get_args()` fails generation instead. * **Class constants, static methods and public static properties** are materialized verbatim from the real source (they read no instance state and run locally in the worker), together with any non-public static helpers the methods call. Constants keep their declared visibility, so a non-public one stays unreadable from outside the stub as it is in the real class. +* A static method is materialized only when it can actually run in the worker. + One that reads a **static property** — whose value the Rust side owns, as + `ProcessExecutor::$timeout` does — or that reaches a class a **guard** + shadows there — as `ProcessExecutor::escape()` reaches `Composer\Util\Platform` + — forwards instead, through `__shirabeCallStatic` on handle 0, carrying a + comment that names the reason. The test covers the transitive closure of the + non-public static helpers the method calls, since those are materialized with + it. A forwarded static may not take by-ref parameters; that combination fails + generation. * **Instance properties** are not declared on the stub, whatever their visibility: they are entity state. Every root stub instead carries `__get`/`__set`/`__isset`/`__unset` forwarders, so each access reaches the @@ -137,8 +158,10 @@ Generation fails — instead of emitting something quietly wrong — on: * a target missing from the classifier report, classified other than `rust-proxy`/`contract`, or a report carrying violations, -* by-ref or variadic parameters (in constructors too), static interface - methods, magic methods other than `__toString`/`__clone`, +* variadic parameters (in constructors too), by-ref parameters in a + constructor or in a forwarded static method, `func_get_args()` in a forwarded + body, static interface methods, magic methods other than + `__toString`/`__clone`, * an omitted override diverging from the inherited stub signature, * a subclass target listed before its base class, or extending a class that is neither a target nor provided by `php/runtime/`, diff --git a/scripts/plugin-class-classifier/lists/static-state.list b/scripts/plugin-class-classifier/lists/static-state.list index a33faeec..86e38713 100644 --- a/scripts/plugin-class-classifier/lists/static-state.list +++ b/scripts/plugin-class-classifier/lists/static-state.list @@ -16,6 +16,6 @@ Composer\Util\Git memo-cache Composer\Util\Hg memo-cache Composer\Util\Http\ProxyManager memo-cache Composer\Util\Platform needs-sync -Composer\Util\ProcessExecutor seed-once +Composer\Util\ProcessExecutor needs-sync Composer\Util\Silencer memo-cache Composer\Util\Svn memo-cache diff --git a/scripts/plugin-stub-generator/generate-stubs b/scripts/plugin-stub-generator/generate-stubs index c58e6a92..4bcda8d7 100755 --- a/scripts/plugin-stub-generator/generate-stubs +++ b/scripts/plugin-stub-generator/generate-stubs @@ -104,7 +104,7 @@ foreach (array_keys($exempt) as $fqcn) { } try { - $generator = new Generator($composerRoot, $report, $targets, $runtimeProvided); + $generator = new Generator($composerRoot, $report, $targets, $runtimeProvided, $exemptions); $files = $generator->generate(); $guardGenerator = new GuardGenerator( $project, diff --git a/scripts/plugin-stub-generator/src/Generator.php b/scripts/plugin-stub-generator/src/Generator.php index 0b8ef037..fdaae5ee 100644 --- a/scripts/plugin-stub-generator/src/Generator.php +++ b/scripts/plugin-stub-generator/src/Generator.php @@ -4,14 +4,16 @@ declare(strict_types=1); namespace Shirabe\PluginStubGenerator; +use PhpParser\Node; use PhpParser\Node\Name; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Interface_; +use PhpParser\NodeFinder; /** * Emits the proxy stub files deterministically from the Composer sources and the classifier - * report. Anything the emitter cannot faithfully proxy (by-ref or variadic parameters, + * report. Anything the emitter cannot faithfully proxy (variadic parameters, `func_get_args()`, * magic methods, non-public constants) fails generation instead of degrading silently. */ final class Generator @@ -101,20 +103,32 @@ final class Generator /** @var array<string, true> */ private array $runtimeSet = []; + /** @var array<string, true> */ + private array $guardExemptSet = []; + + private NodeFinder $nodeFinder; + /** * @param list<string> $targets * @param list<string> $runtimeProvided FQCNs of the hand-written dual-mode classes under * php/runtime/; they may serve as stub base classes * but must never be generation targets themselves + * @param list<string> $guardExempt FQCNs the Rust side owns that the child still resolves + * to the real Composer class (guard-exemptions.list) */ public function __construct( string $composerRoot, private readonly Report $report, private readonly array $targets, private readonly array $runtimeProvided = [], + array $guardExempt = [], ) { $this->project = new Project($composerRoot); $this->printer = new NamePrinter(); + $this->nodeFinder = new NodeFinder(); + foreach ($guardExempt as $fqcn) { + $this->guardExemptSet[$fqcn] = true; + } foreach ($targets as $fqcn) { $this->targetSet[$fqcn] = true; } @@ -263,7 +277,10 @@ final class Generator $ownInstanceMethods[$name] = $method; } } - $staticMethods = $this->materializeStatics($file, $publicStatics, $nonPublicStatics); + [$staticMethods, $forwardedStatics] = $this->partitionStatics($file, $publicStatics, $nonPublicStatics); + foreach ($forwardedStatics as $name => [$method, $reason]) { + $staticMethods[] = $this->renderStaticForwarder($fqcn, $name, $method, $file, $reason); + } $surface = []; $emitted = []; @@ -394,34 +411,119 @@ final class Generator } /** - * Static methods read no instance state; their real implementation is materialized - * verbatim so they run locally in the worker. Non-public static helpers they call - * (through `self::`, `static::` or the class name) are materialized along with them. + * Splits the public static methods into the ones that run locally in the worker and the ones + * that have to forward. A static method reads no instance state, so its real implementation is + * materialized verbatim — unless it reaches something the worker does not have: a static + * property, whose value the Rust side owns, or a class a guard shadows there. Non-public + * static helpers a materialized method calls (through `self::`, `static::` or the class name) + * are materialized along with it, and count towards what it reaches. * * @param array<string, ClassMethod> $publicStatics * @param array<string, ClassMethod> $nonPublicStatics - * @return list<string> + * @return array{list<string>, array<string, array{ClassMethod, string}>} verbatim texts, and + * the methods to forward with the reason each of them cannot run locally */ - private function materializeStatics(SourceFile $file, array $publicStatics, array $nonPublicStatics): array + private function partitionStatics(SourceFile $file, array $publicStatics, array $nonPublicStatics): array { $texts = []; + $forwarded = []; foreach ($publicStatics as $name => $method) { + $blocker = $this->staticBlocker($file, $method, $nonPublicStatics); + if ($blocker !== null) { + $forwarded[$name] = [$method, $blocker]; + continue; + } $texts[$name] = $file->verbatim($method->getStartLine(), $method->getEndLine()); } $scan = array_values($texts); while ($scan !== []) { $text = array_shift($scan); - foreach ($nonPublicStatics as $name => $method) { + foreach ($this->calledHelpers($file, $text, $nonPublicStatics) as $name => $method) { if (isset($texts[$name])) { continue; } - $receiver = '(?:self|static|' . preg_quote($file->classLike->name?->toString() ?? '', '/') . ')'; - if (preg_match('/(?<![\w$])' . $receiver . '::' . preg_quote($name, '/') . '\s*\(/', $text) === 1) { - $scan[] = $texts[$name] = $file->verbatim($method->getStartLine(), $method->getEndLine()); + $scan[] = $texts[$name] = $file->verbatim($method->getStartLine(), $method->getEndLine()); + } + } + return [array_values($texts), $forwarded]; + } + + /** + * The non-public static helpers a method body calls by name. + * + * @param array<string, ClassMethod> $nonPublicStatics + * @return array<string, ClassMethod> + */ + private function calledHelpers(SourceFile $file, string $text, array $nonPublicStatics): array + { + $receiver = '(?:self|static|' . preg_quote($file->classLike->name?->toString() ?? '', '/') . ')'; + $found = []; + foreach ($nonPublicStatics as $name => $method) { + if (preg_match('/(?<![\w$])' . $receiver . '::' . preg_quote($name, '/') . '\s*\(/', $text) === 1) { + $found[$name] = $method; + } + } + return $found; + } + + /** + * Why a static method cannot be materialized into the worker, or null when it can. The answer + * covers the transitive closure of the non-public static helpers it calls, since those are + * materialized with it and reach whatever it reaches. + * + * @param array<string, ClassMethod> $nonPublicStatics + */ + private function staticBlocker(SourceFile $file, ClassMethod $method, array $nonPublicStatics): ?string + { + $closure = [$method]; + $seen = []; + $queue = [$method]; + while ($queue !== []) { + $current = array_shift($queue); + $text = $file->verbatim($current->getStartLine(), $current->getEndLine()); + foreach ($this->calledHelpers($file, $text, $nonPublicStatics) as $name => $helper) { + if (isset($seen[$name])) { + continue; + } + $seen[$name] = true; + $closure[] = $helper; + $queue[] = $helper; + } + } + foreach ($closure as $node) { + foreach ($this->nodeFinder->findInstanceOf($node, Node\Expr\StaticPropertyFetch::class) as $fetch) { + $property = $fetch->name instanceof Node\VarLikeIdentifier ? $fetch->name->toString() : ''; + return "reads the static property \$$property, whose value the Rust side owns"; + } + $references = array_merge( + $this->nodeFinder->findInstanceOf($node, Node\Expr\StaticCall::class), + $this->nodeFinder->findInstanceOf($node, Node\Expr\ClassConstFetch::class), + $this->nodeFinder->findInstanceOf($node, Node\Expr\New_::class), + ); + foreach ($references as $reference) { + if (!$reference->class instanceof Name) { + continue; + } + $fqcn = SourceFile::resolvedName($reference->class); + if ($this->isGuardedInChild($fqcn)) { + return "references $fqcn, which a guard shadows in the worker"; } } } - return array_values($texts); + return null; + } + + /** + * Whether the worker resolves this class to a guard rather than to executable code: the Rust + * side owns it and neither a stub, a runtime definition nor a guard exemption stands in for + * it, so materialized code reaching it would raise an explicit error at run time. + */ + private function isGuardedInChild(string $fqcn): bool + { + if (isset($this->targetSet[$fqcn]) || isset($this->runtimeSet[$fqcn]) || isset($this->guardExemptSet[$fqcn])) { + return false; + } + return in_array($this->report->category($fqcn), ['rust-proxy', 'rust-snapshot', 'unsupported'], true); } /** Interfaces implemented by the class, each interface preceding the ones it extends. */ @@ -457,13 +559,64 @@ final class Generator private function renderProxyMethod(string $fqcn, string $name, ClassMethod $method, SourceFile $target): string { + [$params, $args, $outPositions] = $this->renderParams($fqcn, $name, $method, $target); + $returnType = $this->printer->renderType($method->returnType, $target); + $signature = "public function $name(" . implode(', ', $params) . ')' + . ($returnType === '' ? '' : ": $returnType"); + $body = $this->renderForwardingBody( + "\\ShirabeRpcRuntime::callRust(\$this->__rhandle, '$name', %s%s)", + $args, + $outPositions, + $this->inspectsArgCount($fqcn, $name, $method), + $returnType, + ); + return " $signature\n {\n$body\n }"; + } + + /** + * The counterpart of renderProxyMethod for a static method that cannot be materialized: the + * call carries the class alongside the method name, since no handle identifies a receiver. + * `$reason` is why it cannot, and is emitted with it so the generated file explains itself. + */ + private function renderStaticForwarder( + string $fqcn, + string $name, + ClassMethod $method, + SourceFile $target, + string $reason, + ): string { + [$params, $args, $outPositions] = $this->renderParams($fqcn, $name, $method, $target); + if ($outPositions !== []) { + $this->errors[] = "$fqcn::$name: a forwarded static method cannot take by-ref parameters yet"; + } + $returnType = $this->printer->renderType($method->returnType, $target); + $signature = "public static function $name(" . implode(', ', $params) . ')' + . ($returnType === '' ? '' : ": $returnType"); + // self::class, not static::class: the forwarded statics stand for this class's own state, + // and a plugin subclass redeclaring it is not modelled on the Rust side. + $body = $this->renderForwardingBody( + "\\ShirabeRpcRuntime::callRust(0, '__shirabeCallStatic', [self::class, '$name', %s]%s)", + $args, + $outPositions, + $this->inspectsArgCount($fqcn, $name, $method), + $returnType, + ); + return " // Forwarded rather than materialized: it $reason.\n" + . " $signature\n {\n$body\n }"; + } + + /** + * The parameter list of a forwarded method and the argument expressions matching it. + * + * @return array{list<string>, list<string>, list<int>} parameters, arguments, by-ref positions + */ + private function renderParams(string $fqcn, string $name, ClassMethod $method, SourceFile $target): array + { $params = []; $args = []; - foreach ($method->params as $param) { + $outPositions = []; + foreach ($method->params as $position => $param) { $paramName = $param->var->name; - if ($param->byRef) { - $this->errors[] = "$fqcn::$name: by-ref parameter \$$paramName cannot be proxied yet"; - } if ($param->variadic) { $this->errors[] = "$fqcn::$name: variadic parameter \$$paramName cannot be proxied yet"; } @@ -471,6 +624,10 @@ final class Generator if ($param->type !== null) { $rendered = $this->printer->renderType($param->type, $target) . ' '; } + if ($param->byRef) { + $rendered .= '&'; + $outPositions[] = $position; + } $rendered .= '$' . $paramName; if ($param->default !== null) { $rendered .= ' = ' . $this->printer->renderExpr($param->default, $target); @@ -478,19 +635,85 @@ final class Generator $params[] = $rendered; $args[] = '$' . $paramName; } + return [$params, $args, $outPositions]; + } - $returnType = $this->printer->renderType($method->returnType, $target); - $signature = "public function $name(" . implode(', ', $params) . ')' - . ($returnType === '' ? '' : ": $returnType"); - $call = "\\ShirabeRpcRuntime::callRust(\$this->__rhandle, '$name', [" . implode(', ', $args) . '])'; - // `self`/`static` returns are fluent interfaces; the local stub itself is returned to - // preserve identity instead of round-tripping the handle. - $body = match ($returnType) { - 'void' => " $call;", - 'self', 'static' => " $call;\n return \$this;", - default => " return $call;", - }; - return " $signature\n {\n$body\n }"; + /** + * Whether the real method branches on how many arguments it was called with. The stub has to + * reproduce that arity across the boundary — sending every declared parameter would make the + * Rust side answer a call the plugin never made. + */ + private function inspectsArgCount(string $fqcn, string $name, ClassMethod $method): bool + { + $inspects = false; + foreach ($this->nodeFinder->findInstanceOf($method, Node\Expr\FuncCall::class) as $call) { + if (!$call->name instanceof Name) { + continue; + } + $function = strtolower($call->name->toString()); + if ($function === 'func_get_args') { + $this->errors[] = "$fqcn::$name: func_get_args() cannot be proxied yet"; + } + if ($function === 'func_num_args') { + $inspects = true; + } + } + return $inspects; + } + + /** + * The body every forwarding method shares: build the argument list, make the call, and copy + * each by-ref parameter back out of the response. + * + * `$callTemplate` takes the argument-list expression and the trailing arguments of + * `ShirabeRpcRuntime::callRust`, which only a by-ref parameter needs. + * + * @param list<string> $args + * @param list<int> $outPositions + */ + private function renderForwardingBody( + string $callTemplate, + array $args, + array $outPositions, + bool $arityAware, + string $returnType, + ): string { + $lines = []; + $argsExpr = '[' . implode(', ', $args) . ']'; + if ($arityAware) { + $lines[] = " \$__args = $argsExpr;"; + $lines[] = ' array_splice($__args, func_num_args());'; + $argsExpr = '$__args'; + } + if ($outPositions === []) { + $call = sprintf($callTemplate, $argsExpr, ''); + // `self`/`static` returns are fluent interfaces; the local stub itself is returned to + // preserve identity instead of round-tripping the handle. + $lines[] = match ($returnType) { + 'void' => " $call;", + 'self', 'static' => " $call;\n return \$this;", + default => " return $call;", + }; + return implode("\n", $lines); + } + + $tail = ', [' . implode(', ', $outPositions) . '], $__out'; + $call = sprintf($callTemplate, $argsExpr, $tail); + $lines[] = ' $__out = [];'; + $lines[] = $returnType === 'void' ? " $call;" : " \$__result = $call;"; + foreach ($outPositions as $position) { + // Absent when the callee left the parameter alone, which PHP reproduces by simply + // not writing to it. + $lines[] = " if (array_key_exists($position, \$__out)) {"; + $lines[] = " {$args[$position]} = \$__out[$position];"; + $lines[] = ' }'; + } + if ($returnType === 'self' || $returnType === 'static') { + $lines[] = ' return $this;'; + } elseif ($returnType !== 'void') { + $lines[] = ' return $__result;'; + } + return implode("\n", $lines); } /** diff --git a/scripts/plugin-stub-generator/targets.list b/scripts/plugin-stub-generator/targets.list index c013c3c2..2c9609ce 100644 --- a/scripts/plugin-stub-generator/targets.list +++ b/scripts/plugin-stub-generator/targets.list @@ -34,3 +34,4 @@ Composer\DependencyResolver\Operation\UninstallOperation Composer\DependencyResolver\Operation\MarkAliasInstalledOperation Composer\DependencyResolver\Operation\MarkAliasUninstalledOperation Composer\Util\Filesystem +Composer\Util\ProcessExecutor |