refactor(hooks): consolidate proceed() via getOriginalCallable, stub-type the hook layer, final concrete hooks

[lisachenko]

Stacked on #197 — contains its commits; merge #197 first and this diff reduces to the hook-consolidation changes.

Six focused refactors of the engine hook layer (src/ClassExtension/Hook, src/System/Hook), no behavior change. #197 had just rewritten the fake-scope handling in seven of these hooks, so this builds on that shape rather than fighting it.

What changed

1. One proceed() prologue instead of twenty-two. Every hook hand-rolled if (!$this->hasOriginalHandler()) { throw new \LogicException('Original handler is not available'); } followed by an untyped ($this->originalHandler)(...) call. AbstractHook::getOriginalCallable() already is that prologue — it throws the same exception and returns a value narrowed to callable — and two hooks used it already. The rest are converted; five hooks that already called it kept a now-dead guard in front, which is deleted. CreateObjectHook keeps its two-branch shape (it falls back to ReflectionClass::newInstanceRaw() when no handler was captured) but asks hasOriginalHandler() instead of comparing the raw property.

2. Return types on the converted proceed() methods. Half of them were bare public function proceed(). Each type is derived from the C callback typedef the class documents and from what the value actually is: int for the verdict-returning handlers (compare, do_operation, interface_gets_implemented, cast_object), ?object for the pointer-returning ones (get_property_ptr_ptr, get_properties_for), object for write_property (zend_std_write_property reports the written slot or &EG(error_zval), never NULL), void for unset_property and zend_ast_process, mixed for accessors that materialize an engine value into a PHP one. GetPropertyPointerHook::handle() deliberately stays mixed: its value comes from the user handler, and a stricter declaration would turn a userland contract violation into a TypeError raised inside an FFI trampoline where nothing can catch it (issue #50).

3. Stub-typed engine state. The hook layer was the one subsystem that missed the generated-struct-stub cutover, which is where the bulk of its baseline came from: the libffi trampoline delivers callback arguments as mixed, and every assignment into a bare FFI\CData field produced an assign.propertyType entry. Applied the AGENTS.md convention exactly as Reflection\*/Type\* do it — the field is declared object and carries a @var docblock naming its ZEngine\Generated stub view, with one narrowing at the handle() boundary where the raw arguments arrive. That narrowing replaces the assert($x instanceof CData) scaffolding those statements carried (AGENTS.md names that weak form as what the stub migration supersedes); asserts that check real engine or handler invariants stay.

Handles the stubs deliberately do not model keep a CData declaration with a note saying why: the void ** property cache slot, the int * / zend_long * out-parameters of get_debug_info and count_elements, and the zend_object ** / zend_function ** / zend_class_entry ** double pointers of get_method and get_closure. AstProcessHook keeps a raw zend_ast handle too, because the AST wrappers that own that struct (NodeFactory/Node) have not been migrated yet.

Two owning boundaries were widened to the usual @param CData|Stub shape: Executor::setFakeScope()/withFakeScope() (every property hook now passes a typed $object->ce) and ReflectionClass::newInstanceRaw(). Also removed CompareValuesHook::$returnValue, declared but never read or written — compare_values has no result slot.

4. AbstractMethodResolutionHook goes through the owning reflection. resolveRawFunction() looked the class up in the engine class table, read ->function_table off the raw struct, took its address and wrapped it in a Type\HashTable — the "callers never reach into a raw CData" rule, and a duplicate of what ReflectionClass does when constructed. It is now (new ReflectionClass($className))->getMethodTable(): the constructor performs the class-table lookup and raises the "should be in the engine" ReflectionException itself, and getMethodTable() is the accessor that owns ce->function_table. Eleven lines become three.

On the alternative: PR #196 proposed a static entry-level helper (ReflectionClass::entryMethodTable()) for this shape. That helper is being withdrawn as against the framework's object model — consumers hold Reflection* objects, not raw entries plus static utilities — so this uses the instance API and takes no dependency on #196. The extra cost (a native reflection constructor plus three unused table wrappers) is paid only on the slow path: a wrapper produced by proceed() short-circuits on the identity fast path above it, and the VM inline-caches the resolved function per call site for compile-time constant method names.

5. IteratorBridge state is shared, not copied. The per-iteration record was an array shape, and PHP arrays are value types — so what each vtable callback read out of the registry was a snapshot, and marking an iteration broken had to go back through a second registry lookup by address with an isset() recheck around it. It is now a tiny @internal BridgedIterator with public typed properties: one lookup per callback preamble, and breakIteration() writes the flag straight through the record it is handed. The one observable difference is in the pathological case where the engine iterator is destroyed from inside the userland callback that then throws — the swallowed Throwable is now reported as the E_USER_WARNING it always should have been, instead of being dropped because the registry entry had already gone.

6. final on the concrete hooks. Every concrete hook is a leaf implementing one engine callback typedef; nothing in src/ or tests/ extends one, and the two that implement HookInterface directly (OpCodeHook, IteratorBridge) were final already. 27 more are sealed, which makes the extension point explicit: hooks are customized by passing a user-handler closure, not by subclassing.

Baseline delta

	before	after
phpstan-baseline.neon, total entries	401	315
src/ClassExtension/Hook + src/System/Hook	86	0

Entries were pruned by hand, verified with a throwaway override config setting reportUnmatchedIgnoredErrors: true (phpstan.dist.neon turns it off). Under that config the only remaining findings are the eight pre-existing unmatched inline @phpstan-ignore identifiers in CountElementsHook/GetClosureHook/GetMethodHook — unchanged in count and kind by this PR. Two further inline @phpstan-ignore callable.nonCallable comments were removed because the code no longer produces the error.

The five hook-related baseline entries that remain are outside this PR's scope: Hook\AbstractHook (2), Hook\HookInterface (1) and EngineExtension\Hook\ExtensionConstructorHook (2).

Validation

vendor/bin/phpstan analyse clean at level max and PHP_CS_FIXER_IGNORE_ENV=1 vendor/bin/php-cs-fixer fix --dry-run clean after every commit.

Tests were not run. This branch targets PHP 8.4 and the container runs PHP 8.5, so per the AGENTS.md version rule nothing that reaches Core::init() may be executed here — validation is static-only. CI runs the suite on the matching interpreter.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG