diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-08-07 20:51:59 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-08-07 20:51:59 +0900 |
| commit | 18a37a098157a98edbc8473e9c0d8dff3e8a88fa (patch) | |
| tree | 471682a4493fc8ddc1577a6f1e3c3d9c203ebf2c | |
| parent | cd9e4a2b67cdea258e1daa2d9d830b7643bc19bb (diff) | |
| download | php-shirabe-18a37a098157a98edbc8473e9c0d8dff3e8a88fa.tar.gz php-shirabe-18a37a098157a98edbc8473e9c0d8dff3e8a88fa.tar.zst php-shirabe-18a37a098157a98edbc8473e9c0d8dff3e8a88fa.zip | |
fix(plugin): make proxy stub property access an explicit error
A proxy stub declares none of the real class's instance properties, and the
`__get`/`__set` forwarders were emitted only for classes that declare a public
one. Every other property access therefore got PHP's own answer for an
undeclared property — null on a read, a dynamic property on a write — so plugin
code reading state the entity holds ran on with null and failed somewhere else
entirely, or not at all.
Emit the forwarders on every root stub, add `__isset`/`__unset` alongside them
so `isset()` cannot answer false silently either, and serve all four from one
dispatcher that answers the state the Rust-side entity exposes and rejects every
other name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
12 files changed, 247 insertions, 55 deletions
diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/Config.php b/crates/shirabe-php-rpc/php/stubs/Composer/Config.php index bccb8dcb..2424e5e3 100644 --- a/crates/shirabe-php-rpc/php/stubs/Composer/Config.php +++ b/crates/shirabe-php-rpc/php/stubs/Composer/Config.php @@ -51,6 +51,26 @@ class Config implements \ShirabeRustStub \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(bool $useEnvironment = true, ?string $baseDir = null) { [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust(0, '__shirabeConstruct', [static::class, [$useEnvironment, $baseDir]]); diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/Downloader/DownloadManager.php b/crates/shirabe-php-rpc/php/stubs/Composer/Downloader/DownloadManager.php index 3828ac4a..198bb4d0 100644 --- a/crates/shirabe-php-rpc/php/stubs/Composer/Downloader/DownloadManager.php +++ b/crates/shirabe-php-rpc/php/stubs/Composer/Downloader/DownloadManager.php @@ -51,6 +51,26 @@ class DownloadManager implements \ShirabeRustStub \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, bool $preferSource = false, ?Filesystem $filesystem = null) { [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust(0, '__shirabeConstruct', [static::class, [$io, $preferSource, $filesystem]]); diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/EventDispatcher/EventDispatcher.php b/crates/shirabe-php-rpc/php/stubs/Composer/EventDispatcher/EventDispatcher.php index e2369ed2..be9e8f8d 100644 --- a/crates/shirabe-php-rpc/php/stubs/Composer/EventDispatcher/EventDispatcher.php +++ b/crates/shirabe-php-rpc/php/stubs/Composer/EventDispatcher/EventDispatcher.php @@ -53,6 +53,26 @@ class EventDispatcher implements \ShirabeRustStub \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(PartialComposer $composer, IOInterface $io, ?ProcessExecutor $process = null) { [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust(0, '__shirabeConstruct', [static::class, [$composer, $io, $process]]); diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/IO/BaseIO.php b/crates/shirabe-php-rpc/php/stubs/Composer/IO/BaseIO.php index ecb4db2f..e32e8ee3 100644 --- a/crates/shirabe-php-rpc/php/stubs/Composer/IO/BaseIO.php +++ b/crates/shirabe-php-rpc/php/stubs/Composer/IO/BaseIO.php @@ -48,6 +48,26 @@ abstract class BaseIO implements IOInterface, \ShirabeRustStub \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() { [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust(0, '__shirabeConstruct', [static::class, []]); diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/Installer/InstallationManager.php b/crates/shirabe-php-rpc/php/stubs/Composer/Installer/InstallationManager.php index 7f41f6b9..6d3e9cf9 100644 --- a/crates/shirabe-php-rpc/php/stubs/Composer/Installer/InstallationManager.php +++ b/crates/shirabe-php-rpc/php/stubs/Composer/Installer/InstallationManager.php @@ -58,6 +58,26 @@ class InstallationManager implements \ShirabeRustStub \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(Loop $loop, IOInterface $io, ?EventDispatcher $eventDispatcher = null) { [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust(0, '__shirabeConstruct', [static::class, [$loop, $io, $eventDispatcher]]); diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/Package/BasePackage.php b/crates/shirabe-php-rpc/php/stubs/Composer/Package/BasePackage.php index 9fca286a..65eebd56 100644 --- a/crates/shirabe-php-rpc/php/stubs/Composer/Package/BasePackage.php +++ b/crates/shirabe-php-rpc/php/stubs/Composer/Package/BasePackage.php @@ -48,6 +48,26 @@ abstract class BasePackage implements PackageInterface, \ShirabeRustStub \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(string $name) { [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust(0, '__shirabeConstruct', [static::class, [$name]]); @@ -76,17 +96,6 @@ abstract class BasePackage implements PackageInterface, \ShirabeRustStub ]; public static $stabilities = self::STABILITIES; - /** The real class declares public properties; every access forwards to the entity. */ - 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 static function packageNameToRegexp(string $allowPattern, string $wrap = '{^%s$}i'): string { $cleanedAllowPattern = str_replace('\\*', '.*', preg_quote($allowPattern)); diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/PartialComposer.php b/crates/shirabe-php-rpc/php/stubs/Composer/PartialComposer.php index a48133cb..6f90113a 100644 --- a/crates/shirabe-php-rpc/php/stubs/Composer/PartialComposer.php +++ b/crates/shirabe-php-rpc/php/stubs/Composer/PartialComposer.php @@ -52,6 +52,26 @@ class PartialComposer implements \ShirabeRustStub \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() { [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust(0, '__shirabeConstruct', [static::class, []]); diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/Repository/ArrayRepository.php b/crates/shirabe-php-rpc/php/stubs/Composer/Repository/ArrayRepository.php index a5358ff2..8210031a 100644 --- a/crates/shirabe-php-rpc/php/stubs/Composer/Repository/ArrayRepository.php +++ b/crates/shirabe-php-rpc/php/stubs/Composer/Repository/ArrayRepository.php @@ -48,6 +48,26 @@ class ArrayRepository implements RepositoryInterface, \ShirabeRustStub \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(array $packages = []) { [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust(0, '__shirabeConstruct', [static::class, [$packages]]); diff --git a/crates/shirabe-php-rpc/php/stubs/Composer/Repository/RepositoryManager.php b/crates/shirabe-php-rpc/php/stubs/Composer/Repository/RepositoryManager.php index 9f3567d3..f2af01e5 100644 --- a/crates/shirabe-php-rpc/php/stubs/Composer/Repository/RepositoryManager.php +++ b/crates/shirabe-php-rpc/php/stubs/Composer/Repository/RepositoryManager.php @@ -53,6 +53,26 @@ class RepositoryManager implements \ShirabeRustStub \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, Config $config, HttpDownloader $httpDownloader, ?EventDispatcher $eventDispatcher = null, ?ProcessExecutor $process = null) { [$this->__rhandle, $this->__epoch] = \ShirabeRpcRuntime::callRust(0, '__shirabeConstruct', [static::class, [$io, $config, $httpDownloader, $eventDispatcher, $process]]); diff --git a/crates/shirabe/src/plugin/php_plugin_proxy.rs b/crates/shirabe/src/plugin/php_plugin_proxy.rs index bd40b358..541718af 100644 --- a/crates/shirabe/src/plugin/php_plugin_proxy.rs +++ b/crates/shirabe/src/plugin/php_plugin_proxy.rs @@ -305,6 +305,12 @@ impl RustMethodDispatcher for PluginRpcDispatcher<'_> { None => Err(runtime_throw(format!("unknown Rust handle {rhandle}"))), }; } + if matches!(method_name, "__get" | "__set" | "__isset" | "__unset") { + return match entity { + Some(entity) => dispatch_property_access(&entity, method_name, &args), + None => Err(runtime_throw(format!("unknown Rust handle {rhandle}"))), + }; + } match entity { Some(RustEntity::Io(io)) => dispatch_io_method(&io, method_name, &args), Some(RustEntity::Composer(composer)) => { @@ -458,6 +464,47 @@ fn clone_entity(entity: &RustEntity) -> Result<PluginValue, PhpThrow> { ])) } +/// Serves the `__get`/`__set`/`__isset`/`__unset` forwarders every proxy stub carries. +fn dispatch_property_access( + entity: &RustEntity, + method_name: &str, + args: &[PluginValue], +) -> Result<PluginValue, PhpThrow> { + let property = required_string_arg(method_name, args.first())?; + // `BasePackage::$id` is the one instance property the entities expose so far. + if let RustEntity::Package(package) = entity + && property == "id" + { + return match method_name { + "__get" => Ok(PluginValue::Int( + package.borrow().as_package_interface().get_id(), + )), + "__isset" => Ok(PluginValue::Bool(true)), + "__set" => { + let id = match args.get(1) { + Some(PluginValue::Int(id)) => *id, + other => { + return Err(runtime_throw(format!( + "the package property `id` takes an int, got {other:?}" + ))); + } + }; + package.borrow_mut().as_package_interface_mut().set_id(id); + Ok(PluginValue::Null) + } + _ => Err(runtime_throw( + "the package property `id` cannot be unset over RPC".to_string(), + )), + }; + } + // TODO(plugin): the instance properties the proxied classes expose are widened on demand, + // driven by these explicit errors from real plugins. Each one has to decide how the state + // the real class keeps in that property is served from the Rust-side entity. + Err(runtime_throw(format!( + "the property `{property}` is not available over RPC yet" + ))) +} + fn dispatch_composer_method( composer: &ComposerHandle, method_name: &str, @@ -1552,32 +1599,6 @@ fn dispatch_package_method( } return Ok(PluginValue::Null); } - // `BasePackage::$id` is the one public property of the package classes, so the stub's - // property forwarders only ever carry it. - "__get" | "__set" => { - let property = required_string_arg(method_name, args.first())?; - if property != "id" { - return Err(runtime_throw(format!( - "the package property `{property}` is not available over RPC" - ))); - } - return if method_name == "__get" { - Ok(PluginValue::Int( - package.borrow().as_package_interface().get_id(), - )) - } else { - let id = match args.get(1) { - Some(PluginValue::Int(id)) => *id, - other => { - return Err(runtime_throw(format!( - "the package property `id` takes an int, got {other:?}" - ))); - } - }; - package.borrow_mut().as_package_interface_mut().set_id(id); - Ok(PluginValue::Null) - }; - } "equals" => { let other = package_from_arg(method_name, args.first())?; let this = PackageInterfaceHandle::from_rc_unchecked(package.clone()); diff --git a/docs/dev/plugin-stub-generation.md b/docs/dev/plugin-stub-generation.md index 27835921..9743534a 100644 --- a/docs/dev/plugin-stub-generation.md +++ b/docs/dev/plugin-stub-generation.md @@ -75,9 +75,12 @@ the generator's vendor directory or the classifier report is unavailable. 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. -* **Public instance properties** are not declared on the stub; `__get`/`__set` - forwarders carry every access (including dynamic-property writes) to the - Rust side, where an unsupported name is an explicit error. +* **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 + Rust side, where an unsupported name is an explicit error. They are emitted + unconditionally because PHP's own answer for an undeclared property — null on + a read, a dynamic property on a write, false on `isset()` — is silent. * **`__toString`** is forwarded like any other method. **`__clone`** is part of the boilerplate on every stub, whether or not the real class declares one: PHP has already copied the stub by the time it runs, so the copy asks the diff --git a/scripts/plugin-stub-generator/src/Generator.php b/scripts/plugin-stub-generator/src/Generator.php index 395155dd..12967787 100644 --- a/scripts/plugin-stub-generator/src/Generator.php +++ b/scripts/plugin-stub-generator/src/Generator.php @@ -12,8 +12,7 @@ use PhpParser\Node\Stmt\Interface_; /** * 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, - * magic methods, public properties, non-public constants) fails generation instead of - * degrading silently. + * magic methods, non-public constants) fails generation instead of degrading silently. */ final class Generator { @@ -59,7 +58,6 @@ final class Generator PHP; private const PROPERTY_FORWARDERS = <<<'PHP' - /** The real class declares public properties; every access forwards to the entity. */ public function __get($name) { return \ShirabeRpcRuntime::callRust($this->__rhandle, '__get', [$name]); @@ -69,6 +67,16 @@ final class Generator { \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]); + } PHP; private Project $project; @@ -202,20 +210,13 @@ final class Generator $isRoot = true; } - $hasPublicInstanceProperties = false; + // Instance properties are entity state, so the stub declares none of them. $staticProperties = []; foreach ($class->getProperties() as $property) { - if (!$property->isPublic()) { - continue; - } - if ($property->isStatic()) { + if ($property->isPublic() && $property->isStatic()) { // A public static property reads no instance state; its real declaration is // materialized so it lives locally in the worker, like static methods. $staticProperties[] = $file->verbatim($property->getStartLine(), $property->getEndLine()); - } else { - // Instance properties are entity state: the stub declares none and lets the - // __get/__set forwarders below carry every access to the Rust side. - $hasPublicInstanceProperties = true; } } @@ -331,6 +332,7 @@ final class Generator $members = []; if ($isRoot) { $members[] = self::BOILERPLATE; + $members[] = self::PROPERTY_FORWARDERS; } if ($isRoot || $constructor !== null) { $members[] = $this->renderConstructor($fqcn, $constructor, $file); @@ -341,9 +343,6 @@ final class Generator if ($staticProperties !== []) { $members[] = implode("\n", $staticProperties); } - if ($hasPublicInstanceProperties) { - $members[] = self::PROPERTY_FORWARDERS; - } $members = array_merge($members, $staticMethods, $methodTexts); $body = implode("\n\n", $members); |
