From 1b8f35cf3cba110e30fb070ffe179dd42c9a7b9e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 21:15:51 +0000 Subject: [PATCH 1/5] feat(core)!: enable function and initialization interception from registered pointcuts Features::INTERCEPT_FUNCTIONS and Features::INTERCEPT_INITIALIZATIONS made users switch on machinery that the registered pointcuts already describe. Both are removed: - execution(Namespace\function(*)) pointcuts weave function proxies without a flag; AdviceMatcher already skips every advisor without a KIND_FUNCTION pointcut. - ConstructorExecutionTransformer is always registered and turns itself on only while an advisor with a KIND_INIT pointcut is registered. The new internal AdvisorIndex loads the aspects that are not loaded yet (lazy aspects included) and collects the pointcut kinds again after every registration in the container. Before, an initialization(...) pointcut without the flag wove __initialization() but never ran its advice. Passing one of the removed bits now fails with a hint to drop it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Hhj7Aqnac5xDA6L31WeJs8 --- CHANGELOG.md | 1 + UPGRADE-4.0.md | 5 + demos/autoload_aspect.php | 3 - docs/advices.md | 5 +- docs/configuration.md | 13 +- docs/php85-limitations.md | 8 +- docs/pointcuts.md | 4 +- src/Aop/AGENTS.md | 5 +- src/Aop/Features.php | 19 +-- src/Core/AGENTS.md | 1 + src/Core/AdviceMatcher.php | 12 +- src/Core/AdvisorIndex.php | 84 +++++++++++++ src/Core/AspectKernel.php | 31 +++-- src/Core/FrameworkServices.php | 9 +- src/Instrument/AGENTS.md | 2 +- .../ConstructorExecutionTransformer.php | 13 ++ .../Transformer/PrefilteredNodeRewriter.php | 4 +- tests/Core/AdviceMatcherTest.php | 18 +-- tests/Core/AdvisorIndexTest.php | 111 ++++++++++++++++++ tests/Core/AspectKernelTest.php | 60 ++++++---- tests/Core/ContainerTest.php | 2 +- .../project/src/Application/MainFactory.php | 15 +++ tests/Functional/ClassWeavingTest.php | 27 +++++ .../ConstructorExecutionTransformerTest.php | 47 ++++++++ .../TransformationPerformanceTest.php | 2 +- 25 files changed, 407 insertions(+), 94 deletions(-) create mode 100644 src/Core/AdvisorIndex.php create mode 100644 tests/Core/AdvisorIndexTest.php create mode 100644 tests/Fixtures/project/src/Application/MainFactory.php diff --git a/CHANGELOG.md b/CHANGELOG.md index c3d9d50b..510062d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ See [UPGRADE-4.0.md](UPGRADE-4.0.md) for the migration guide. * [Performance] **Proxy bodies built as AST** — the proxy generators build every intercepted method, function and (static) initialization body directly as syntax tree nodes (`static $__joinPoint` with its `@var` docblock, the `InterceptorInjector::for*()` call and the `__invoke()`), instead of printing each body to a string and parsing it back; the proxy is printed once. Intercepted methods are looked up in a name index built once per woven class, declared types are converted from the syntax tree without a round trip through reflection types, and attribute arguments are cloned by one shared traverser. Generated proxies are byte-identical; proxy generation takes about a third of its former time (weaving case of the transformation benchmark: −43%). `ClassProxyGenerator::getJoinpointInvocationBody()` is replaced by `getJoinpointInvocationStatements()` and `createOriginalMethodCallable()`. **Removed** +* [BC BREAK] Removed `Features::INTERCEPT_FUNCTIONS` and `Features::INTERCEPT_INITIALIZATIONS`: the framework enables function and initialization interception from the registered pointcuts themselves. `execution(Namespace\function(*))` pointcuts weave function proxies without a flag, and the rewriting of `new` turns itself on while an `initialization(...)` pointcut is registered, lazily registered aspects included (the new internal `AdvisorIndex` loads them and tracks the pointcut kinds of every advisor). Before, an `initialization(...)` pointcut without the flag wove `__initialization()` but never ran its advice. Passing one of the removed bits fails with a hint to drop it. * [BC BREAK] Removed `Features::PARAMETER_WIDENING` and the parameter-widening code path in the proxy generators. The feature was a PHP 7.0/7.1 compatibility aid (parameter type widening, wiki.php.net/rfc/parameter-no-type-variance) that has been a no-op concern since the PHP 7.2 baseline: generated proxies always keep the original parameter types. Remove the flag from `AspectKernel::configureAop()` options if you passed it. * [BC BREAK] Removed DeclareError support, including the `DeclareError` attribute, `DeclareErrorInterceptor`, and `PointcutBuilder::declareError()`. Use `Before` or `Around` interceptors to emit user warnings or throw exceptions instead. * [BC BREAK] Removed support for the "dynamic" pointcut (`dynamic(public Foo->method*(*))`), including `MagicMethodDynamicPointcut`, `DynamicInvocationMatcherInterceptor`, the `Pointcut::KIND_DYNAMIC` constant and the `$instanceOrScope`/`$arguments` parameters of `Pointcut::matches()`. Use a traditional execution pointcut for the magic methods instead, e.g. `execution(public Foo->__call(*))` or `execution(public Foo::__callStatic(*))`, and check the invoked method name from `$invocation->getArguments()[0]` inside the advice. diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index bcf47ccd..ca15fcb5 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -182,6 +182,11 @@ that relied on that inheritance layer: be implemented. `Features::ALL` combines every flag, and `AspectKernel::hasFeature()` throws on an unknown flag. - `Features::PARAMETER_WIDENING` is removed. Delete the flag from your kernel options if you passed it. +- `Features::INTERCEPT_FUNCTIONS` and `Features::INTERCEPT_INITIALIZATIONS` are removed: function and + initialization interception are enabled by the registered `execution(Namespace\function(*))` and + `initialization(...)` pointcuts. Delete the flags from your kernel options; passing them throws an + `InvalidConfigurationException`. While an `initialization(...)` pointcut is registered, every `new` in woven + files is rewritten, so an aspect that declares one without the flag starts intercepting object creation. Removed unused API ------------------ diff --git a/demos/autoload_aspect.php b/demos/autoload_aspect.php index 5b91183e..42cb7953 100644 --- a/demos/autoload_aspect.php +++ b/demos/autoload_aspect.php @@ -11,7 +11,6 @@ */ use Demo\Aspect\AwesomeAspectKernel; -use Go\Aop\Features; include __DIR__ . '/../vendor/autoload.php'; @@ -20,6 +19,4 @@ 'debug' => true, 'appDir' => __DIR__, 'cacheDir' => __DIR__ . '/cache', - - 'features' => Features::INTERCEPT_FUNCTIONS, ]); diff --git a/docs/advices.md b/docs/advices.md index bb6ea127..6ed86698 100644 --- a/docs/advices.md +++ b/docs/advices.md @@ -102,14 +102,13 @@ protected null $jsonSerializable; ## Function interception -With `Features::INTERCEPT_FUNCTIONS`, `execution(App\Service\file_get_contents(*))` intercepts calls of +`execution(App\Service\file_get_contents(*))` intercepts calls of `file_get_contents()` from code in the `App\Service` namespace that calls the function without a leading backslash. The advice receives a `FunctionInvocation`. ## Object creation -With `Features::INTERCEPT_INITIALIZATIONS`, `initialization(App\Model\Order)` intercepts `new Order(...)` in woven -files. The advice receives a `ConstructorInvocation`. `staticinitialization(App\Model\*)` runs once, after the class +`initialization(App\Model\Order)` intercepts `new Order(...)` in woven files. The advice receives a `ConstructorInvocation`. `staticinitialization(App\Model\*)` runs once, after the class is loaded. ## Registering aspects diff --git a/docs/configuration.md b/docs/configuration.md index 02d155f1..59f54ead 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -29,15 +29,19 @@ bit throws `Go\Aop\Exception\InvalidConfigurationException`. ## Features -Combine the flags with `|`, e.g. `'features' => Features::INTERCEPT_FUNCTIONS | Features::PREBUILT_CACHE`. +Combine the flags with `|`, e.g. `'features' => Features::INTERCEPT_INCLUDES | Features::PREBUILT_CACHE`. | Flag | Effect | |-----------------------------|--------| -| `INTERCEPT_FUNCTIONS` | Enables `execution(Namespace\function(*))` pointcuts for system functions called from your namespaced code. Expensive: every woven file is scanned for function calls. | -| `INTERCEPT_INITIALIZATIONS` | Enables `initialization(...)` pointcuts by rewriting `new Foo(...)` expressions in woven files. | | `INTERCEPT_INCLUDES` | Routes `include`/`require` in woven files through the weaver, for legacy code that is not loaded by Composer. | | `PREBUILT_CACHE` | Trusts the cache built at deploy time completely. Skips the cache directory and writability probes, source `filemtime` comparisons, tracked-resource checks and advisor cache freshness checks, and never writes to the cache directory. Rebuilding the cache on every deploy is your responsibility. Works on read-only file systems. | +Function and initialization pointcuts need no flag: the framework enables their machinery from the registered +aspects, lazily registered ones included. An `execution(Namespace\function(*))` pointcut weaves function proxies into +the matching namespaces. An `initialization(...)` pointcut turns on the rewriting of `new Foo(...)` expressions in +woven files; while one is registered, every rewritten `new` goes through the interceptor, so keep such pointcuts to +the code that needs them. + ## Extending the transformation pipeline Every file is transformed by a chain of `Go\Instrument\Transformer\SourceTransformer` services, in registration @@ -46,7 +50,8 @@ order: 1. `SyntaxTreeRewriter` walks the syntax tree of the file once, for all `NodeRewriter` rules. It is a framework service, so it always opens the chain. 2. The transformers registered by `AspectKernel::registerTransformerServices()`: `WeavingTransformer`, plus the - `INTERCEPT_INITIALIZATIONS`, `INTERCEPT_INCLUDES` and magic-constant rules. + `new` rewriting rule (active while an `initialization(...)` pointcut is registered), the `INTERCEPT_INCLUDES` + rule and the magic-constant rule. 3. Your own transformers. To change single syntax nodes, implement `NodeRewriter`. It declares the node classes it handles and rewrites diff --git a/docs/php85-limitations.md b/docs/php85-limitations.md index c73ba157..0f8f4fe1 100644 --- a/docs/php85-limitations.md +++ b/docs/php85-limitations.md @@ -68,10 +68,10 @@ Backed enum cases declared with constant expressions (e.g. `case Negative = -1;` the generated proxy enum, producing a fatal error at load time. Fixed by re-emitting the original case expression verbatim in the proxy enum. -### `new` in initializers under `INTERCEPT_INITIALIZATIONS` — tracked in [#603](https://github.com/goaop/framework/issues/603) +### `new` in initializers with an `initialization(...)` pointcut — tracked in [#603](https://github.com/goaop/framework/issues/603) -`new` expressions in property/parameter initializers do not work correctly when the -`INTERCEPT_INITIALIZATIONS` kernel feature is enabled. +`new` expressions in property/parameter initializers do not work correctly while an +`initialization(...)` pointcut is registered. ### Global constants in attribute arguments — tracked in [#602](https://github.com/goaop/framework/issues/602) @@ -104,7 +104,7 @@ properties from property join points. This is a PHP engine constraint, not a bug | Closures / FCC in attribute arguments | ❌ Limited | Tracked in [#601](https://github.com/goaop/framework/issues/601) | | Promoted-property interception | ❌ Limited | Tracked in [#599](https://github.com/goaop/framework/issues/599) | | Enum constant-expression case values | ❌→✅ Fixed | Tracked in [#600](https://github.com/goaop/framework/issues/600) | -| `new` in initializers + `INTERCEPT_INITIALIZATIONS` | ❌ Limited | Tracked in [#603](https://github.com/goaop/framework/issues/603) | +| `new` in initializers + `initialization(...)` pointcut | ❌ Limited | Tracked in [#603](https://github.com/goaop/framework/issues/603) | | Global constants in attribute arguments | ❌ Limited | Tracked in [#602](https://github.com/goaop/framework/issues/602) | | Class-level attributes on woven classes | ❌ Limited | Tracked in [#598](https://github.com/goaop/framework/issues/598) | | Static property interception via `access()` | ❌ Never | No property hooks for static properties (PHP engine constraint) | diff --git a/docs/pointcuts.md b/docs/pointcuts.md index 3e2a5652..ccf3a509 100644 --- a/docs/pointcuts.md +++ b/docs/pointcuts.md @@ -9,10 +9,10 @@ creation. It is the argument of an advice attribute, e.g. `#[Before('execution(p |------------------------------------------|---------| | `execution(modifiers Class->method(*))` | Execution of a dynamic method | | `execution(modifiers Class::method(*))` | Execution of a static method | -| `execution(Namespace\function(*))` | Calls of a system function from that namespace (`Features::INTERCEPT_FUNCTIONS`) | +| `execution(Namespace\function(*))` | Calls of a system function from that namespace | | `access(modifiers Class->property)` | Reads and writes of an instance property | | `within(ClassFilter)` | Every member (methods and properties) of the matching classes | -| `initialization(ClassFilter)` | Object creation with `new` (`Features::INTERCEPT_INITIALIZATIONS`) | +| `initialization(ClassFilter)` | Object creation with `new` | | `staticinitialization(ClassFilter)` | The first load of the matching classes | | `@execution(Attribute)` | Methods that carry the attribute | | `@access(Attribute)` | Properties that carry the attribute | diff --git a/src/Aop/AGENTS.md b/src/Aop/AGENTS.md index c3f28662..92822dd7 100644 --- a/src/Aop/AGENTS.md +++ b/src/Aop/AGENTS.md @@ -30,7 +30,7 @@ Proxy generators use TypeGenerator::renderTypeForPhpDoc() to emit V as 2nd gener | AbstractMethodInvocation | MethodInvocation | Base; protected readonly Closure $closureToCall (FCC); TRAIT_ALIAS_SUFFIX='OriginalAlias'; keeps method reflection | | DynamicTraitAliasMethodInvocation | DynamicMethodInvocation | receives $this->mOriginalAlias(...) or parent::m(...); proceed() via ReflectionMethod::invokeArgs (handles by-ref correctly) | | StaticTraitAliasMethodInvocation | StaticMethodInvocation | FCC shim: static fn(array $args) => forward_static_call_array(...); bindTo(null, $scope) per call | -| ReflectionConstructorInvocation | ConstructorInvocation | newInstanceWithoutConstructor() then call constructor (requires INTERCEPT_INITIALIZATIONS feature) | +| ReflectionConstructorInvocation | ConstructorInvocation | newInstanceWithoutConstructor() then call constructor (`new` rewritten while an initialization pointcut is registered) | | ReflectionFunctionInvocation | FunctionInvocation | receives FCC to global fn (e.g. \strlen(...) with leading \ to avoid recursive proxy call) | | ClassFieldAccess | FieldAccess | Property hooks call read()/write() by value; readByReference() only for the &get hook of arrays (binding released after) | | StaticInitializationJoinpoint | ClassJoinpoint | Fired once after proxy class loaded via `Proxy::__staticInitialization()` | @@ -69,5 +69,6 @@ when `$level > 0` and pops it in `finally`. Check every change with `composer te ## Features (src/Aop/Features.php) Final class with bitmask constants (`ALL` is the mask of every flag): -- INTERCEPT_FUNCTIONS=1, INTERCEPT_INITIALIZATIONS=2, INTERCEPT_INCLUDES=4 +- INTERCEPT_INCLUDES=4 (bits 1/2 were INTERCEPT_FUNCTIONS/INTERCEPT_INITIALIZATIONS, removed: AspectKernel rejects them with a hint) +- No flag for function/initialization interception: pointcut kinds decide (AdvisorIndex, see src/Core/AGENTS.md) - PREBUILT_CACHE=64 — assume cache already prepared, skip freshness checks diff --git a/src/Aop/Features.php b/src/Aop/Features.php index 77df1e33..153567f9 100644 --- a/src/Aop/Features.php +++ b/src/Aop/Features.php @@ -14,21 +14,12 @@ /** * Framework feature flags for the `features` kernel option, combined with `|` + * + * Interception of system functions and of `new` needs no flag: it is enabled by the registered + * `execution(Namespace\function(*))` and `initialization(...)` pointcuts themselves. */ final class Features { - /** - * Enables interception of system function. - * By default this feature is disabled, because this option is very expensive. - */ - public const int INTERCEPT_FUNCTIONS = 1; - - /** - * Enables interception of "new" operator in the source code - * By default this feature is disabled, because it's very tricky - */ - public const int INTERCEPT_INITIALIZATIONS = 2; - /** * Enables interception of "include"/"require" operations in legacy code * By default this feature is disabled, because only composer should be used @@ -49,9 +40,7 @@ final class Features /** * Mask of every defined feature flag */ - public const int ALL = self::INTERCEPT_FUNCTIONS - | self::INTERCEPT_INITIALIZATIONS - | self::INTERCEPT_INCLUDES + public const int ALL = self::INTERCEPT_INCLUDES | self::PREBUILT_CACHE; /** diff --git a/src/Core/AGENTS.md b/src/Core/AGENTS.md index 6793a9db..f5d3e3d7 100644 --- a/src/Core/AGENTS.md +++ b/src/Core/AGENTS.md @@ -10,6 +10,7 @@ ## Aspect loading - AspectLoader — scans aspect classes for pointcut/advice attributes → Advisor[]; CachedAspectLoader decorates it (both implement AspectLoaderInterface) +- AdvisorIndex (@internal) — loads unloaded aspects, returns advisors, hasPointcutKind(Pointcut::KIND_*) = union of advisor pointcut kinds, recollected whenever getServicesByInterface(Advisor) returns another array (after any registration). Gates kind-specific machinery instead of feature flags (`new` rewriting ← KIND_INIT); function advices need no gate (AdviceMatcher skips non-KIND_FUNCTION advisors) ### Compiled advisor cache - Lives in the nested Go\Core\Cache namespace (CachedAspectLoader, AdvisorCacheCompiler, AdvisorCachePrinter, CacheFileWriter — all @internal); format, naming and error-handling rules: see src/Core/Cache/AGENTS.md diff --git a/src/Core/AdviceMatcher.php b/src/Core/AdviceMatcher.php index 2a0062bd..1775efc9 100644 --- a/src/Core/AdviceMatcher.php +++ b/src/Core/AdviceMatcher.php @@ -29,11 +29,6 @@ */ class AdviceMatcher implements AdviceMatcherInterface { - /** - * Constructor - * - * @param bool $isInterceptFunctions Optional flag to enable function interception - */ /** * Reflections of every internal function, built on the first function match of the request * @@ -41,8 +36,6 @@ class AdviceMatcher implements AdviceMatcherInterface */ private static ?array $internalFunctions = null; - public function __construct(private readonly bool $isInterceptFunctions = false) {} - /** * Returns list of function advices for namespace * @@ -52,10 +45,7 @@ public function __construct(private readonly bool $isInterceptFunctions = false) */ public function getAdvicesForFunctions(ReflectionFileNamespace $namespace, array $advisors): array { - if (!$this->isInterceptFunctions) { - return []; - } - + // Only advisors with a function pointcut are matched: without one a namespace costs a scan of the advisors $advices = []; foreach ($advisors as $advisorId => $advisor) { diff --git a/src/Core/AdvisorIndex.php b/src/Core/AdvisorIndex.php new file mode 100644 index 00000000..6ed17a3f --- /dev/null +++ b/src/Core/AdvisorIndex.php @@ -0,0 +1,84 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Core; + +use Go\Aop\Advisor; +use Go\Aop\Pointcut; +use Go\Aop\PointcutAdvisor; + +/** + * Index of the advisors of every registered aspect and of the pointcut kinds they use + * + * Lets the transformation pipeline enable the machinery a pointcut kind needs (e.g. the rewriting of `new` for + * initialization pointcuts) only while an advisor of that kind is registered. Aspects that are not loaded yet are + * loaded on the first query, and the kinds are collected again after every registration in the container, so an + * aspect registered or loaded later is taken into account as well. + * + * @internal Framework service, not a public extension point + */ +final class AdvisorIndex +{ + /** + * Advisors the kinds below were collected from + * + * @var array|null + */ + private ?array $indexedAdvisors = null; + + /** + * Bitmask of the pointcut kinds of the indexed advisors + */ + private int $pointcutKinds = 0; + + public function __construct( + private readonly AspectContainer $container, + private readonly AspectLoaderInterface $aspectLoader, + ) {} + + /** + * Returns the advisors of every registered aspect, loading the aspects that are not loaded yet + * + * @return array + */ + public function getAdvisors(): array + { + foreach ($this->aspectLoader->getUnloadedAspects() as $unloadedAspect) { + $this->aspectLoader->loadAndRegister($unloadedAspect); + } + + return $this->container->getServicesByInterface(Advisor::class); + } + + /** + * Checks whether a pointcut of any registered advisor can match a join point of the given kind + * + * @param int $pointcutKind One of the Pointcut::KIND_* constants + */ + public function hasPointcutKind(int $pointcutKind): bool + { + $advisors = $this->getAdvisors(); + // The container returns the same array until the next registration, compared without a walk + if ($advisors !== $this->indexedAdvisors) { + $pointcutKinds = 0; + foreach ($advisors as $advisor) { + if ($advisor instanceof PointcutAdvisor) { + $pointcutKinds |= $advisor->getPointcut()->getKind(); + } + } + $this->indexedAdvisors = $advisors; + $this->pointcutKinds = $pointcutKinds; + } + + return ($this->pointcutKinds & $pointcutKind) !== 0; + } +} diff --git a/src/Core/AspectKernel.php b/src/Core/AspectKernel.php index 389ec22f..5c48fb34 100644 --- a/src/Core/AspectKernel.php +++ b/src/Core/AspectKernel.php @@ -54,6 +54,11 @@ */ abstract class AspectKernel { + /** + * Bits of the removed INTERCEPT_FUNCTIONS (1) and INTERCEPT_INITIALIZATIONS (2) features + */ + private const int REMOVED_FEATURES = 1 | 2; + /** * Composer packages the framework runs on during weaving, always excluded from weaving */ @@ -150,7 +155,6 @@ public function init(array $options = []): void $container = $this->container = new $this->options['containerClass']($resourcesToTrack); $container->add(AspectKernel::class, $this); - $container->add('kernel.interceptFunctions', $this->hasFeature(Features::INTERCEPT_FUNCTIONS)); // The framework's own services are deferred definitions registered through the // generic lazy container API - the container itself knows nothing about them. @@ -319,6 +323,12 @@ protected function normalizeOptions(array $options): array )); } $features = $merged['features'] ?? 0; + if (is_int($features) && ($features & self::REMOVED_FEATURES) !== 0) { + throw new InvalidConfigurationException( + 'Features INTERCEPT_FUNCTIONS and INTERCEPT_INITIALIZATIONS were removed, remove them from the "features" ' + . 'option: function and initialization pointcuts are enabled automatically by the registered aspects.', + ); + } if (!is_int($features) || ($features & ~Features::ALL) !== 0) { throw new InvalidConfigurationException(sprintf( 'Option "features" must be a combination of Go\\Aop\\Features constants, got %s.', @@ -379,9 +389,10 @@ abstract protected function configureAop(AspectContainer $container): void; * * Every registered container service implementing SourceTransformer becomes part of * the chain, in registration order (registration order IS the transformation order); - * feature flags gate registration exactly as they used to gate construction. Nothing - * is constructed here - these are deferred definitions, materialized on the first - * cache miss only. + * feature flags gate registration exactly as they used to gate construction, and the + * rewriting of `new` turns itself on only while an initialization pointcut is + * registered (AdvisorIndex). Nothing is constructed here - these are deferred + * definitions, materialized on the first cache miss only. * * Rewrites of single syntax tree nodes are NodeRewriter services instead: the * SyntaxTreeRewriter stage, a framework service registered before this hook (so @@ -395,12 +406,12 @@ abstract protected function configureAop(AspectContainer $container): void; */ protected function registerTransformerServices(AspectContainer $container): void { - if ($this->hasFeature(Features::INTERCEPT_INITIALIZATIONS)) { - $container->addLazyService( - ConstructorExecutionTransformer::class, - fn(): ConstructorExecutionTransformer => new ConstructorExecutionTransformer(), - ); - } + $container->addLazyService( + ConstructorExecutionTransformer::class, + fn(AspectContainer $container): ConstructorExecutionTransformer => new ConstructorExecutionTransformer( + $container->getService(AdvisorIndex::class), + ), + ); if ($this->hasFeature(Features::INTERCEPT_INCLUDES)) { $container->addLazyService( FilterInjectorTransformer::class, diff --git a/src/Core/FrameworkServices.php b/src/Core/FrameworkServices.php index a305dd82..04c5ef1c 100644 --- a/src/Core/FrameworkServices.php +++ b/src/Core/FrameworkServices.php @@ -36,9 +36,7 @@ public static function register(AspectContainer $container): void new PointcutGrammar(), )); - $container->addLazyService(AdviceMatcher::class, fn(AspectContainer $container): AdviceMatcher => new AdviceMatcher( - (bool) $container->getValue('kernel.interceptFunctions'), - )); + $container->addLazyService(AdviceMatcher::class, fn(): AdviceMatcher => new AdviceMatcher()); $container->addLazyService(AttributeAspectLoaderExtension::class, fn(AspectContainer $container): AttributeAspectLoaderExtension => new AttributeAspectLoaderExtension( $container->getService(PointcutLexer::class), @@ -62,6 +60,11 @@ public static function register(AspectContainer $container): void return new CachedAspectLoader($container, AspectLoader::class, $options); }); + $container->addLazyService(AdvisorIndex::class, fn(AspectContainer $container): AdvisorIndex => new AdvisorIndex( + $container, + $container->getService(CachedAspectLoader::class), + )); + // Registered ahead of the kernel's transformer services, so the single syntax tree walk // opens the transformation chain even when AspectKernel::registerTransformerServices() // is overridden; the NodeRewriter rules are resolved by their tag when it is first used diff --git a/src/Instrument/AGENTS.md b/src/Instrument/AGENTS.md index 3747ccd1..c89bdf35 100644 --- a/src/Instrument/AGENTS.md +++ b/src/Instrument/AGENTS.md @@ -11,7 +11,7 @@ Applied per loaded file. Each returns TransformerResult: Transformed|Abstain|Aborted (skips the rest of the chain, reverts to the original source, recorded as untransformed). 1. SyntaxTreeRewriter — framework service (FrameworkServices, so it survives an overridden registerTransformerServices()); walks the syntax tree once and dispatches every node to the NodeRewriter rules (container tag) declaring its type (`getNodeTypes()`), in registration order. Rules are stateless per node (weaving re-enters the loader for other files mid-chain); ancestors are passed to the rules from the dispatcher's stack (no parent attributes on the shared tree, they cost ~3x a walk). PrefilteredNodeRewriter rules declare getSourceMarkers(): looked up case-insensitively in StreamMetaData::$originalSource before the walk, a rule without a marker in the file is skipped, no rule left → no walk (rules-per-node-class cache keyed by the set of applicable rules). Built-in rules: - - ConstructorExecutionTransformer — new expressions outside constant-expression contexts (only if INTERCEPT_INITIALIZATIONS enabled) + - ConstructorExecutionTransformer — new expressions outside constant-expression contexts; always registered, getSourceMarkers() returns [] (rule off) while AdvisorIndex reports no KIND_INIT advisor - FilterInjectorTransformer — include/require (only if INTERCEPT_INCLUDES enabled); at runtime rewrite() serves, in production, a file known to the include map natively (original file if untransformed, recorded cacheUri if woven, no file_exists/freshness check, no _transformation.cache); unknown files and debug mode → php://filter - MagicConstantTransformer — `__FILE__`/`__DIR__` → original paths, wraps `getFileName()`; never counts as a transformation (only cache-dir files need it; PHP resolves magic constants of a `php://filter/.../resource=` include to ) 2. WeavingTransformer — main; AdviceMatcher + CachedAspectLoader → proxy generators. Sees the token edits of the rules. diff --git a/src/Instrument/Transformer/ConstructorExecutionTransformer.php b/src/Instrument/Transformer/ConstructorExecutionTransformer.php index 5f7a641e..74fd91ea 100644 --- a/src/Instrument/Transformer/ConstructorExecutionTransformer.php +++ b/src/Instrument/Transformer/ConstructorExecutionTransformer.php @@ -15,6 +15,8 @@ use Go\Aop\Exception\WeavingException; use Go\Aop\Framework\ReflectionConstructorInvocation; use Go\Aop\InitializationAware; +use Go\Aop\Pointcut; +use Go\Core\AdvisorIndex; use PhpParser\Node; use PhpParser\Node\Attribute; use PhpParser\Node\Const_; @@ -46,6 +48,12 @@ final class ConstructorExecutionTransformer implements PrefilteredNodeRewriter */ private static ?self $instance = null; + /** + * @param AdvisorIndex|null $advisorIndex Index of the registered advisors: the rule is active only while one of + * them has an initialization pointcut. Without it, the rule is always active + */ + public function __construct(private readonly ?AdvisorIndex $advisorIndex = null) {} + /** * Singletone */ @@ -76,6 +84,11 @@ public function getNodeTypes(): array public function getSourceMarkers(): array { + // Without an initialization pointcut there is nothing to route a `new` to: the rule stays off + if ($this->advisorIndex?->hasPointcutKind(Pointcut::KIND_INIT) === false) { + return []; + } + return ['new']; } diff --git a/src/Instrument/Transformer/PrefilteredNodeRewriter.php b/src/Instrument/Transformer/PrefilteredNodeRewriter.php index b76bb1fe..641b1deb 100644 --- a/src/Instrument/Transformer/PrefilteredNodeRewriter.php +++ b/src/Instrument/Transformer/PrefilteredNodeRewriter.php @@ -25,7 +25,9 @@ interface PrefilteredNodeRewriter extends NodeRewriter /** * Substrings of which a source contains at least one if the rule can rewrite a node of it * - * @return non-empty-list + * An empty list turns the rule off for the file, e.g. while nothing registered needs it. + * + * @return list */ public function getSourceMarkers(): array; } diff --git a/tests/Core/AdviceMatcherTest.php b/tests/Core/AdviceMatcherTest.php index b71f912b..4eb64ccd 100644 --- a/tests/Core/AdviceMatcherTest.php +++ b/tests/Core/AdviceMatcherTest.php @@ -443,18 +443,19 @@ public function testIntroductionAdviceIsSkippedForTraitContext(): void } /** - * Verifies that getAdvicesForFunctions() short-circuits to an empty array when function - * interception is not enabled. + * Verifies that getAdvicesForFunctions() never matches an advisor without a function pointcut: + * function interception needs no feature flag, the pointcut kind enables it. */ - public function testGetAdvicesForFunctionsReturnsEmptyArrayWhenFeatureDisabled(): void + public function testGetAdvicesForFunctionsSkipsAdvisorsWithoutFunctionPointcut(): void { - $adviceMatcher = new AdviceMatcher(isInterceptFunctions: false); + $adviceMatcher = new AdviceMatcher(); $reflectionFile = new ReflectionFile(__FILE__); $namespace = $reflectionFile->getFileNamespace(__NAMESPACE__); $pointcut = $this->createMock(Pointcut::class); - $pointcut->expects($this->never())->method('getKind'); + $pointcut->method('getKind')->willReturn(Pointcut::KIND_METHOD | Pointcut::KIND_INIT | Pointcut::KIND_CLASS); + $pointcut->expects($this->never())->method('matches'); $advice = $this->createMock(Advice::class); $advisor = new GenericPointcutAdvisor($pointcut, $advice); @@ -465,12 +466,11 @@ public function testGetAdvicesForFunctionsReturnsEmptyArrayWhenFeatureDisabled() } /** - * Verifies that getAdvicesForFunctions() matches an internal PHP function by name when - * function interception is enabled. + * Verifies that getAdvicesForFunctions() matches an internal PHP function by name for a function pointcut. */ public function testGetAdvicesForFunctionsMatchesInternalFunction(): void { - $adviceMatcher = new AdviceMatcher(isInterceptFunctions: true); + $adviceMatcher = new AdviceMatcher(); $reflectionFile = new ReflectionFile(__FILE__); $namespace = $reflectionFile->getFileNamespace(__NAMESPACE__); @@ -499,7 +499,7 @@ public function testGetAdvicesForFunctionsMatchesInternalFunction(): void */ public function testGetAdvicesForFunctionsSkipsNonMatchingNamespace(): void { - $adviceMatcher = new AdviceMatcher(isInterceptFunctions: true); + $adviceMatcher = new AdviceMatcher(); $reflectionFile = new ReflectionFile(__FILE__); $namespace = $reflectionFile->getFileNamespace(__NAMESPACE__); diff --git a/tests/Core/AdvisorIndexTest.php b/tests/Core/AdvisorIndexTest.php new file mode 100644 index 00000000..d59dcf2f --- /dev/null +++ b/tests/Core/AdvisorIndexTest.php @@ -0,0 +1,111 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Core; + +use Go\Aop\Advice; +use Go\Aop\Aspect; +use Go\Aop\Pointcut; +use Go\Aop\Pointcut\TruePointcut; +use Go\Aop\Support\GenericPointcutAdvisor; +use PHPUnit\Framework\TestCase; + +final class AdvisorIndexTest extends TestCase +{ + public function testReportsNoPointcutKindWithoutAdvisors(): void + { + $index = new AdvisorIndex(new Container(), $this->createLoader()); + + $this->assertSame([], $index->getAdvisors()); + $this->assertFalse($index->hasPointcutKind(Pointcut::KIND_INIT)); + $this->assertFalse($index->hasPointcutKind(Pointcut::KIND_FUNCTION)); + } + + public function testReportsPointcutKindsOfRegisteredAdvisors(): void + { + $container = new Container(); + $container->add('advisor.init', $this->createAdvisor(Pointcut::KIND_INIT | Pointcut::KIND_CLASS)); + $index = new AdvisorIndex($container, $this->createLoader()); + + $this->assertTrue($index->hasPointcutKind(Pointcut::KIND_INIT)); + $this->assertTrue($index->hasPointcutKind(Pointcut::KIND_CLASS)); + $this->assertFalse($index->hasPointcutKind(Pointcut::KIND_FUNCTION)); + $this->assertFalse($index->hasPointcutKind(Pointcut::KIND_METHOD)); + } + + public function testCollectsPointcutKindsAgainAfterRegistration(): void + { + $container = new Container(); + $container->add('advisor.method', $this->createAdvisor(Pointcut::KIND_METHOD)); + $index = new AdvisorIndex($container, $this->createLoader()); + $this->assertFalse($index->hasPointcutKind(Pointcut::KIND_FUNCTION)); + + $container->add('advisor.function', $this->createAdvisor(Pointcut::KIND_FUNCTION)); + + $this->assertTrue($index->hasPointcutKind(Pointcut::KIND_FUNCTION)); + $this->assertTrue($index->hasPointcutKind(Pointcut::KIND_METHOD)); + } + + /** + * Aspects registered lazily are loaded by the first query, so their pointcuts are known before the weaving + */ + public function testLoadsUnloadedAspectsBeforeCollectingPointcutKinds(): void + { + $container = new Container(); + $aspect = new class implements Aspect {}; + $advisor = $this->createAdvisor(Pointcut::KIND_INIT | Pointcut::KIND_CLASS); + $loader = new class ($container, $aspect, $advisor) implements AspectLoaderInterface { + private bool $isLoaded = false; + + public function __construct( + private readonly AspectContainer $container, + private readonly Aspect $aspect, + private readonly GenericPointcutAdvisor $advisor, + ) {} + + public function load(Aspect $aspect): array + { + return ['advisor.lazy' => $this->advisor]; + } + + public function loadAndRegister(Aspect $aspect): void + { + foreach ($this->load($aspect) as $itemId => $item) { + $this->container->add($itemId, $item); + } + $this->isLoaded = true; + } + + public function getUnloadedAspects(): array + { + return $this->isLoaded ? [] : [$this->aspect]; + } + }; + $index = new AdvisorIndex($container, $loader); + + $this->assertTrue($index->hasPointcutKind(Pointcut::KIND_INIT)); + $this->assertSame(['advisor.lazy' => $advisor], $index->getAdvisors()); + } + + private function createAdvisor(int $pointcutKind): GenericPointcutAdvisor + { + return new GenericPointcutAdvisor(new TruePointcut($pointcutKind), $this->createStub(Advice::class)); + } + + private function createLoader(): AspectLoaderInterface + { + $loader = $this->createStub(AspectLoaderInterface::class); + $loader->method('getUnloadedAspects')->willReturn([]); + + return $loader; + } +} diff --git a/tests/Core/AspectKernelTest.php b/tests/Core/AspectKernelTest.php index 86aebfe8..ec1fc4b3 100644 --- a/tests/Core/AspectKernelTest.php +++ b/tests/Core/AspectKernelTest.php @@ -184,12 +184,10 @@ public function testHasFeatureReturnsTrueOnlyWhenBitIsSet(): void $kernel = $this->makeKernel(); $this->setKernelOptions($kernel, [ - 'features' => Features::INTERCEPT_FUNCTIONS | Features::INTERCEPT_INCLUDES, + 'features' => Features::INTERCEPT_INCLUDES, ]); - $this->assertTrue($kernel->hasFeature(Features::INTERCEPT_FUNCTIONS)); $this->assertTrue($kernel->hasFeature(Features::INTERCEPT_INCLUDES)); - $this->assertFalse($kernel->hasFeature(Features::INTERCEPT_INITIALIZATIONS)); $this->assertFalse($kernel->hasFeature(Features::PREBUILT_CACHE)); } @@ -201,6 +199,8 @@ public static function unknownFeatures(): iterable yield 'zero' => [0]; yield 'undefined bit' => [8]; yield 'defined and undefined bits' => [Features::PREBUILT_CACHE | 128]; + yield 'removed INTERCEPT_FUNCTIONS bit' => [1]; + yield 'removed INTERCEPT_INITIALIZATIONS bit' => [2]; } #[DataProvider('unknownFeatures')] @@ -337,7 +337,7 @@ public function testNormalizeOptionsRejectsUnknownOptionWithoutSuggestion(): voi public static function invalidFeatures(): iterable { yield 'unknown bit' => [8]; - yield 'known and unknown bits' => [Features::INTERCEPT_FUNCTIONS | 128]; + yield 'known and unknown bits' => [Features::INTERCEPT_INCLUDES | 128]; yield 'string' => ['1']; yield 'boolean' => [true]; } @@ -356,11 +356,37 @@ public function testNormalizeOptionsRejectsInvalidFeatures(mixed $features): voi ]]); } + /** + * @return iterable + */ + public static function removedFeatures(): iterable + { + yield 'INTERCEPT_FUNCTIONS' => [1]; + yield 'INTERCEPT_INITIALIZATIONS' => [2]; + yield 'removed and known bits' => [2 | Features::PREBUILT_CACHE]; + } + + /** + * Function and initialization pointcuts need no feature flag anymore, the removed flags point to that + */ + #[DataProvider('removedFeatures')] + public function testNormalizeOptionsRejectsRemovedFeatures(int $features): void + { + $kernel = $this->makeKernel(); + + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessageIsOrContains('Features INTERCEPT_FUNCTIONS and INTERCEPT_INITIALIZATIONS were removed'); + + $this->invokeProtected($kernel, 'normalizeOptions', [[ + 'cacheDir' => '/some/cache/dir', + 'features' => $features, + ]]); + } + public function testNormalizeOptionsAcceptsEveryKnownFeature(): void { $kernel = $this->makeKernel(); - $features = Features::INTERCEPT_FUNCTIONS | Features::INTERCEPT_INITIALIZATIONS - | Features::INTERCEPT_INCLUDES | Features::PREBUILT_CACHE; + $features = Features::INTERCEPT_INCLUDES | Features::PREBUILT_CACHE; $normalized = $this->invokeProtectedArray($kernel, 'normalizeOptions', [[ 'cacheDir' => '/some/cache/dir', @@ -416,7 +442,7 @@ public function testNormalizeOptionsResolvesAndMergesGivenOptions(): void 'debug' => true, 'cacheDir' => '/some/cache/dir', 'cacheFileMode' => 0644, - 'features' => Features::INTERCEPT_FUNCTIONS, + 'features' => Features::INTERCEPT_INCLUDES, 'includePaths' => ['/some/include'], 'excludePaths' => ['/some/exclude'], 'containerClass' => Container::class, @@ -424,7 +450,7 @@ public function testNormalizeOptionsResolvesAndMergesGivenOptions(): void $this->assertTrue($normalized['debug']); $this->assertSame(0644, $normalized['cacheFileMode']); - $this->assertSame(Features::INTERCEPT_FUNCTIONS, $normalized['features']); + $this->assertSame(Features::INTERCEPT_INCLUDES, $normalized['features']); $this->assertSame(Container::class, $normalized['containerClass']); $cacheDir = $normalized['cacheDir']; @@ -469,7 +495,7 @@ public function testNormalizeOptionsIgnoresNonStringEntriesInPathLists(): void } } - public function testRegisterTransformerServicesAlwaysRegistersWeavingAndMagicConstantTransformers(): void + public function testRegisterTransformerServicesAlwaysRegistersWeavingConstructorAndMagicConstantTransformers(): void { $kernel = $this->makeKernel(); @@ -480,19 +506,7 @@ public function testRegisterTransformerServicesAlwaysRegistersWeavingAndMagicCon $this->assertTrue($container->has(WeavingTransformer::class)); $this->assertTrue($container->has(MagicConstantTransformer::class)); - $this->assertFalse($container->has(ConstructorExecutionTransformer::class)); - $this->assertFalse($container->has(FilterInjectorTransformer::class)); - } - - public function testRegisterTransformerServicesRegistersConstructorExecutionTransformerWhenFeatureEnabled(): void - { - $kernel = $this->makeKernel(); - - $this->setKernelOptions($kernel, ['features' => Features::INTERCEPT_INITIALIZATIONS]); - - $container = new Container(); - $this->invokeProtected($kernel, 'registerTransformerServices', [$container]); - + // Turns itself on only while an initialization pointcut is registered, see AdvisorIndex $this->assertTrue($container->has(ConstructorExecutionTransformer::class)); $this->assertFalse($container->has(FilterInjectorTransformer::class)); } @@ -507,7 +521,6 @@ public function testRegisterTransformerServicesRegistersFilterInjectorTransforme $this->invokeProtected($kernel, 'registerTransformerServices', [$container]); $this->assertTrue($container->has(FilterInjectorTransformer::class)); - $this->assertFalse($container->has(ConstructorExecutionTransformer::class)); } /** @@ -554,7 +567,6 @@ private function registerPipeline(AspectKernel $kernel): Container $container = new Container(); $container->add(AspectKernel::class, $kernel); - $container->add('kernel.interceptFunctions', false); FrameworkServices::register($container); // Invoked on the kernel class itself, so an overridden hook is the one that runs (new ReflectionMethod($kernel, 'registerTransformerServices'))->invoke($kernel, $container); diff --git a/tests/Core/ContainerTest.php b/tests/Core/ContainerTest.php index e5f04c26..208879de 100644 --- a/tests/Core/ContainerTest.php +++ b/tests/Core/ContainerTest.php @@ -47,7 +47,6 @@ protected function setUp(): void 'containerClass' => Container::class, ]); $this->container->add(AspectKernel::class, $mockKernel); - $this->container->add('kernel.interceptFunctions', false); FrameworkServices::register($this->container); } @@ -73,6 +72,7 @@ public static function lazyInternalServices(): array AdviceMatcher::class => [AdviceMatcher::class], AspectLoader::class => [AspectLoader::class], CachedAspectLoader::class => [CachedAspectLoader::class], + AdvisorIndex::class => [AdvisorIndex::class], SyntaxTreeRewriter::class => [SyntaxTreeRewriter::class], // [CachePathManager::class], // Need to politely switch to options instead of whole kernel ]; diff --git a/tests/Fixtures/project/src/Application/MainFactory.php b/tests/Fixtures/project/src/Application/MainFactory.php new file mode 100644 index 00000000..2ead2ad2 --- /dev/null +++ b/tests/Fixtures/project/src/Application/MainFactory.php @@ -0,0 +1,15 @@ +assertClassStaticInitializationWoven(Main::class, 'Go\\Tests\\TestProject\\Aspect\\InitializationAspect->afterClassStaticInitialization'); } + /** + * The initialization pointcut of InitializationAspect turns the rewriting of `new` on by itself, no feature flag needed + */ + public function testInitializationAdviceRunsOnNewInWovenCode(): void + { + $phpExecutable = (new PhpExecutableFinder())->find(); + $script = sprintf( + 'include %s; ob_start(); $main = new %s()->create(); $output = ob_get_clean(); echo $main::class, "|", $output;', + var_export($this->configuration['frontController'], true), + '\\' . MainFactory::class, + ); + assert($phpExecutable !== false); + $process = new Process( + [$phpExecutable, '-r', $script], + null, + ['GO_AOP_CONFIGURATION' => $this->getConfigurationName()], + ); + $process->run(); + + $this->assertTrue( + $process->isSuccessful(), + 'Running the woven class failed: ' . $process->getOutput() . $process->getErrorOutput(), + ); + $this->assertSame(Main::class . '|It invokes before class instance is initialized.', trim($process->getOutput())); + } + public function testItWeavesFinalClasses(): void { // it weaves FinalClass class diff --git a/tests/Instrument/Transformer/ConstructorExecutionTransformerTest.php b/tests/Instrument/Transformer/ConstructorExecutionTransformerTest.php index ead4e1f2..7b0b35c9 100644 --- a/tests/Instrument/Transformer/ConstructorExecutionTransformerTest.php +++ b/tests/Instrument/Transformer/ConstructorExecutionTransformerTest.php @@ -12,7 +12,14 @@ namespace Go\Instrument\Transformer; +use Go\Aop\Advice; use Go\Aop\Exception\WeavingException; +use Go\Aop\Pointcut; +use Go\Aop\Pointcut\TruePointcut; +use Go\Aop\Support\GenericPointcutAdvisor; +use Go\Core\AdvisorIndex; +use Go\Core\AspectLoaderInterface; +use Go\Core\Container; use Go\Instrument\Transformer\Stubs\ConstructedStub; use Go\Instrument\Transformer\Stubs\InitializationAwareStub; use PhpParser\Node\Expr\New_; @@ -196,6 +203,46 @@ class_alias(ConstructedStub::class, $lateClass); } } + /** + * Without an initialization pointcut there is nothing to route a `new` to, so the rule stays off + */ + public function testRuleIsOffWithoutInitializationPointcut(): void + { + $container = new Container(); + $container->add('advisor.method', new GenericPointcutAdvisor(new TruePointcut(Pointcut::KIND_METHOD), $this->createStub(Advice::class))); + $rewriter = new SyntaxTreeRewriter(new ConstructorExecutionTransformer(new AdvisorIndex($container, $this->createAspectLoader()))); + $source = 'createMetadata($source); + + $this->assertSame(TransformerResult::Abstain, $rewriter->transform($metadata)); + $this->assertSame($source, $metadata->source); + } + + /** + * An initialization pointcut registered later, e.g. by a lazily loaded aspect, turns the rule on + */ + public function testRuleIsOnOnceInitializationPointcutIsRegistered(): void + { + $container = new Container(); + $rewriter = new SyntaxTreeRewriter(new ConstructorExecutionTransformer(new AdvisorIndex($container, $this->createAspectLoader()))); + $this->assertSame(TransformerResult::Abstain, $rewriter->transform($this->createMetadata('add('advisor.init', new GenericPointcutAdvisor($initPointcut, $this->createStub(Advice::class))); + $metadata = $this->createMetadata('assertSame(TransformerResult::Transformed, $rewriter->transform($metadata)); + $this->assertStringContainsString(ConstructorExecutionTransformer::class . '::getInstance()->{\stdClass::class}', $metadata->source); + } + + private function createAspectLoader(): AspectLoaderInterface + { + $loader = $this->createStub(AspectLoaderInterface::class); + $loader->method('getUnloadedAspects')->willReturn([]); + + return $loader; + } + private function createMetadata(string $source): StreamMetaData { $stream = fopen('php://input', 'r'); diff --git a/tests/Performance/TransformationPerformanceTest.php b/tests/Performance/TransformationPerformanceTest.php index 4a50732b..ef818eaf 100644 --- a/tests/Performance/TransformationPerformanceTest.php +++ b/tests/Performance/TransformationPerformanceTest.php @@ -119,7 +119,7 @@ public static function ruleSets(): iterable } /** - * @param bool $allRules Enables the rules of INTERCEPT_INITIALIZATIONS and INTERCEPT_INCLUDES too + * @param bool $allRules Enables the `new` rewriting (ungated) and INTERCEPT_INCLUDES rules too * @param float $maxRatio Budget: transformation time / plain walk time */ #[DataProvider('ruleSets')] From 6799df5cff362ece4ecd812e4b10bfc5050db551 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 21:17:13 +0000 Subject: [PATCH 2/5] test(core): assert the removed-feature message without PHPUnit 13.4-only API expectExceptionMessageIsOrContains() is missing on the lowest supported PHPUnit, and expectExceptionMessage() is deprecated on the newest one. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Hhj7Aqnac5xDA6L31WeJs8 --- tests/Core/AspectKernelTest.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/Core/AspectKernelTest.php b/tests/Core/AspectKernelTest.php index ec1fc4b3..55e9cddd 100644 --- a/tests/Core/AspectKernelTest.php +++ b/tests/Core/AspectKernelTest.php @@ -374,13 +374,15 @@ public function testNormalizeOptionsRejectsRemovedFeatures(int $features): void { $kernel = $this->makeKernel(); - $this->expectException(InvalidConfigurationException::class); - $this->expectExceptionMessageIsOrContains('Features INTERCEPT_FUNCTIONS and INTERCEPT_INITIALIZATIONS were removed'); - - $this->invokeProtected($kernel, 'normalizeOptions', [[ - 'cacheDir' => '/some/cache/dir', - 'features' => $features, - ]]); + try { + $this->invokeProtected($kernel, 'normalizeOptions', [[ + 'cacheDir' => '/some/cache/dir', + 'features' => $features, + ]]); + $this->fail('Removed features must be rejected'); + } catch (InvalidConfigurationException $exception) { + $this->assertStringContainsString('Features INTERCEPT_FUNCTIONS and INTERCEPT_INITIALIZATIONS were removed', $exception->getMessage()); + } } public function testNormalizeOptionsAcceptsEveryKnownFeature(): void From 71c442f27eb134b9013652ac75a55ef2da43413a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 21:20:27 +0000 Subject: [PATCH 3/5] test(weaving): cover the weaver loading aspects that are not loaded yet AdvisorIndex now loads the aspects before the weaver runs in the default pipeline, so the weaver's own loading path (kept for kernels that override the transformer services) lost its indirect coverage. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Hhj7Aqnac5xDA6L31WeJs8 --- .../Transformer/WeavingTransformerTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index 15325e63..0faa8b24 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -13,6 +13,7 @@ namespace Go\Instrument\Transformer; use Go\Aop\Advisor; +use Go\Aop\Aspect; use Go\Aop\Exception\WeavingException; use Go\Aop\Framework\BeforeInterceptor; use Go\Core\AdviceMatcherInterface; @@ -237,6 +238,21 @@ public function testUnadvisedClassWithUnknownParentIsNotTransformed(): void $this->assertSame($source, $metadata->source); } + /** + * Aspects not loaded yet are loaded before matching: the AdvisorIndex usually loads them earlier, but a kernel + * overriding registerTransformerServices() may run the weaver without the `new` rewriting rule + */ + public function testLoadsUnloadedAspectsBeforeMatching(): void + { + $aspect = $this->createStub(Aspect::class); + $loader = $this->createMock(AspectLoader::class); + $loader->method('getUnloadedAspects')->willReturn([$aspect]); + $loader->expects($this->once())->method('loadAndRegister')->with($aspect); + + $transformer = new WeavingTransformer($this->kernel, $this->adviceMatcher, $this->cachePathManager, $loader); + $transformer->transform($this->loadTestMetadata('class-typehint')); + } + /** * Main test case for method with typehint */ From e417662d69cca6a4282660c553c79ee299990300 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 21:53:46 +0000 Subject: [PATCH 4/5] refactor(core): drop the dedicated check for the removed feature bits Feature bit values are internal: bits 1 and 2 are rejected by the generic unknown-feature validation like any other undefined bit. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Hhj7Aqnac5xDA6L31WeJs8 --- CHANGELOG.md | 2 +- UPGRADE-4.0.md | 6 +++--- src/Aop/AGENTS.md | 2 +- src/Core/AspectKernel.php | 11 ----------- tests/Core/AspectKernelTest.php | 31 ------------------------------- 5 files changed, 5 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 510062d1..212f2dfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,7 @@ See [UPGRADE-4.0.md](UPGRADE-4.0.md) for the migration guide. * [Performance] **Proxy bodies built as AST** — the proxy generators build every intercepted method, function and (static) initialization body directly as syntax tree nodes (`static $__joinPoint` with its `@var` docblock, the `InterceptorInjector::for*()` call and the `__invoke()`), instead of printing each body to a string and parsing it back; the proxy is printed once. Intercepted methods are looked up in a name index built once per woven class, declared types are converted from the syntax tree without a round trip through reflection types, and attribute arguments are cloned by one shared traverser. Generated proxies are byte-identical; proxy generation takes about a third of its former time (weaving case of the transformation benchmark: −43%). `ClassProxyGenerator::getJoinpointInvocationBody()` is replaced by `getJoinpointInvocationStatements()` and `createOriginalMethodCallable()`. **Removed** -* [BC BREAK] Removed `Features::INTERCEPT_FUNCTIONS` and `Features::INTERCEPT_INITIALIZATIONS`: the framework enables function and initialization interception from the registered pointcuts themselves. `execution(Namespace\function(*))` pointcuts weave function proxies without a flag, and the rewriting of `new` turns itself on while an `initialization(...)` pointcut is registered, lazily registered aspects included (the new internal `AdvisorIndex` loads them and tracks the pointcut kinds of every advisor). Before, an `initialization(...)` pointcut without the flag wove `__initialization()` but never ran its advice. Passing one of the removed bits fails with a hint to drop it. +* [BC BREAK] Removed `Features::INTERCEPT_FUNCTIONS` and `Features::INTERCEPT_INITIALIZATIONS`: the framework enables function and initialization interception from the registered pointcuts themselves. `execution(Namespace\function(*))` pointcuts weave function proxies without a flag, and the rewriting of `new` turns itself on while an `initialization(...)` pointcut is registered, lazily registered aspects included (the new internal `AdvisorIndex` loads them and tracks the pointcut kinds of every advisor). Before, an `initialization(...)` pointcut without the flag wove `__initialization()` but never ran its advice. * [BC BREAK] Removed `Features::PARAMETER_WIDENING` and the parameter-widening code path in the proxy generators. The feature was a PHP 7.0/7.1 compatibility aid (parameter type widening, wiki.php.net/rfc/parameter-no-type-variance) that has been a no-op concern since the PHP 7.2 baseline: generated proxies always keep the original parameter types. Remove the flag from `AspectKernel::configureAop()` options if you passed it. * [BC BREAK] Removed DeclareError support, including the `DeclareError` attribute, `DeclareErrorInterceptor`, and `PointcutBuilder::declareError()`. Use `Before` or `Around` interceptors to emit user warnings or throw exceptions instead. * [BC BREAK] Removed support for the "dynamic" pointcut (`dynamic(public Foo->method*(*))`), including `MagicMethodDynamicPointcut`, `DynamicInvocationMatcherInterceptor`, the `Pointcut::KIND_DYNAMIC` constant and the `$instanceOrScope`/`$arguments` parameters of `Pointcut::matches()`. Use a traditional execution pointcut for the magic methods instead, e.g. `execution(public Foo->__call(*))` or `execution(public Foo::__callStatic(*))`, and check the invoked method name from `$invocation->getArguments()[0]` inside the advice. diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index ca15fcb5..b84d5285 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -184,9 +184,9 @@ that relied on that inheritance layer: passed it. - `Features::INTERCEPT_FUNCTIONS` and `Features::INTERCEPT_INITIALIZATIONS` are removed: function and initialization interception are enabled by the registered `execution(Namespace\function(*))` and - `initialization(...)` pointcuts. Delete the flags from your kernel options; passing them throws an - `InvalidConfigurationException`. While an `initialization(...)` pointcut is registered, every `new` in woven - files is rewritten, so an aspect that declares one without the flag starts intercepting object creation. + `initialization(...)` pointcuts. Delete the flags from your kernel options. While an `initialization(...)` + pointcut is registered, every `new` in woven files is rewritten, so an aspect that declares one without the + flag starts intercepting object creation. Removed unused API ------------------ diff --git a/src/Aop/AGENTS.md b/src/Aop/AGENTS.md index 92822dd7..1725c11a 100644 --- a/src/Aop/AGENTS.md +++ b/src/Aop/AGENTS.md @@ -69,6 +69,6 @@ when `$level > 0` and pops it in `finally`. Check every change with `composer te ## Features (src/Aop/Features.php) Final class with bitmask constants (`ALL` is the mask of every flag): -- INTERCEPT_INCLUDES=4 (bits 1/2 were INTERCEPT_FUNCTIONS/INTERCEPT_INITIALIZATIONS, removed: AspectKernel rejects them with a hint) +- INTERCEPT_INCLUDES=4 - No flag for function/initialization interception: pointcut kinds decide (AdvisorIndex, see src/Core/AGENTS.md) - PREBUILT_CACHE=64 — assume cache already prepared, skip freshness checks diff --git a/src/Core/AspectKernel.php b/src/Core/AspectKernel.php index 5c48fb34..42575612 100644 --- a/src/Core/AspectKernel.php +++ b/src/Core/AspectKernel.php @@ -54,11 +54,6 @@ */ abstract class AspectKernel { - /** - * Bits of the removed INTERCEPT_FUNCTIONS (1) and INTERCEPT_INITIALIZATIONS (2) features - */ - private const int REMOVED_FEATURES = 1 | 2; - /** * Composer packages the framework runs on during weaving, always excluded from weaving */ @@ -323,12 +318,6 @@ protected function normalizeOptions(array $options): array )); } $features = $merged['features'] ?? 0; - if (is_int($features) && ($features & self::REMOVED_FEATURES) !== 0) { - throw new InvalidConfigurationException( - 'Features INTERCEPT_FUNCTIONS and INTERCEPT_INITIALIZATIONS were removed, remove them from the "features" ' - . 'option: function and initialization pointcuts are enabled automatically by the registered aspects.', - ); - } if (!is_int($features) || ($features & ~Features::ALL) !== 0) { throw new InvalidConfigurationException(sprintf( 'Option "features" must be a combination of Go\\Aop\\Features constants, got %s.', diff --git a/tests/Core/AspectKernelTest.php b/tests/Core/AspectKernelTest.php index 55e9cddd..6a93f3da 100644 --- a/tests/Core/AspectKernelTest.php +++ b/tests/Core/AspectKernelTest.php @@ -199,8 +199,6 @@ public static function unknownFeatures(): iterable yield 'zero' => [0]; yield 'undefined bit' => [8]; yield 'defined and undefined bits' => [Features::PREBUILT_CACHE | 128]; - yield 'removed INTERCEPT_FUNCTIONS bit' => [1]; - yield 'removed INTERCEPT_INITIALIZATIONS bit' => [2]; } #[DataProvider('unknownFeatures')] @@ -356,35 +354,6 @@ public function testNormalizeOptionsRejectsInvalidFeatures(mixed $features): voi ]]); } - /** - * @return iterable - */ - public static function removedFeatures(): iterable - { - yield 'INTERCEPT_FUNCTIONS' => [1]; - yield 'INTERCEPT_INITIALIZATIONS' => [2]; - yield 'removed and known bits' => [2 | Features::PREBUILT_CACHE]; - } - - /** - * Function and initialization pointcuts need no feature flag anymore, the removed flags point to that - */ - #[DataProvider('removedFeatures')] - public function testNormalizeOptionsRejectsRemovedFeatures(int $features): void - { - $kernel = $this->makeKernel(); - - try { - $this->invokeProtected($kernel, 'normalizeOptions', [[ - 'cacheDir' => '/some/cache/dir', - 'features' => $features, - ]]); - $this->fail('Removed features must be rejected'); - } catch (InvalidConfigurationException $exception) { - $this->assertStringContainsString('Features INTERCEPT_FUNCTIONS and INTERCEPT_INITIALIZATIONS were removed', $exception->getMessage()); - } - } - public function testNormalizeOptionsAcceptsEveryKnownFeature(): void { $kernel = $this->makeKernel(); From b2816cd219470d4d9d7316d1523f9bc1ee93aa65 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 21:54:28 +0000 Subject: [PATCH 5/5] docs(upgrade): drop the note on the removed interception flags Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Hhj7Aqnac5xDA6L31WeJs8 --- UPGRADE-4.0.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index b84d5285..bcf47ccd 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -182,11 +182,6 @@ that relied on that inheritance layer: be implemented. `Features::ALL` combines every flag, and `AspectKernel::hasFeature()` throws on an unknown flag. - `Features::PARAMETER_WIDENING` is removed. Delete the flag from your kernel options if you passed it. -- `Features::INTERCEPT_FUNCTIONS` and `Features::INTERCEPT_INITIALIZATIONS` are removed: function and - initialization interception are enabled by the registered `execution(Namespace\function(*))` and - `initialization(...)` pointcuts. Delete the flags from your kernel options. While an `initialization(...)` - pointcut is registered, every `new` in woven files is rewritten, so an aspect that declares one without the - flag starts intercepting object creation. Removed unused API ------------------