diff --git a/CHANGELOG.md b/CHANGELOG.md index c3d9d50b..212f2dfe 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. * [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/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..1725c11a 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 +- 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..42575612 100644 --- a/src/Core/AspectKernel.php +++ b/src/Core/AspectKernel.php @@ -150,7 +150,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. @@ -379,9 +378,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 +395,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..6a93f3da 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)); } @@ -337,7 +335,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]; } @@ -359,8 +357,7 @@ public function testNormalizeOptionsRejectsInvalidFeatures(mixed $features): voi 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 +413,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 +421,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 +466,7 @@ public function testNormalizeOptionsIgnoresNonStringEntriesInPathLists(): void } } - public function testRegisterTransformerServicesAlwaysRegistersWeavingAndMagicConstantTransformers(): void + public function testRegisterTransformerServicesAlwaysRegistersWeavingConstructorAndMagicConstantTransformers(): void { $kernel = $this->makeKernel(); @@ -480,19 +477,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 +492,6 @@ public function testRegisterTransformerServicesRegistersFilterInjectorTransforme $this->invokeProtected($kernel, 'registerTransformerServices', [$container]); $this->assertTrue($container->has(FilterInjectorTransformer::class)); - $this->assertFalse($container->has(ConstructorExecutionTransformer::class)); } /** @@ -554,7 +538,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/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 */ 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')]