From e59523474fa94cb4ab0c1120a4335a69bb0b445f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 13:59:18 +0000 Subject: [PATCH 1/7] feat(core)!: weave without creating aspects The weaver collected aspects as objects: AspectLoader asked the container for every Aspect service, and AttributeAspectLoaderExtension bound each advice method to that instance. A deferred aspect stayed an uninitialized lazy proxy, but PHP can not make a class without instance properties lazy, so the factory of such an aspect ran as soon as weaving started, before an application container attached with setDelegate() existed. Integration bridges had to create it behind the back of the application container or fail (goaop/goaop-yii3-bridge#3). Aspects are now loaded by class. AspectContainer::getServiceIdsByInterface() lists the aspect ids without creating the services, loader extensions get the ReflectionClass only, and advices are built with the lazy form of the Interceptor facade. Its proxies carry the aspect class, the advice method, the order and the expression, which ordering, GeneratedInterceptor and the advisor cache compiler read without initializing them. Every aspect is created on the first call of one of its advices. BREAKING CHANGE: AspectLoaderExtension::load() takes the ReflectionClass of the aspect only, AbstractAspectLoaderExtension::parsePointcut() the aspect class name, AspectLoaderInterface works with aspect class names, and AspectContainer implementations must provide getServiceIdsByInterface(). Aspects must be registered under their class name. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe --- CHANGELOG.md | 1 + UPGRADE-4.0.md | 19 ++++++++ docs/configuration.md | 7 +-- src/Aop/AGENTS.md | 2 +- src/Aop/Framework/AbstractInterceptor.php | 32 ++++++++++--- src/Aop/Framework/GeneratedInterceptor.php | 12 +++++ src/Aop/Framework/Interceptor.php | 26 ++++++++++- src/Console/Command/DebugAspectCommand.php | 32 +++++++------ src/Core/AGENTS.md | 3 +- src/Core/AbstractAspectLoaderExtension.php | 10 +++-- src/Core/AspectContainer.php | 13 ++++++ src/Core/AspectLoader.php | 26 ++++++----- src/Core/AspectLoaderExtension.php | 6 ++- src/Core/AspectLoaderInterface.php | 15 +++++-- src/Core/AttributeAspectLoaderExtension.php | 42 +++++++++-------- src/Core/Cache/CachedAspectLoader.php | 28 ++++++------ src/Core/Container.php | 13 ++++++ src/Core/IntroductionAspectExtension.php | 7 ++- .../Transformer/WeavingTransformer.php | 2 +- tests/Aop/Framework/InterceptorTest.php | 20 +++++++++ .../DebugAdvisorCommandInProcessTest.php | 2 +- .../DebugAspectCommandInProcessTest.php | 10 ++--- .../AbstractAspectLoaderExtensionTest.php | 23 +++++----- tests/Core/AdvisorIndexTest.php | 45 ++++++++++++++++--- tests/Core/AspectLoaderTest.php | 40 +++++++++++------ .../AttributeAspectLoaderExtensionTest.php | 42 ++++++++++++++--- tests/Core/Cache/CachedAspectLoaderTest.php | 34 +++++++------- tests/Core/ContainerTest.php | 22 +++++++++ .../Core/IntroductionAspectExtensionTest.php | 13 +++--- .../Transformer/WeavingTransformerTest.php | 2 +- 30 files changed, 396 insertions(+), 153 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 434f0ba42..9d80b9461 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ See [UPGRADE-4.0.md](UPGRADE-4.0.md) for the migration guide. * [Feature] [BC BREAK] **Interface-keyed registration listeners** — new `AspectContainer::onRegistration(string $interfaceFQCN, Closure $listener)`: the listener receives the id (class-name) and the container whenever a deferred service whose id implements the interface is registered via `addLazyService()`; matching never autoloads, so an id whose class is not loaded yet reaches the listener as a candidate. Values are never touched, so laziness is preserved; with no listeners armed (production) registration stays a pure array write. The kernel arms one `Aspect::class` listener in debug mode to track aspect source files as freshness resources at registration time, resolving them through composer without loading the aspects (previously hardwired into `registerAspect()`). `AspectContainer::addResource()` is now part of the interface and `final public` on `Container` (was `final protected`). * [Feature] [BC BREAK] **PSR-11 container** — `AspectContainer` extends `Psr\Container\ContainerInterface`: `getValue()` is renamed to `get()`, `has()` is inherited from the PSR interface. `ServiceNotFoundException` implements `NotFoundExceptionInterface`, and a container entry of the wrong type in `getService()` throws the new `Go\Aop\Exception\ContainerException` (an `InvalidConfigurationException` implementing `ContainerExceptionInterface`). * [Feature] **Aspects from the application container** — `AspectContainer::setDelegate(ContainerInterface $delegate)` attaches the PSR-11 container of the application: `get()`/`getService()`/`has()` fall back to it for unknown ids. Aspects are registered in `configureAop()` with `addLazyService()` as before, and their factories resolve dependencies from the delegate on first use, once the application has built its container; weaving and cache warm-up never need it. The delegate is never enumerated (PSR-11 has no lookup by interface or tag), framework bridges can collect tagged services and register them. +* [Feature] [BC BREAK] **Weaving never creates aspects** — the aspect loader reads aspects from their classes and builds every aspect-method advice in the lazy static-data form of the `Interceptor` facade, which exposes the aspect class, the advice method, the order and the expression without resolving the aspect. An aspect is created on the first call of one of its advices, also one without instance properties, whose factory used to run as soon as the weaver collected the aspects (before an application container attached with `setDelegate()` existed). New `AspectContainer::getServiceIdsByInterface()` lists service ids without creating the services. `AspectLoaderExtension::load()` takes the `ReflectionClass` of the aspect only, `AbstractAspectLoaderExtension::parsePointcut()` the aspect class name. * [Feature] Console commands declare their name, description and help with `#[AsCommand]`, `bin/aspect` loads them lazily, and every command returns `Command::SUCCESS`/`Command::FAILURE` (`debug:weaving` used to return the raw error count). `cache:warmup:aop` stops cleanly on SIGINT/SIGTERM. * [Feature] [BC BREAK] **Doctrine bridge rewritten for the trait-based engine (#671)** — new `Go\Bridge\Doctrine\WovenEntityClassLocator` lets Doctrine mapping drivers (ORM 3.6+) discover entities through the autoloader, so the woven class is mapped instead of the original source, and entities loaded before discovery are no longer missed. `MetadataLoadInterceptor` no longer rewrites metadata (the old mapped-superclass workaround only matched the extend-based engine); it now reports woven entities with intercepted mapped properties when Doctrine native lazy objects are disabled. It is registered as a plain listener: `$eventManager->addEventListener(Events::loadClassMetadata, new MetadataLoadInterceptor())` (it is no longer an `EventSubscriber`). diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index 7bad58a02..542e76b18 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -114,6 +114,25 @@ object of the wrong type now fails on first use, not at registration. `AspectContainer::addResource()` is part of the interface. `onRegistration()` lets you react to deferred registrations of services implementing an interface. +Weaving never creates aspects: the aspect loader reads an aspect from its class, and each advice refers to it by +class and method name. A deferred aspect is created on the first call of one of its advices, including an aspect +without instance properties, which PHP can not make lazy (its factory used to run as soon as the weaver collected +the aspects). Register aspects under their class name: generated code resolves them with +`$container->getService(MonitorAspect::class)`. `AspectContainer::getServiceIdsByInterface()` lists the ids of the +services implementing an interface without creating them; a custom `containerClass` implements it too. + +Custom aspect loader extensions get the reflection of the aspect only: + +```php +// before +public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array + +// 4.0 +public function load(ReflectionClass $reflectionAspect): array +``` + +`AbstractAspectLoaderExtension::parsePointcut()` takes the aspect class name instead of the aspect instance. + ### PSR-11 container `AspectContainer` extends `Psr\Container\ContainerInterface`, so the container can be passed to any code diff --git a/docs/configuration.md b/docs/configuration.md index 58513e914..9d3a64bd1 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -66,7 +66,7 @@ ApplicationAspectKernel::getInstance()->getContainer()->setDelegate($psrContaine ``` Weaving and the cache warm-up only need the class of the aspect, so they run before `setDelegate()`, even in -`bin/aspect`. The factory runs on the first advice call; a dependency that is still missing then throws +`bin/aspect`, and never create an aspect. The factory runs on the first advice call; a dependency that is still missing then throws `ServiceNotFoundException`. - A factory never asks the aspect container for its own id: that id resolves to the aspect being built. @@ -75,8 +75,9 @@ Weaving and the cache warm-up only need the class of the aspect, so they run bef (some containers can, each in its own way). Aspects are therefore never discovered in the delegate, every aspect is registered in `configureAop()`. An integration bridge for a specific framework can collect the services by tag or interface from its container and register them with `addLazyService()`. -- An aspect without instance properties cannot be created lazily, so its factory runs when the aspects are - collected for weaving, before the delegate exists. Such an aspect has no dependencies to inject. +- Weaving never creates an aspect: it reads the aspect class, and the advices refer to the aspect by class and + method name. Every aspect, also one without instance properties (which PHP can not make lazy), is created by its + factory on the first call of one of its advices, once the application container is attached. ## Extending the transformation pipeline diff --git a/src/Aop/AGENTS.md b/src/Aop/AGENTS.md index 20dc5c08f..7e1a02838 100644 --- a/src/Aop/AGENTS.md +++ b/src/Aop/AGENTS.md @@ -48,7 +48,7 @@ when `$level > 0` and pops it in `finally`. Check every change with `composer te ## Advice wiring (src/Aop/Framework/) - The — proxy-code accessor: aspect(X::class) fetches aspect from container; advice('advisorId') resolves container-backed closure advice (unwraps Advisor/AbstractInterceptor to raw Closure) -- Interceptor — @internal factory facade with TWO construction modes: before()/after()/around()/afterThrowing(class-string|Closure, ?string $methodName=null, int $order=0, string $expression=''). With aspect class + method name it returns a native PHP 8.4 lazy proxy (via Go\Core\NativeLazyProxy::create) — interceptor construction, The::aspect() resolution and FCC creation all defer until first real use (invocation/ordering), so unmatched advices never instantiate their aspect; ONLY compiled advisor cache files use this lazy form. A ready Closure constructs eagerly, and generated PROXY classes deliberately use the eager `The::aspect(X::class)->method(...)` FCC form (InterceptorListGenerator): the proxy method/hook is already executing, so the interceptor is needed right now and a lazy detour would be pure overhead; The::advice('') stays eager too. Free to change between releases +- Interceptor — @internal factory facade with TWO construction modes: before()/after()/around()/afterThrowing(class-string|Closure, ?string $methodName=null, int $order=0, string $expression=''). With aspect class + method name it returns a native PHP 8.4 lazy proxy (via Go\Core\NativeLazyProxy::create) — interceptor construction, The::aspect() resolution and FCC creation all defer until the advice is invoked; adviceOrder, pointcutExpression, aspectClass and adviceMethodName are set on the proxy with setRawValueWithoutLazyInitialization(), so ordering, GeneratedInterceptor::fromAdvice() and compileToPhp() read them without initializing it. The aspect loader (AttributeAspectLoaderExtension) and compiled advisor cache files use this lazy form, so weaving never creates an aspect. A ready Closure constructs eagerly, and generated PROXY classes deliberately use the eager `The::aspect(X::class)->method(...)` FCC form (InterceptorListGenerator): the proxy method/hook is already executing, so the interceptor is needed right now and a lazy detour would be pure overhead; The::advice('') stays eager too. Free to change between releases - GeneratedInterceptor — internal descriptor built by AbstractJoinpoint::flatAndSortAdvices() via fromAdvice(); usesContainerAdvice=true when advice closure isn't scoped to an Aspect class - AdviceTypeEnum — Advice::getType() kind + sorting priority (before → after/afterThrowing → around → introduction); replaced AdviceBefore/AdviceAfter/AdviceAround marker interfaces - Advice methods MUST be public (FCC calls them on the aspect instance from generated code) diff --git a/src/Aop/Framework/AbstractInterceptor.php b/src/Aop/Framework/AbstractInterceptor.php index cb200fae3..4b85e98ff 100644 --- a/src/Aop/Framework/AbstractInterceptor.php +++ b/src/Aop/Framework/AbstractInterceptor.php @@ -68,6 +68,18 @@ public function __construct( * @internal Framework-internal introspection point (e.g. debug:advisor command), not a public API */ public private(set) string $pointcutExpression = '', + /** + * @var class-string|null Aspect class of an advice method, null for a plain closure advice + * + * @internal Set by the lazy {@see Interceptor} facade: the weaver reads it without creating the aspect + */ + public private(set) ?string $aspectClass = null, + /** + * Name of the advice method of {@see $aspectClass} + * + * @internal Set together with {@see $aspectClass} + */ + public private(set) ?string $adviceMethodName = null, ) {} /** @@ -90,10 +102,18 @@ public function getRawAdvice(): Closure */ final public function compileToPhp(): Expr { - $reflectionAdvice = new ReflectionFunction($this->adviceMethod); - $scopeReflectionClass = $reflectionAdvice->getClosureScopeClass(); - if (!isset($scopeReflectionClass) || !is_subclass_of($scopeReflectionClass->name, Aspect::class)) { - throw NotCompilableException::interceptorWithoutAspect(); + // A lazy interceptor knows its aspect method: reading the advice closure would create the aspect + if ($this->aspectClass !== null && $this->adviceMethodName !== null) { + $aspectClass = $this->aspectClass; + $methodName = $this->adviceMethodName; + } else { + $reflectionAdvice = new ReflectionFunction($this->adviceMethod); + $scopeReflectionClass = $reflectionAdvice->getClosureScopeClass(); + if (!isset($scopeReflectionClass) || !is_subclass_of($scopeReflectionClass->name, Aspect::class)) { + throw NotCompilableException::interceptorWithoutAspect(); + } + $aspectClass = $scopeReflectionClass->name; + $methodName = $reflectionAdvice->name; } $factoryMethod = match ($this::class) { @@ -105,8 +125,8 @@ final public function compileToPhp(): Expr }; $args = [ - new Arg(new ClassConstFetch(new FullyQualified($scopeReflectionClass->name), 'class')), - new Arg(new String_($reflectionAdvice->name)), + new Arg(new ClassConstFetch(new FullyQualified($aspectClass), 'class')), + new Arg(new String_($methodName)), ]; if ($this->adviceOrder !== 0) { $args[] = new Arg(new Int_($this->adviceOrder), name: new Identifier('order')); diff --git a/src/Aop/Framework/GeneratedInterceptor.php b/src/Aop/Framework/GeneratedInterceptor.php index 4ca0e4b57..0f18b823c 100644 --- a/src/Aop/Framework/GeneratedInterceptor.php +++ b/src/Aop/Framework/GeneratedInterceptor.php @@ -39,6 +39,18 @@ public static function fromAdvice(string $advisorId, Advice $advice): self throw AspectException::unsupportedAdvice($advisorId, $advice); } + // A lazy interceptor knows its aspect method: reading the advice closure would create the aspect + if ($advice->aspectClass !== null && $advice->adviceMethodName !== null) { + return new self( + $advice->getType()->value, + $advice->aspectClass, + $advice->adviceMethodName, + $advice->adviceOrder, + $advisorId, + false, + ); + } + $reflectionAdvice = new ReflectionFunction($advice->getRawAdvice()); $scopeClass = $reflectionAdvice->getClosureScopeClass(); $usesContainerAdvice = $scopeClass === null || !is_subclass_of($scopeClass->name, Aspect::class); diff --git a/src/Aop/Framework/Interceptor.php b/src/Aop/Framework/Interceptor.php index c8ed9a7a1..0fe9ceddd 100644 --- a/src/Aop/Framework/Interceptor.php +++ b/src/Aop/Framework/Interceptor.php @@ -16,6 +16,7 @@ use Go\Aop\Aspect; use Go\Aop\Exception\AspectException; use Go\Core\NativeLazyProxy; +use ReflectionProperty; /** * Factory facade for generated interceptor declarations, with two construction modes. @@ -37,6 +38,13 @@ */ final class Interceptor { + /** + * Properties describing the advice, set on the lazy proxies without initializing them + * + * @var array{ReflectionProperty, ReflectionProperty, ReflectionProperty, ReflectionProperty}|null + */ + private static ?array $descriptionProperties = null; + /** * @param class-string|Closure $aspectClassOrAdvice */ @@ -95,13 +103,29 @@ private static function createLazily( // Interceptor classes are framework-owned and lazy-compatible by construction, // so they take the probe-free trusted path. - return NativeLazyProxy::create( + $interceptor = NativeLazyProxy::create( $interceptorClass, static fn(): AbstractInterceptor => new $interceptorClass( The::aspect($aspectClassOrAdvice)->$methodName(...), $order, $expression, + $aspectClassOrAdvice, + $methodName, ), ); + // The descriptive data is readable without initializing the proxy: weaving, ordering and cache compilation + // read it, the aspect is only created when the advice runs + [$orderProperty, $expressionProperty, $aspectProperty, $methodProperty] = self::$descriptionProperties ??= [ + new ReflectionProperty(AbstractInterceptor::class, 'adviceOrder'), + new ReflectionProperty(AbstractInterceptor::class, 'pointcutExpression'), + new ReflectionProperty(AbstractInterceptor::class, 'aspectClass'), + new ReflectionProperty(AbstractInterceptor::class, 'adviceMethodName'), + ]; + $orderProperty->setRawValueWithoutLazyInitialization($interceptor, $order); + $expressionProperty->setRawValueWithoutLazyInitialization($interceptor, $expression); + $aspectProperty->setRawValueWithoutLazyInitialization($interceptor, $aspectClassOrAdvice); + $methodProperty->setRawValueWithoutLazyInitialization($interceptor, $methodName); + + return $interceptor; } } diff --git a/src/Console/Command/DebugAspectCommand.php b/src/Console/Command/DebugAspectCommand.php index 820c11756..243e2cee0 100644 --- a/src/Console/Command/DebugAspectCommand.php +++ b/src/Console/Command/DebugAspectCommand.php @@ -16,7 +16,7 @@ use Go\Aop\Aspect; use Go\Aop\Pointcut; use Go\Core\AspectLoader; -use ReflectionObject; +use ReflectionClass; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -61,16 +61,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int $aspectName = $input->getOption('aspect'); if (!is_string($aspectName) || $aspectName === '') { $io->text('' . $this->aspectKernel::class . ' has following enabled aspects:'); - $aspects = $container->getServicesByInterface(Aspect::class); + $aspects = $container->getServiceIdsByInterface(Aspect::class); } else { $aspectName = ltrim($aspectName, '\\'); if (!$container->has($aspectName) || !is_subclass_of($aspectName, Aspect::class)) { - $registeredAspects = array_keys($container->getServicesByInterface(Aspect::class)); + $registeredAspects = $container->getServiceIdsByInterface(Aspect::class); $io->error(sprintf('Aspect "%s" is not registered in the kernel.', $aspectName) . self::suggestAlternative($aspectName, $registeredAspects)); return Command::FAILURE; } - $aspects[] = $container->getService($aspectName); + $aspects[] = $aspectName; } $this->showRegisteredAspectsInfo($io, $aspects); @@ -78,23 +78,27 @@ protected function execute(InputInterface $input, OutputInterface $output): int } /** - * Shows an information about registered aspects + * Shows an information about registered aspects, read from their classes: no aspect is created * - * @param Aspect[] $aspects List of aspects + * @param list $aspects Classes of the aspects */ private function showRegisteredAspectsInfo(SymfonyStyle $io, array $aspects): void { - foreach ($aspects as $aspect) { - $this->showAspectInfo($io, $aspect); + foreach ($aspects as $aspectClass) { + if (is_subclass_of($aspectClass, Aspect::class)) { + $this->showAspectInfo($io, $aspectClass); + } } } /** * Displays an information about single aspect + * + * @param class-string $aspectClass */ - private function showAspectInfo(SymfonyStyle $io, Aspect $aspect): void + private function showAspectInfo(SymfonyStyle $io, string $aspectClass): void { - $refAspect = new ReflectionObject($aspect); + $refAspect = new ReflectionClass($aspectClass); $aspectName = $refAspect->getName(); $io->section($aspectName); $io->writeln('Defined in: ' . $refAspect->getFileName() . ''); @@ -102,20 +106,22 @@ private function showAspectInfo(SymfonyStyle $io, Aspect $aspect): void if ($docComment) { $io->writeln($this->getPrettyText($docComment)); } - $this->showAspectPointcutsAndAdvisors($io, $aspect); + $this->showAspectPointcutsAndAdvisors($io, $aspectClass); } /** * Shows an information about aspect pointcuts and advisors + * + * @param class-string $aspectClass */ - private function showAspectPointcutsAndAdvisors(SymfonyStyle $io, Aspect $aspect): void + private function showAspectPointcutsAndAdvisors(SymfonyStyle $io, string $aspectClass): void { $container = $this->aspectKernel->getContainer(); $aspectLoader = $container->getService(AspectLoader::class); $io->writeln('Pointcuts and advices'); - $aspectItems = $aspectLoader->load($aspect); + $aspectItems = $aspectLoader->load($aspectClass); $aspectItemsInfo = []; foreach ($aspectItems as $itemId => $item) { $itemType = 'Unknown'; diff --git a/src/Core/AGENTS.md b/src/Core/AGENTS.md index 969c4a644..3684a2bb7 100644 --- a/src/Core/AGENTS.md +++ b/src/Core/AGENTS.md @@ -1,7 +1,7 @@ # src/Core — Container and aspect loading ## Container (Container.php) -- Generic, AOP-unaware DI container, PSR-11 (AspectContainer extends Psr\Container\ContainerInterface): add(by class-string|key), get(), has(), getService(), addLazyService(Closure), setDelegate(), onRegistration(interfaceFQCN, listener), addResource() +- Generic, AOP-unaware DI container, PSR-11 (AspectContainer extends Psr\Container\ContainerInterface): add(by class-string|key), get(), has(), getService(), addLazyService(Closure), setDelegate(), onRegistration(interfaceFQCN, listener), addResource(), getServiceIdsByInterface() (ids of eager/materialized entries, then matching deferred ids; never materializes) - Delegate (app's PSR-11 container, attached after init via setDelegate()): get()/has() fall back to it on a miss only (hot path unchanged, delegated values are not stored); no dedicated registration API: addLazyService() factories resolve dependencies through get()/getService() on first use, so registration/weaving/warm-up never need the delegate. getServicesByInterface() never enumerates it (PSR-11 has no lookup by interface/tag; bridges collect tagged services and register them) - Automatic tagging by interface; deferred services materialize as native lazy proxies via NativeLazyProxy (engine probe, no userland compatibility predicate) - add() runs on every request: tags via class_implements(), the class file of an added object becomes a resource only on the next isFreshSince() (no reflection or stat calls at registration) @@ -11,6 +11,7 @@ ## Aspect loading - AspectLoader — scans aspect classes for pointcut/advice attributes → Advisor[]; CachedAspectLoader decorates it (both implement AspectLoaderInterface) +- Aspects are loaded BY CLASS, never by instance: getUnloadedAspects() lists ids via AspectContainer::getServiceIdsByInterface() (no factory runs), AspectLoaderExtension::load(ReflectionClass) reads attributes only, AttributeAspectLoaderExtension builds advices with the lazy Interceptor facade (aspectClass/adviceMethodName/order/expression readable without initialization). Weaving, warm-up and the advisor cache never create an aspect — also not one PHP can't make lazy (no instance properties); it is created on its first advice call (`The::aspect()`). Never reintroduce `getServicesByInterface(Aspect::class)` or `getClosure($aspect)` on the weaving path - 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 diff --git a/src/Core/AbstractAspectLoaderExtension.php b/src/Core/AbstractAspectLoaderExtension.php index 31ded0296..4e6d1a5a3 100644 --- a/src/Core/AbstractAspectLoaderExtension.php +++ b/src/Core/AbstractAspectLoaderExtension.php @@ -41,15 +41,16 @@ public function __construct( * General method for parsing pointcuts * * @throws PointcutSyntaxException if there was an error during parsing + * @param class-string $aspectClass Aspect the expression belongs to, `$this` in it * @param ReflectionMethod|ReflectionProperty|ReflectionClass $reflection * @template T of Aspect */ final protected function parsePointcut( - Aspect $aspect, + string $aspectClass, ReflectionMethod|ReflectionProperty|ReflectionClass $reflection, string $pointcutExpression, ): Pointcut { - $stream = $this->makeLexicalAnalyze($aspect, $reflection, $pointcutExpression); + $stream = $this->makeLexicalAnalyze($aspectClass, $reflection, $pointcutExpression); return $this->parseTokenStream($reflection, $pointcutExpression, $stream); } @@ -57,18 +58,19 @@ final protected function parsePointcut( /** * Performs lexical analyze of pointcut * + * @param class-string $aspectClass * @param ReflectionMethod|ReflectionProperty|ReflectionClass $reflection * @template T of Aspect * * @throws PointcutSyntaxException */ private function makeLexicalAnalyze( - Aspect $aspect, + string $aspectClass, ReflectionMethod|ReflectionProperty|ReflectionClass $reflection, string $pointcutExpression, ): TokenStream { try { - $resolvedThisPointcut = str_replace('$this', $aspect::class, $pointcutExpression); + $resolvedThisPointcut = str_replace('$this', $aspectClass, $pointcutExpression); $stream = $this->pointcutLexer->lex($resolvedThisPointcut); } catch (RecognitionException $e) { throw PointcutSyntaxException::unrecognizedLexicalStructure($pointcutExpression, $reflection, $e); diff --git a/src/Core/AspectContainer.php b/src/Core/AspectContainer.php index 84894e230..cddda32a5 100644 --- a/src/Core/AspectContainer.php +++ b/src/Core/AspectContainer.php @@ -99,6 +99,19 @@ public function getService(string $className): object; */ public function getServicesByInterface(string $interfaceTagClassName): array; + /** + * Returns the ids of the services tagged with the marker interface, without creating any of them + * + * Unlike {@see getServicesByInterface()}, a deferred service is not materialized: its id (a class-name) is matched + * against the interface, which autoloads the class but never runs its factory. The weaver reads aspects this way, + * by class, so an aspect is only created when one of its advices runs. + * + * @param class-string $interfaceTagClassName Interface name of services to look up + * + * @return list Ids of eagerly added and materialized services first, then the deferred ones + */ + public function getServiceIdsByInterface(string $interfaceTagClassName): array; + /** * Returns a value from the container * diff --git a/src/Core/AspectLoader.php b/src/Core/AspectLoader.php index 1e236903d..f44e2129f 100644 --- a/src/Core/AspectLoader.php +++ b/src/Core/AspectLoader.php @@ -49,15 +49,17 @@ public function __construct( * * @see loadAndRegister() method for registration * + * @param class-string $aspectClass + * * @return array */ - public function load(Aspect $aspect): array + public function load(string $aspectClass): array { - $refAspect = new ReflectionClass($aspect); + $refAspect = new ReflectionClass($aspectClass); $loadedItems = []; foreach ($this->aspectLoaders as $loader) { - $loadedItems += $loader->load($aspect, $refAspect); + $loadedItems += $loader->load($refAspect); } return $loadedItems; @@ -65,28 +67,30 @@ public function load(Aspect $aspect): array /** * Loads and register all items of aspect in the container + * + * @param class-string $aspectClass */ - public function loadAndRegister(Aspect $aspect): void + public function loadAndRegister(string $aspectClass): void { - $loadedItems = $this->load($aspect); + $loadedItems = $this->load($aspectClass); foreach ($loadedItems as $itemId => $item) { $this->container->add($itemId, $item); } - $this->loadedAspects[$aspect::class] = $aspect::class; + $this->loadedAspects[$aspectClass] = $aspectClass; } /** - * Returns list of unloaded aspects in the container + * Returns the classes of the aspects registered in the container that are not loaded yet * - * @return list + * @return list> */ public function getUnloadedAspects(): array { $unloadedAspects = []; - foreach ($this->container->getServicesByInterface(Aspect::class) as $aspect) { - if (!isset($this->loadedAspects[$aspect::class])) { - $unloadedAspects[] = $aspect; + foreach ($this->container->getServiceIdsByInterface(Aspect::class) as $aspectClass) { + if (!isset($this->loadedAspects[$aspectClass]) && is_subclass_of($aspectClass, Aspect::class)) { + $unloadedAspects[] = $aspectClass; } } diff --git a/src/Core/AspectLoaderExtension.php b/src/Core/AspectLoaderExtension.php index 6af6b8b0b..a9160933d 100644 --- a/src/Core/AspectLoaderExtension.php +++ b/src/Core/AspectLoaderExtension.php @@ -25,12 +25,14 @@ interface AspectLoaderExtension /** * Loads definition from specific point of aspect into the container * - * @param T $aspect Instance of aspect + * Only the class is read, the aspect is never created here: advices refer to it by class and method name, the + * aspect is resolved from the container when one of them runs for the first time. + * * @param ReflectionClass $reflectionAspect Reflection of aspect * * @return array * * @template T of Aspect */ - public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array; + public function load(ReflectionClass $reflectionAspect): array; } diff --git a/src/Core/AspectLoaderInterface.php b/src/Core/AspectLoaderInterface.php index f0b6ce5a0..2b170c20d 100644 --- a/src/Core/AspectLoaderInterface.php +++ b/src/Core/AspectLoaderInterface.php @@ -26,21 +26,28 @@ interface AspectLoaderInterface /** * Loads an aspect with the help of aspect loaders, but don't register it in the container * + * Only the class is read: the aspect itself is never created, its advices resolve it from the container when + * they run for the first time. + * * @see loadAndRegister() method for registration * + * @param class-string $aspectClass + * * @return array */ - public function load(Aspect $aspect): array; + public function load(string $aspectClass): array; /** * Loads and register all items of aspect in the container + * + * @param class-string $aspectClass */ - public function loadAndRegister(Aspect $aspect): void; + public function loadAndRegister(string $aspectClass): void; /** - * Returns list of unloaded aspects in the container + * Returns the classes of the aspects registered in the container that are not loaded yet * - * @return list + * @return list> */ public function getUnloadedAspects(): array; } diff --git a/src/Core/AttributeAspectLoaderExtension.php b/src/Core/AttributeAspectLoaderExtension.php index 5aeac85ac..6529af1e3 100644 --- a/src/Core/AttributeAspectLoaderExtension.php +++ b/src/Core/AttributeAspectLoaderExtension.php @@ -14,11 +14,8 @@ use Go\Aop\Aspect; use Go\Aop\Exception\AspectException; -use Go\Aop\Framework\AfterInterceptor; -use Go\Aop\Framework\AfterThrowingInterceptor; -use Go\Aop\Framework\AroundInterceptor; -use Go\Aop\Framework\BeforeInterceptor; -use Go\Aop\Intercept\Interceptor; +use Go\Aop\Framework\Interceptor; +use Go\Aop\Intercept\Interceptor as InterceptorInterface; use Go\Aop\Support\GenericPointcutAdvisor; use Go\Lang\Attribute; use Go\Lang\Attribute\After; @@ -41,8 +38,9 @@ final class AttributeAspectLoaderExtension extends AbstractAspectLoaderExtension */ public const string ADVISOR_ID_SUFFIX = '.advice'; - public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array + public function load(ReflectionClass $reflectionAspect): array { + $aspectClass = $reflectionAspect->getName(); $loadedItems = []; foreach ($reflectionAspect->getMethods() as $aspectMethod) { $methodId = $reflectionAspect->getName() . '->' . $aspectMethod->getName(); @@ -56,12 +54,12 @@ public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array foreach ($pointcutAttributes as $reflectionAttribute) { $attribute = $reflectionAttribute->newInstance(); - $loadedItems[$methodId] = $this->parsePointcut($aspect, $aspectMethod, $attribute->expression); + $loadedItems[$methodId] = $this->parsePointcut($aspectClass, $aspectMethod, $attribute->expression); } foreach ($adviceAttributes as $reflectionAttribute) { $attribute = $reflectionAttribute->newInstance(); - $pointcut = $this->parsePointcut($aspect, $aspectMethod, $attribute->expression); - $interceptor = $this->getAdvice($attribute, $aspect, $aspectMethod); + $pointcut = $this->parsePointcut($aspectClass, $aspectMethod, $attribute->expression); + $interceptor = $this->getAdvice($attribute, $aspectClass, $aspectMethod); $advisorId = $pointcutAttributes === [] ? $methodId : $methodId . self::ADVISOR_ID_SUFFIX; $loadedItems[$advisorId] = new GenericPointcutAdvisor($pointcut, $interceptor); @@ -72,27 +70,33 @@ public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array } /** - * Returns an advice (interceptor) instance by meta-type attribute and closure + * Returns an advice (interceptor) for the advice method, without creating the aspect + * + * The interceptor is the lazy form of the {@see Interceptor} facade: it carries the aspect class, the method name, + * the order and the expression as plain data, and resolves the aspect from the container on first use only. + * + * @param class-string $aspectClass * * @throws AspectException If the advice method is not public or the attribute is unsupported */ protected function getAdvice( AbstractInterceptor $interceptorAttribute, - Aspect $aspect, + string $aspectClass, ReflectionMethod $aspectMethod, - ): Interceptor { + ): InterceptorInterface { if (!$aspectMethod->isPublic()) { throw AspectException::nonPublicAdviceMethod($aspectMethod); } - $adviceCallback = $aspectMethod->getClosure($aspect); - $adviceOrder = $interceptorAttribute->order; - $pointcutExpression = $interceptorAttribute->expression; + $methodName = $aspectMethod->getName(); + $order = $interceptorAttribute->order; + $expression = $interceptorAttribute->expression; + return match (true) { - $interceptorAttribute instanceof Before => new BeforeInterceptor($adviceCallback, $adviceOrder, $pointcutExpression), - $interceptorAttribute instanceof After => new AfterInterceptor($adviceCallback, $adviceOrder, $pointcutExpression), - $interceptorAttribute instanceof Around => new AroundInterceptor($adviceCallback, $adviceOrder, $pointcutExpression), - $interceptorAttribute instanceof AfterThrowing => new AfterThrowingInterceptor($adviceCallback, $adviceOrder, $pointcutExpression), + $interceptorAttribute instanceof Before => Interceptor::before($aspectClass, $methodName, $order, $expression), + $interceptorAttribute instanceof After => Interceptor::after($aspectClass, $methodName, $order, $expression), + $interceptorAttribute instanceof Around => Interceptor::around($aspectClass, $methodName, $order, $expression), + $interceptorAttribute instanceof AfterThrowing => Interceptor::afterThrowing($aspectClass, $methodName, $order, $expression), default => throw AspectException::unsupportedAdviceAttribute($interceptorAttribute), }; } diff --git a/src/Core/Cache/CachedAspectLoader.php b/src/Core/Cache/CachedAspectLoader.php index c7e0c88c4..f2ef9c30b 100644 --- a/src/Core/Cache/CachedAspectLoader.php +++ b/src/Core/Cache/CachedAspectLoader.php @@ -94,18 +94,18 @@ public function __construct( $this->advisorCacheCompiler = new AdvisorCacheCompiler(); } - public function load(Aspect $aspect): array + public function load(string $aspectClass): array { if ($this->cacheDir === null || $this->cacheDir === '') { - return $this->loader->load($aspect); + return $this->loader->load($aspectClass); } - $refAspect = new ReflectionClass($aspect); + $refAspect = new ReflectionClass($aspectClass); $aspectFileName = $refAspect->getFileName(); $cacheFileName = $aspectFileName !== false ? $this->resolveCacheFileName($aspectFileName) : null; if ($aspectFileName === false || $cacheFileName === null) { // Aspects without a resolvable source file below the application root are not cached - return $this->loader->load($aspect); + return $this->loader->load($aspectClass); } // With a prebuilt cache an existing advisor cache file is trusted without any @@ -114,7 +114,7 @@ public function load(Aspect $aspect): array if ($this->isPrebuiltCache) { $loadedItems = file_exists($cacheFileName) ? $this->loadFromCache($cacheFileName) : null; - return $loadedItems ?? $this->loader->load($aspect); + return $loadedItems ?? $this->loader->load($aspectClass); } // A fresh cache file is used when it is compatible, even when it holds no items (an aspect @@ -126,7 +126,7 @@ public function load(Aspect $aspect): array } } - $loadedItems = $this->loader->load($aspect); + $loadedItems = $this->loader->load($aspectClass); $this->saveToCache($refAspect->getName(), $loadedItems, $cacheFileName); return $loadedItems; @@ -135,27 +135,27 @@ public function load(Aspect $aspect): array /** * Loads and register all items of aspect in the container */ - public function loadAndRegister(Aspect $aspect): void + public function loadAndRegister(string $aspectClass): void { - $loadedItems = $this->load($aspect); + $loadedItems = $this->load($aspectClass); foreach ($loadedItems as $itemId => $item) { $this->container->add($itemId, $item); } - $this->loadedAspects[$aspect::class] = $aspect::class; + $this->loadedAspects[$aspectClass] = $aspectClass; } /** - * Returns list of unloaded aspects in the container + * Returns the classes of the aspects registered in the container that are not loaded yet * - * @return list + * @return list> */ public function getUnloadedAspects(): array { $unloadedAspects = []; - foreach ($this->container->getServicesByInterface(Aspect::class) as $aspect) { - if (!isset($this->loadedAspects[$aspect::class])) { - $unloadedAspects[] = $aspect; + foreach ($this->container->getServiceIdsByInterface(Aspect::class) as $aspectClass) { + if (!isset($this->loadedAspects[$aspectClass]) && is_subclass_of($aspectClass, Aspect::class)) { + $unloadedAspects[] = $aspectClass; } } diff --git a/src/Core/Container.php b/src/Core/Container.php index 5c143678a..e6a97331e 100644 --- a/src/Core/Container.php +++ b/src/Core/Container.php @@ -224,6 +224,19 @@ public function getServicesByInterface(string $interfaceTagClassName): array return $values; } + public function getServiceIdsByInterface(string $interfaceTagClassName): array + { + $ids = $this->tags[$interfaceTagClassName] ?? []; + // A snapshot of the ids: the is_subclass_of() autoload can re-enter the container (see above) + foreach (array_keys($this->factories) as $id) { + if (is_subclass_of($id, $interfaceTagClassName) && !in_array($id, $ids, true)) { + $ids[] = $id; + } + } + + return $ids; + } + /** * Materializes a deferred service into a container entry and tags it by its interfaces. * diff --git a/src/Core/IntroductionAspectExtension.php b/src/Core/IntroductionAspectExtension.php index ebe8fdea0..5c3939543 100644 --- a/src/Core/IntroductionAspectExtension.php +++ b/src/Core/IntroductionAspectExtension.php @@ -29,7 +29,7 @@ */ class IntroductionAspectExtension extends AbstractAspectLoaderExtension { - public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array + public function load(ReflectionClass $reflectionAspect): array { $loadedItems = []; foreach ($reflectionAspect->getProperties() as $aspectProperty) { @@ -43,13 +43,13 @@ public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array ? $reflectionAttribute->newInstance() : null; if ($attribute instanceof DeclareParents) { - $pointcut = $this->parsePointcut($aspect, $aspectProperty, $attribute->expression); + $pointcut = $this->parsePointcut($reflectionAspect->getName(), $aspectProperty, $attribute->expression); // Introduction doesn't have own syntax and uses any suitable class-filter $pointcut = new Pointcut\AndPointcut( Pointcut::KIND_INTRODUCTION | Pointcut::KIND_CLASS, $pointcut, ); - $advice = $this->getAdvice($attribute, $aspect, $aspectProperty); + $advice = $this->getAdvice($attribute, $aspectProperty); $advisor = new GenericPointcutAdvisor($pointcut, $advice); $loadedItems[$propertyId] = $advisor; @@ -69,7 +69,6 @@ public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array */ protected function getAdvice( AbstractAttribute $interceptorAttribute, - Aspect $aspect, ReflectionProperty $aspectProperty, ): Advice { return match (true) { diff --git a/src/Instrument/Transformer/WeavingTransformer.php b/src/Instrument/Transformer/WeavingTransformer.php index eb7c1fcf1..f5239ee3a 100644 --- a/src/Instrument/Transformer/WeavingTransformer.php +++ b/src/Instrument/Transformer/WeavingTransformer.php @@ -1103,7 +1103,7 @@ private static function removeCachePath(string $path): void /** * Utility method to load and register unloaded aspects * - * @param Aspect[] $unloadedAspects List of unloaded aspects + * @param list> $unloadedAspects Classes of the unloaded aspects */ private function loadAndRegisterAspects(array $unloadedAspects): void { diff --git a/tests/Aop/Framework/InterceptorTest.php b/tests/Aop/Framework/InterceptorTest.php index 6d722ff9a..352781345 100644 --- a/tests/Aop/Framework/InterceptorTest.php +++ b/tests/Aop/Framework/InterceptorTest.php @@ -44,6 +44,26 @@ public function testCreatesBeforeInterceptor(): void $this->assertSame(10, $interceptor->adviceOrder); } + public function testLazyInterceptorDescribesItsAdviceWithoutCreatingTheAspect(): void + { + $interceptor = Interceptor::around(InterceptorTestAspect::class, 'recordInvocation', 7, 'execution(public Foo->bar(*))'); + $reflection = new ReflectionClass(AroundInterceptor::class); + + $this->assertSame(InterceptorTestAspect::class, $interceptor->aspectClass); + $this->assertSame('recordInvocation', $interceptor->adviceMethodName); + $this->assertSame(7, $interceptor->adviceOrder); + $this->assertSame('execution(public Foo->bar(*))', $interceptor->pointcutExpression); + + // What the weaver and the advisor cache read about the advice does not initialize the interceptor + $descriptor = GeneratedInterceptor::fromAdvice('advisor.id', $interceptor); + $this->assertSame(InterceptorTestAspect::class, $descriptor->aspectClass); + $this->assertSame('recordInvocation', $descriptor->adviceMethod); + $this->assertSame(7, $descriptor->order); + $this->assertFalse($descriptor->usesContainerAdvice); + $interceptor->compileToPhp(); + $this->assertTrue($reflection->isUninitializedLazyObject($interceptor), 'No aspect may be resolved'); + } + public function testAspectClassRequiresAdviceMethodName(): void { try { diff --git a/tests/Console/Command/DebugAdvisorCommandInProcessTest.php b/tests/Console/Command/DebugAdvisorCommandInProcessTest.php index a14efc69d..3bf238b51 100644 --- a/tests/Console/Command/DebugAdvisorCommandInProcessTest.php +++ b/tests/Console/Command/DebugAdvisorCommandInProcessTest.php @@ -75,7 +75,7 @@ public function testExecuteListsRegisteredAdvisors(): void public function testShowAdvisorsListRegistersUnloadedAspects(): void { - $aspect = $this->createStub(\Go\Aop\Aspect::class); + $aspect = $this->createStub(\Go\Aop\Aspect::class)::class; $aspectLoader = $this->createMock(CachedAspectLoader::class); $aspectLoader->method('getUnloadedAspects')->willReturn([$aspect]); diff --git a/tests/Console/Command/DebugAspectCommandInProcessTest.php b/tests/Console/Command/DebugAspectCommandInProcessTest.php index f4a10319e..ddfe59616 100644 --- a/tests/Console/Command/DebugAspectCommandInProcessTest.php +++ b/tests/Console/Command/DebugAspectCommandInProcessTest.php @@ -20,7 +20,6 @@ use Go\Core\AspectLoader; use Go\Tests\TestProject\Aspect\LoggingAspect; use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; use stdClass; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -49,7 +48,7 @@ public function testExecuteListsEnabledAspects(): void { $container = $this->createMock(AspectContainer::class); $container->expects($this->once()) - ->method('getServicesByInterface') + ->method('getServiceIdsByInterface') ->with(Aspect::class) ->willReturn([]); @@ -70,7 +69,7 @@ public function testExecuteWithUnknownAspectOptionFailsWithSuggestion(): void { $container = $this->createStub(AspectContainer::class); $container->method('has')->willReturn(false); - $container->method('getServicesByInterface')->willReturn([LoggingAspect::class => new LoggingAspect($this->createStub(LoggerInterface::class))]); + $container->method('getServiceIdsByInterface')->willReturn([LoggingAspect::class]); $kernel = $this->createStub(AspectKernel::class); $kernel->method('getContainer')->willReturn($container); @@ -89,9 +88,6 @@ public function testExecuteWithUnknownAspectOptionFailsWithSuggestion(): void public function testExecuteFiltersByAspectOptionAndShowsPointcutsAndAdvisors(): void { - $logger = $this->createStub(LoggerInterface::class); - $aspect = new LoggingAspect($logger); - $aspectLoader = $this->createStub(AspectLoader::class); $aspectLoader->method('load')->willReturn([ 'a.pointcut' => $this->createStub(Pointcut::class), @@ -101,9 +97,9 @@ public function testExecuteFiltersByAspectOptionAndShowsPointcutsAndAdvisors(): $container = $this->createStub(AspectContainer::class); $container->method('has')->willReturn(true); + // The aspect is read from its class, never created $container->method('getService')->willReturnMap([ [AspectLoader::class, $aspectLoader], - [LoggingAspect::class, $aspect], ]); $kernel = $this->createStub(AspectKernel::class); diff --git a/tests/Core/AbstractAspectLoaderExtensionTest.php b/tests/Core/AbstractAspectLoaderExtensionTest.php index 89e268be1..4010e62c8 100644 --- a/tests/Core/AbstractAspectLoaderExtensionTest.php +++ b/tests/Core/AbstractAspectLoaderExtensionTest.php @@ -40,7 +40,7 @@ public function testParsesValidPointcutExpressionForMethodReflection(): void $aspect = new AbstractAspectLoaderExtensionTestAspect(); $reflection = new ReflectionMethod($aspect, 'someMethod'); - $pointcut = $this->extension->doParsePointcut($aspect, $reflection, 'execution(public Foo->bar(*))'); + $pointcut = $this->extension->doParsePointcut($aspect::class, $reflection, 'execution(public Foo->bar(*))'); // @phpstan-ignore method.alreadyNarrowedType (runtime double-check of the declared return type) $this->assertInstanceOf(Pointcut::class, $pointcut); @@ -53,7 +53,7 @@ public function testResolvesThisInPointcutExpressionToAspectClassName(): void // "$this" must be replaced with the aspect's own class name before lexing $pointcut = $this->extension->doParsePointcut( - $aspect, + $aspect::class, $reflection, 'execution(public $this->someMethod(*))', ); @@ -70,7 +70,7 @@ public function testThrowsUnexpectedValueExceptionOnLexicalErrorForMethodReflect $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Can not recognize the lexical structure/'); - $this->extension->doParsePointcut($aspect, $reflection, 'execution(public Foo->@#$%^bar(*))'); + $this->extension->doParsePointcut($aspect::class, $reflection, 'execution(public Foo->@#$%^bar(*))'); } public function testThrowsUnexpectedValueExceptionOnLexicalErrorForPropertyReflection(): void @@ -81,7 +81,7 @@ public function testThrowsUnexpectedValueExceptionOnLexicalErrorForPropertyRefle $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Can not recognize the lexical structure/'); - $this->extension->doParsePointcut($aspect, $reflection, 'execution(public Foo->@#$%^bar(*))'); + $this->extension->doParsePointcut($aspect::class, $reflection, 'execution(public Foo->@#$%^bar(*))'); } public function testThrowsUnexpectedValueExceptionOnLexicalErrorForClassReflection(): void @@ -92,7 +92,7 @@ public function testThrowsUnexpectedValueExceptionOnLexicalErrorForClassReflecti $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Can not recognize the lexical structure/'); - $this->extension->doParsePointcut($aspect, $reflection, 'execution(public Foo->@#$%^bar(*))'); + $this->extension->doParsePointcut($aspect::class, $reflection, 'execution(public Foo->@#$%^bar(*))'); } public function testThrowsUnexpectedValueExceptionOnParserErrorForMethodReflection(): void @@ -103,7 +103,7 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForMethodReflecti $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Unexpected token/'); - $this->extension->doParsePointcut($aspect, $reflection, 'public execution(*)'); + $this->extension->doParsePointcut($aspect::class, $reflection, 'public execution(*)'); } public function testThrowsUnexpectedValueExceptionOnParserErrorForPropertyReflection(): void @@ -114,7 +114,7 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForPropertyReflec $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Unexpected token/'); - $this->extension->doParsePointcut($aspect, $reflection, 'public execution(*)'); + $this->extension->doParsePointcut($aspect::class, $reflection, 'public execution(*)'); } public function testThrowsUnexpectedValueExceptionOnParserErrorForClassReflection(): void @@ -125,7 +125,7 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForClassReflectio $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Unexpected token/'); - $this->extension->doParsePointcut($aspect, $reflection, 'public execution(*)'); + $this->extension->doParsePointcut($aspect::class, $reflection, 'public execution(*)'); } } @@ -134,21 +134,22 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForClassReflectio */ final class AbstractAspectLoaderExtensionTestExtension extends AbstractAspectLoaderExtension implements AspectLoaderExtension { - public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array + public function load(ReflectionClass $reflectionAspect): array { return []; } /** * @template T of Aspect + * @param class-string $aspectClass * @param ReflectionMethod|ReflectionProperty|ReflectionClass $reflection */ public function doParsePointcut( - Aspect $aspect, + string $aspectClass, ReflectionMethod|ReflectionProperty|ReflectionClass $reflection, string $pointcutExpression, ): Pointcut { - return $this->parsePointcut($aspect, $reflection, $pointcutExpression); + return $this->parsePointcut($aspectClass, $reflection, $pointcutExpression); } } diff --git a/tests/Core/AdvisorIndexTest.php b/tests/Core/AdvisorIndexTest.php index d59dcf2fa..2b5525090 100644 --- a/tests/Core/AdvisorIndexTest.php +++ b/tests/Core/AdvisorIndexTest.php @@ -15,8 +15,13 @@ use Go\Aop\Advice; use Go\Aop\Aspect; use Go\Aop\Pointcut; +use Go\Aop\Pointcut\PointcutGrammar; +use Go\Aop\Pointcut\PointcutLexer; +use Go\Aop\Pointcut\PointcutParser; use Go\Aop\Pointcut\TruePointcut; use Go\Aop\Support\GenericPointcutAdvisor; +use Go\Lang\Attribute\Before; +use LogicException; use PHPUnit\Framework\TestCase; final class AdvisorIndexTest extends TestCase @@ -63,23 +68,24 @@ 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 { + $loader = new class ($container, $aspect::class, $advisor) implements AspectLoaderInterface { private bool $isLoaded = false; public function __construct( private readonly AspectContainer $container, - private readonly Aspect $aspect, + /** @var class-string */ + private readonly string $aspectClass, private readonly GenericPointcutAdvisor $advisor, ) {} - public function load(Aspect $aspect): array + public function load(string $aspectClass): array { return ['advisor.lazy' => $this->advisor]; } - public function loadAndRegister(Aspect $aspect): void + public function loadAndRegister(string $aspectClass): void { - foreach ($this->load($aspect) as $itemId => $item) { + foreach ($this->load($aspectClass) as $itemId => $item) { $this->container->add($itemId, $item); } $this->isLoaded = true; @@ -87,7 +93,7 @@ public function loadAndRegister(Aspect $aspect): void public function getUnloadedAspects(): array { - return $this->isLoaded ? [] : [$this->aspect]; + return $this->isLoaded ? [] : [$this->aspectClass]; } }; $index = new AdvisorIndex($container, $loader); @@ -96,6 +102,27 @@ public function getUnloadedAspects(): array $this->assertSame(['advisor.lazy' => $advisor], $index->getAdvisors()); } + /** + * Weaving reads aspects by class: even an aspect PHP can not make lazy (no instance properties) is not created, + * its factory runs on the first call of one of its advices only + */ + public function testLoadsDeferredAspectsWithoutRunningTheirFactories(): void + { + $container = new Container(); + $container->addLazyService( + AdvisorIndexTestStatelessAspect::class, + static fn(): never => throw new LogicException('Weaving must not create the aspect'), + ); + $loader = new AspectLoader( + $container, + new AttributeAspectLoaderExtension(new PointcutLexer(), new PointcutParser(new PointcutGrammar())), + ); + $index = new AdvisorIndex($container, $loader); + + $this->assertTrue($index->hasPointcutKind(Pointcut::KIND_METHOD)); + $this->assertArrayHasKey(AdvisorIndexTestStatelessAspect::class . '->beforeMethod', $index->getAdvisors()); + } + private function createAdvisor(int $pointcutKind): GenericPointcutAdvisor { return new GenericPointcutAdvisor(new TruePointcut($pointcutKind), $this->createStub(Advice::class)); @@ -109,3 +136,9 @@ private function createLoader(): AspectLoaderInterface return $loader; } } + +final class AdvisorIndexTestStatelessAspect implements Aspect +{ + #[Before('execution(public NonExistent\\**->*(*))')] + public function beforeMethod(): void {} +} diff --git a/tests/Core/AspectLoaderTest.php b/tests/Core/AspectLoaderTest.php index cbf26ce79..b19f17378 100644 --- a/tests/Core/AspectLoaderTest.php +++ b/tests/Core/AspectLoaderTest.php @@ -15,6 +15,7 @@ use Go\Aop\Advisor; use Go\Aop\Aspect; use Go\Aop\Pointcut; +use LogicException; use PHPUnit\Framework\TestCase; #[\PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations] @@ -25,7 +26,7 @@ public function testLoadReturnsEmptyArrayWithoutRegisteredLoaders(): void $container = new Container(); $loader = new AspectLoader($container); - $items = $loader->load(new AspectLoaderTestAspectOne()); + $items = $loader->load(AspectLoaderTestAspectOne::class); $this->assertSame([], $items); } @@ -45,8 +46,7 @@ public function testLoadCombinesItemsFromAllLoaderExtensions(): void $loader = new AspectLoader($container, $extensionOne, $extensionTwo); - $aspect = new AspectLoaderTestAspectOne(); - $items = $loader->load($aspect); + $items = $loader->load(AspectLoaderTestAspectOne::class); $this->assertSame( ['item.one' => $pointcutOne, 'item.two' => $pointcutTwo], @@ -69,7 +69,7 @@ public function testLoadKeepsFirstLoaderItemWhenKeysCollide(): void $loader = new AspectLoader($container, $extensionOne, $extensionTwo); - $items = $loader->load(new AspectLoaderTestAspectOne()); + $items = $loader->load(AspectLoaderTestAspectOne::class); // array union (+=) semantics: the first loader's item for a colliding key wins $this->assertSame($pointcutFromFirst, $items['shared.key']); @@ -84,7 +84,7 @@ public function testLoadAndRegisterAddsLoadedItemsToContainer(): void $extension->method('load')->willReturn(['aspect.advisor' => $advisor]); $loader = new AspectLoader($container, $extension); - $loader->loadAndRegister(new AspectLoaderTestAspectOne()); + $loader->loadAndRegister(AspectLoaderTestAspectOne::class); $this->assertSame($advisor, $container->get('aspect.advisor')); } @@ -92,23 +92,37 @@ public function testLoadAndRegisterAddsLoadedItemsToContainer(): void public function testGetUnloadedAspectsReturnsOnlyAspectsNotYetLoaded(): void { $container = new Container(); - $aspectOne = new AspectLoaderTestAspectOne(); - $aspectTwo = new AspectLoaderTestAspectTwo(); - - $container->add(AspectLoaderTestAspectOne::class, $aspectOne); - $container->add(AspectLoaderTestAspectTwo::class, $aspectTwo); + $container->add(AspectLoaderTestAspectOne::class, new AspectLoaderTestAspectOne()); + $container->add(AspectLoaderTestAspectTwo::class, new AspectLoaderTestAspectTwo()); $loader = new AspectLoader($container); // Before anything is loaded, both aspects are unloaded $this->assertSame( - [$aspectOne, $aspectTwo], + [AspectLoaderTestAspectOne::class, AspectLoaderTestAspectTwo::class], $loader->getUnloadedAspects(), ); - $loader->loadAndRegister($aspectOne); + $loader->loadAndRegister(AspectLoaderTestAspectOne::class); + + $this->assertSame([AspectLoaderTestAspectTwo::class], $loader->getUnloadedAspects()); + } + + public function testDeferredAspectsAreLoadedWithoutCreatingThem(): void + { + $container = new Container(); + $container->addLazyService( + AspectLoaderTestAspectOne::class, + static fn(): never => throw new LogicException('The aspect must not be created to load it'), + ); + $extension = $this->createMock(AspectLoaderExtension::class); + $extension->method('load')->willReturn([]); + $loader = new AspectLoader($container, $extension); + + $this->assertSame([AspectLoaderTestAspectOne::class], $loader->getUnloadedAspects()); + $loader->loadAndRegister(AspectLoaderTestAspectOne::class); - $this->assertSame([$aspectTwo], $loader->getUnloadedAspects()); + $this->assertSame([], $loader->getUnloadedAspects()); } } diff --git a/tests/Core/AttributeAspectLoaderExtensionTest.php b/tests/Core/AttributeAspectLoaderExtensionTest.php index eeadfa470..fbb215575 100644 --- a/tests/Core/AttributeAspectLoaderExtensionTest.php +++ b/tests/Core/AttributeAspectLoaderExtensionTest.php @@ -8,6 +8,7 @@ use Go\Aop\Aspect; use Go\Aop\Exception\AspectException; use Go\Aop\Exception\PointcutSyntaxException; +use Go\Aop\Framework\AbstractInterceptor as AbstractInterceptorClass; use Go\Aop\Pointcut; use Go\Aop\Pointcut\PointcutGrammar; use Go\Aop\Pointcut\PointcutLexer; @@ -18,6 +19,7 @@ use Go\Lang\Attribute\Pointcut as PointcutAttribute; use Go\Stubs\AttributeAspectLoaderExtensionTestPrivateAspect; use Go\Stubs\AttributeAspectLoaderExtensionTestPublicAspect; +use LogicException; use PHPUnit\Framework\TestCase; use ReflectionClass; @@ -33,11 +35,25 @@ protected function setUp(): void public function testLoadsAdvisorForPublicAdviceMethod(): void { $aspect = new AttributeAspectLoaderExtensionTestPublicAspect(); - $loadedItems = $this->extension->load($aspect, new ReflectionClass($aspect)); + $loadedItems = $this->extension->load(new ReflectionClass($aspect)); $this->assertArrayHasKey($aspect::class . '->publicAdvice', $loadedItems); } + public function testLoadsAdvicesWithoutCreatingTheAspect(): void + { + $loadedItems = $this->extension->load(new ReflectionClass(AttributeAspectLoaderExtensionTestUncreatableAspect::class)); + + $advisor = $loadedItems[AttributeAspectLoaderExtensionTestUncreatableAspect::class . '->advice'] ?? null; + $this->assertInstanceOf(GenericPointcutAdvisor::class, $advisor); + $interceptor = $advisor->getAdvice(); + $this->assertInstanceOf(AbstractInterceptorClass::class, $interceptor); + $this->assertSame(AttributeAspectLoaderExtensionTestUncreatableAspect::class, $interceptor->aspectClass); + $this->assertSame('advice', $interceptor->adviceMethodName); + $this->assertSame(3, $interceptor->adviceOrder); + $this->assertTrue(new ReflectionClass($interceptor::class)->isUninitializedLazyObject($interceptor)); + } + public function testRejectsNonPublicAdviceMethod(): void { $aspect = new AttributeAspectLoaderExtensionTestPrivateAspect(); @@ -45,13 +61,13 @@ public function testRejectsNonPublicAdviceMethod(): void $this->expectException(AspectException::class); $this->expectExceptionMessage('first-class advice callables require all advice methods to be public'); - $this->extension->load($aspect, new ReflectionClass($aspect)); + $this->extension->load(new ReflectionClass($aspect)); } public function testForeignAttributeOnAdviceMethodIsIgnored(): void { $aspect = new AttributeAspectLoaderExtensionTestForeignAttributeAspect(); - $loadedItems = $this->extension->load($aspect, new ReflectionClass($aspect)); + $loadedItems = $this->extension->load(new ReflectionClass($aspect)); $this->assertSame([$aspect::class . '->advice'], array_keys($loadedItems)); } @@ -59,7 +75,7 @@ public function testForeignAttributeOnAdviceMethodIsIgnored(): void public function testPointcutAndAdviceOnOneMethodAreBothLoaded(): void { $aspect = new AttributeAspectLoaderExtensionTestPointcutAndAdviceAspect(); - $loadedItems = $this->extension->load($aspect, new ReflectionClass($aspect)); + $loadedItems = $this->extension->load(new ReflectionClass($aspect)); $methodId = $aspect::class . '->pointcutAndAdvice'; $this->assertInstanceOf(Pointcut::class, $loadedItems[$methodId] ?? null); @@ -78,7 +94,7 @@ public function testSecondAdviceAttributeOnOneMethodFailsLoudly(): void 'Advice method ' . AttributeAspectLoaderExtensionTestTwoAdvicesAspect::class . '::twoAdvices() declares 2 advice attributes', ); - $this->extension->load($aspect, new ReflectionClass($aspect)); + $this->extension->load(new ReflectionClass($aspect)); } public function testPointcutSyntaxErrorCitesTheMethod(): void @@ -88,13 +104,27 @@ public function testPointcutSyntaxErrorCitesTheMethod(): void $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessage(AttributeAspectLoaderExtensionTestBrokenPointcutAspect::class . '->brokenAdvice'); - $this->extension->load($aspect, new ReflectionClass($aspect)); + $this->extension->load(new ReflectionClass($aspect)); } } #[Attribute(Attribute::TARGET_METHOD)] final class AttributeAspectLoaderExtensionTestForeignAttribute {} +/** + * Aspect without instance properties (PHP can not make it lazy) that must never be created by loading it + */ +final class AttributeAspectLoaderExtensionTestUncreatableAspect implements Aspect +{ + public function __construct() + { + throw new LogicException('Loading an aspect must not create it'); + } + + #[Before('execution(public NonExistent\**->*(*))', order: 3)] + public function advice(): void {} +} + final class AttributeAspectLoaderExtensionTestForeignAttributeAspect implements Aspect { #[AttributeAspectLoaderExtensionTestForeignAttribute] diff --git a/tests/Core/Cache/CachedAspectLoaderTest.php b/tests/Core/Cache/CachedAspectLoaderTest.php index 1f34d2a6d..fd0654bc6 100644 --- a/tests/Core/Cache/CachedAspectLoaderTest.php +++ b/tests/Core/Cache/CachedAspectLoaderTest.php @@ -38,7 +38,8 @@ class CachedAspectLoaderTest extends TestCase private string $cacheDir; - private Aspect $aspect; + /** @var class-string */ + private string $aspectClass; /** * Path of the fake aspect source file below the application root @@ -71,9 +72,8 @@ protected function setUp(): void ); require $this->aspectFileName; $className = 'Go\\Core\\TestFixture\\' . $shortClassName; - $aspect = new $className(); - assert($aspect instanceof Aspect); - $this->aspect = $aspect; + assert(is_subclass_of($className, Aspect::class)); + $this->aspectClass = $className; // The naming scheme: app-root prefix replaced by the cache dir, '.php' by '.cache.php' $this->cacheFileName = $this->cacheDir . '/src/Aspect/' . $shortClassName . '.cache.php'; @@ -132,7 +132,7 @@ public function testPrebuiltCacheTrustsExistingCacheFileWithoutFreshnessChecks() $this->writeCacheFile($this->validCacheFileContent(), stale: true); $this->innerLoader->expects($this->never())->method('load'); - $loadedItems = $loader->load($this->aspect); + $loadedItems = $loader->load($this->aspectClass); $this->assertArrayHasKey('pc', $loadedItems); $this->assertInstanceOf(TruePointcut::class, $loadedItems['pc']); @@ -148,7 +148,7 @@ public function testPrebuiltCacheCorruptFileThrowsWithoutRewriting(): void // A corrupt cache file is external interference and fails loudly - no fallback $thrownError = null; try { - $loader->load($this->aspect); + $loader->load($this->aspectClass); } catch (\ParseError $parseError) { $thrownError = $parseError; } @@ -166,7 +166,7 @@ public function testPrebuiltCacheFallsBackToLoaderOnWrongVersionWithoutRewriting $this->innerLoader->expects($this->once())->method('load')->willReturn([]); // A clean version mismatch is the expected upgrade path: silent direct-loader fallback - $this->assertSame([], $loader->load($this->aspect)); + $this->assertSame([], $loader->load($this->aspectClass)); $this->assertSame($wrongVersionContent, file_get_contents($this->cacheFileName)); } @@ -177,7 +177,7 @@ public function testStaleCacheFileIsRebuiltAndRewritten(): void $freshItems = ['pointcut.fresh' => new TruePointcut()]; $this->innerLoader->expects($this->once())->method('load')->willReturn($freshItems); - $this->assertSame($freshItems, $loader->load($this->aspect)); + $this->assertSame($freshItems, $loader->load($this->aspectClass)); // The stale file must have been replaced by a freshly compiled one that includes cleanly $rewrittenData = include $this->cacheFileName; @@ -198,7 +198,7 @@ public function testFreshButCorruptCacheFileThrowsWithoutRebuilding(): void $thrownError = null; try { - $loader->load($this->aspect); + $loader->load($this->aspectClass); } catch (\ParseError $parseError) { $thrownError = $parseError; } @@ -213,7 +213,7 @@ public function testFreshValidCacheFileIsUsedWithoutTouchingTheLoader(): void $this->writeCacheFile($this->validCacheFileContent(), stale: false); $this->innerLoader->expects($this->never())->method('load'); - $loadedItems = $loader->load($this->aspect); + $loadedItems = $loader->load($this->aspectClass); $this->assertArrayHasKey('pc', $loadedItems); $this->assertInstanceOf(TruePointcut::class, $loadedItems['pc']); @@ -232,7 +232,7 @@ public function testPayloadWithAnInvalidEntryIsRejectedAsAWhole(): void $this->innerLoader->expects($this->once())->method('load')->willReturn($freshItems); // Using the valid rest would silently lose advices: the file is rebuilt instead - $this->assertSame($freshItems, $loader->load($this->aspect)); + $this->assertSame($freshItems, $loader->load($this->aspectClass)); $this->assertStringContainsString('pointcut.fresh', (string) file_get_contents($this->cacheFileName)); } @@ -241,12 +241,12 @@ public function testAspectWithoutItemsIsWrittenOnceAndThenOnlyRead(): void $loader = $this->createLoader(0); $this->innerLoader->expects($this->once())->method('load')->willReturn([]); - $this->assertSame([], $loader->load($this->aspect)); + $this->assertSame([], $loader->load($this->aspectClass)); $this->assertFileExists($this->cacheFileName); $cacheModificationTime = filemtime($this->cacheFileName); // An empty compatible cache is a hit: the loader is not called again and nothing is rewritten - $this->assertSame([], $loader->load($this->aspect)); + $this->assertSame([], $loader->load($this->aspectClass)); clearstatcache(); $this->assertSame($cacheModificationTime, filemtime($this->cacheFileName)); } @@ -259,7 +259,7 @@ public function testPrebuiltCacheWithMissingFileLoadsDirectlyWithoutWriting(): v $freshItems = ['pointcut.fresh' => new TruePointcut()]; $this->innerLoader->expects($this->once())->method('load')->willReturn($freshItems); - $this->assertSame($freshItems, $loader->load($this->aspect)); + $this->assertSame($freshItems, $loader->load($this->aspectClass)); $this->assertFileDoesNotExist($this->cacheFileName); } @@ -284,7 +284,7 @@ public function compileToPhp(): \PhpParser\Node\Expr // failure propagates instead of degrading to silent uncached operation $thrownException = null; try { - $loader->load($this->aspect); + $loader->load($this->aspectClass); } catch (NotCompilableException $notCompilableException) { $thrownException = $notCompilableException; } @@ -301,7 +301,7 @@ public function testWithoutCacheDirectoryEveryLoadDelegatesToTheLoader(): void $loadedItems = ['pc' => new TruePointcut()]; $this->innerLoader->expects($this->once())->method('load')->willReturn($loadedItems); - $this->assertSame($loadedItems, $loader->load($this->aspect)); + $this->assertSame($loadedItems, $loader->load($this->aspectClass)); $this->assertFileDoesNotExist($this->cacheFileName); } @@ -313,7 +313,7 @@ public function testAspectOutsideApplicationRootIsLoadedDirectlyWithoutCaching() $loadedItems = ['pc' => new TruePointcut()]; $this->innerLoader->expects($this->once())->method('load')->willReturn($loadedItems); - $this->assertSame($loadedItems, $loader->load($foreignAspect)); + $this->assertSame($loadedItems, $loader->load($foreignAspect::class)); // No shadow file may appear anywhere below the cache directory $cacheEntries = scandir($this->cacheDir); diff --git a/tests/Core/ContainerTest.php b/tests/Core/ContainerTest.php index f0806f97b..a2bf7cdda 100644 --- a/tests/Core/ContainerTest.php +++ b/tests/Core/ContainerTest.php @@ -432,6 +432,28 @@ function () use (&$constructed): StatefulTestAspect { $this->assertFalse((new \ReflectionClass(StatefulTestAspect::class))->isUninitializedLazyObject($aspect)); } + public function testServiceIdsByInterfaceNeverCreateServices(): void + { + $constructed = false; + $this->container->add(StatefulTestAspect::class, new StatefulTestAspect(1)); + // A property-less class: materializing it would run the factory right away + $this->container->addLazyService( + DoSomethingAspect::class, + function () use (&$constructed): DoSomethingAspect { + $constructed = true; + + return new DoSomethingAspect(); + }, + ); + + $this->assertSame( + [StatefulTestAspect::class, DoSomethingAspect::class], + $this->container->getServiceIdsByInterface(Aspect::class), + ); + $this->assertFalse($constructed, 'Listing the ids must not run any factory'); + $this->assertSame([], $this->container->getServiceIdsByInterface(\Countable::class)); + } + public function testPropertylessServiceFallsBackToEagerConstruction(): void { // PHP creates lazy objects of property-less classes as already initialized, diff --git a/tests/Core/IntroductionAspectExtensionTest.php b/tests/Core/IntroductionAspectExtensionTest.php index 88938e063..5a7538dde 100644 --- a/tests/Core/IntroductionAspectExtensionTest.php +++ b/tests/Core/IntroductionAspectExtensionTest.php @@ -44,7 +44,7 @@ protected function setUp(): void public function testLoadsIntroductionAdvisorForDeclareParentsProperty(): void { $aspect = new IntroductionAspectExtensionTestAspect(); - $loadedItems = $this->extension->load($aspect, new ReflectionClass($aspect)); + $loadedItems = $this->extension->load(new ReflectionClass($aspect)); $propertyId = IntroductionAspectExtensionTestAspect::class . '->introduction'; $this->assertArrayHasKey($propertyId, $loadedItems); @@ -68,7 +68,7 @@ public function testForeignAttributeOnPropertyIsIgnored(): void { $aspect = new IntroductionAspectExtensionTestForeignAttributeAspect(); - $this->assertSame([], $this->extension->load($aspect, new ReflectionClass($aspect))); + $this->assertSame([], $this->extension->load(new ReflectionClass($aspect))); } public function testThrowsForAdviceAttributeOnProperty(): void @@ -81,13 +81,13 @@ public function testThrowsForAdviceAttributeOnProperty(): void . IntroductionAspectExtensionTestInvalidAspect::class . '::$invalidIntroduction', ); - $this->extension->load($aspect, new ReflectionClass($aspect)); + $this->extension->load(new ReflectionClass($aspect)); } public function testReturnsEmptyArrayForAspectWithoutAnnotatedProperties(): void { $aspect = new IntroductionAspectExtensionTestEmptyAspect(); - $loadedItems = $this->extension->load($aspect, new ReflectionClass($aspect)); + $loadedItems = $this->extension->load(new ReflectionClass($aspect)); $this->assertSame([], $loadedItems); } @@ -97,10 +97,9 @@ public function testGetAdviceThrowsForUnsupportedAttribute(): void $extension = new class (new PointcutLexer(), new PointcutParser(new PointcutGrammar())) extends IntroductionAspectExtension { public function doGetAdvice( AbstractAttribute $interceptorAttribute, - Aspect $aspect, ReflectionProperty $aspectProperty, ): Advice { - return $this->getAdvice($interceptorAttribute, $aspect, $aspectProperty); + return $this->getAdvice($interceptorAttribute, $aspectProperty); } }; @@ -112,7 +111,7 @@ public function doGetAdvice( $this->expectException(AspectException::class); $this->expectExceptionMessage('Unsupported attribute class: ' . $unsupportedAttribute::class); - $extension->doGetAdvice($unsupportedAttribute, $aspect, $reflection); + $extension->doGetAdvice($unsupportedAttribute, $reflection); } } diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index 0d9fd39f1..457af804a 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -244,7 +244,7 @@ public function testUnadvisedClassWithUnknownParentIsNotTransformed(): void */ public function testLoadsUnloadedAspectsBeforeMatching(): void { - $aspect = $this->createStub(Aspect::class); + $aspect = $this->createStub(Aspect::class)::class; $loader = $this->createMock(AspectLoader::class); $loader->method('getUnloadedAspects')->willReturn([$aspect]); $loader->expects($this->once())->method('loadAndRegister')->with($aspect); From 0f33c8fa02fc9f1a04922dc9ed541608d768f766 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 14:02:36 +0000 Subject: [PATCH 2/7] test(core): cover the unsupported advice attribute of the attribute loader Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe --- .../AttributeAspectLoaderExtensionTest.php | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/Core/AttributeAspectLoaderExtensionTest.php b/tests/Core/AttributeAspectLoaderExtensionTest.php index fbb215575..f82f0c0e6 100644 --- a/tests/Core/AttributeAspectLoaderExtensionTest.php +++ b/tests/Core/AttributeAspectLoaderExtensionTest.php @@ -14,6 +14,7 @@ use Go\Aop\Pointcut\PointcutLexer; use Go\Aop\Pointcut\PointcutParser; use Go\Aop\Support\GenericPointcutAdvisor; +use Go\Lang\Attribute\AbstractInterceptor as AbstractInterceptorAttribute; use Go\Lang\Attribute\After; use Go\Lang\Attribute\Before; use Go\Lang\Attribute\Pointcut as PointcutAttribute; @@ -54,6 +55,19 @@ public function testLoadsAdvicesWithoutCreatingTheAspect(): void $this->assertTrue(new ReflectionClass($interceptor::class)->isUninitializedLazyObject($interceptor)); } + public function testUnsupportedAdviceAttributeIsReported(): void + { + try { + $this->extension->load(new ReflectionClass(AttributeAspectLoaderExtensionTestUnknownAdviceAspect::class)); + $this->fail('An advice attribute no interceptor handles must be rejected'); + } catch (AspectException $exception) { + $this->assertSame( + 'Unsupported method meta class: ' . AttributeAspectLoaderExtensionTestUnknownAdvice::class, + $exception->getMessage(), + ); + } + } + public function testRejectsNonPublicAdviceMethod(): void { $aspect = new AttributeAspectLoaderExtensionTestPrivateAspect(); @@ -111,6 +125,18 @@ public function testPointcutSyntaxErrorCitesTheMethod(): void #[Attribute(Attribute::TARGET_METHOD)] final class AttributeAspectLoaderExtensionTestForeignAttribute {} +/** + * An advice attribute of the framework hierarchy that no interceptor type handles + */ +#[Attribute(Attribute::TARGET_METHOD)] +final class AttributeAspectLoaderExtensionTestUnknownAdvice extends AbstractInterceptorAttribute {} + +final class AttributeAspectLoaderExtensionTestUnknownAdviceAspect implements Aspect +{ + #[AttributeAspectLoaderExtensionTestUnknownAdvice('execution(public NonExistent\\**->*(*))')] + public function advice(): void {} +} + /** * Aspect without instance properties (PHP can not make it lazy) that must never be created by loading it */ From be04ee350f6bdecf2bc17eca8b054b2dd798ff3e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 21:38:12 +0000 Subject: [PATCH 3/7] refactor(aop): pass the aspect method of an interceptor as [class, method] The interceptor constructor takes the advice closure or the pair of aspect class and advice method, and keeps the pair in one aspectMethod property. NativeLazyProxy::create() sets the properties the lazy facade exposes, so the facade no longer handles reflection properties. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe --- phpstan-baseline-strict.php | 2 +- src/Aop/Framework/AbstractInterceptor.php | 50 ++++++++++++------- src/Aop/Framework/GeneratedInterceptor.php | 28 +++++------ src/Aop/Framework/Interceptor.php | 36 +++---------- src/Core/NativeLazyProxy.php | 32 ++++++++++-- tests/Aop/Framework/InterceptorTest.php | 3 +- .../AttributeAspectLoaderExtensionTest.php | 3 +- tests/Core/NativeLazyProxyTest.php | 28 +++++++++++ 8 files changed, 109 insertions(+), 73 deletions(-) diff --git a/phpstan-baseline-strict.php b/phpstan-baseline-strict.php index 1a815e116..ad580c3af 100644 --- a/phpstan-baseline-strict.php +++ b/phpstan-baseline-strict.php @@ -11,7 +11,7 @@ 'message' => '#^Variable method call on Go\\\\Aop\\\\Aspect\\.$#', 'identifier' => 'method.dynamicName', 'count' => 1, - 'path' => __DIR__ . '/src/Aop/Framework/Interceptor.php', + 'path' => __DIR__ . '/src/Aop/Framework/AbstractInterceptor.php', ]; $ignoreErrors[] = [ 'message' => '#^Only booleans are allowed in &&, list\\\\|\\(T of object \\= object\\)\\|null\\>\\|null given on the right side\\.$#', diff --git a/src/Aop/Framework/AbstractInterceptor.php b/src/Aop/Framework/AbstractInterceptor.php index 4b85e98ff..ccac61c11 100644 --- a/src/Aop/Framework/AbstractInterceptor.php +++ b/src/Aop/Framework/AbstractInterceptor.php @@ -55,11 +55,30 @@ */ abstract class AbstractInterceptor implements InterceptorInterface, OrderedAdvice { + /** + * Advice closure, called with the joinpoint + */ + protected readonly Closure $adviceMethod; + + /** + * Aspect class and advice method of an advice given by name, null for a closure advice + * + * @var array{class-string, string}|null + * + * @internal Read by the weaver and the advisor cache: the lazy {@see Interceptor} facade sets it without + * initializing the interceptor, so the aspect is not created + */ + public private(set) ?array $aspectMethod = null; + /** * Default constructor for interceptor + * + * @param Closure|array{class-string, string} $adviceMethod Advice closure, or the aspect class and the + * name of its advice method: the aspect is then resolved from the container ({@see The::aspect()}). + * The array is not a callable for PHP (the advice method is not static), so the type can not be callable. */ public function __construct( - protected readonly Closure $adviceMethod, + Closure|array $adviceMethod, /** Order of advice invocation, lower values are invoked first */ public readonly int $adviceOrder = 0, /** @@ -68,19 +87,15 @@ public function __construct( * @internal Framework-internal introspection point (e.g. debug:advisor command), not a public API */ public private(set) string $pointcutExpression = '', - /** - * @var class-string|null Aspect class of an advice method, null for a plain closure advice - * - * @internal Set by the lazy {@see Interceptor} facade: the weaver reads it without creating the aspect - */ - public private(set) ?string $aspectClass = null, - /** - * Name of the advice method of {@see $aspectClass} - * - * @internal Set together with {@see $aspectClass} - */ - public private(set) ?string $adviceMethodName = null, - ) {} + ) { + if ($adviceMethod instanceof Closure) { + $this->adviceMethod = $adviceMethod; + } else { + [$aspectClass, $methodName] = $adviceMethod; + $this->adviceMethod = The::aspect($aspectClass)->$methodName(...); + $this->aspectMethod = $adviceMethod; + } + } /** * Getter for extracting the advice closure from Interceptor @@ -102,10 +117,9 @@ public function getRawAdvice(): Closure */ final public function compileToPhp(): Expr { - // A lazy interceptor knows its aspect method: reading the advice closure would create the aspect - if ($this->aspectClass !== null && $this->adviceMethodName !== null) { - $aspectClass = $this->aspectClass; - $methodName = $this->adviceMethodName; + // An advice given by name is compiled without reading the closure: that would create the aspect + if ($this->aspectMethod !== null) { + [$aspectClass, $methodName] = $this->aspectMethod; } else { $reflectionAdvice = new ReflectionFunction($this->adviceMethod); $scopeReflectionClass = $reflectionAdvice->getClosureScopeClass(); diff --git a/src/Aop/Framework/GeneratedInterceptor.php b/src/Aop/Framework/GeneratedInterceptor.php index 0f18b823c..270be79b9 100644 --- a/src/Aop/Framework/GeneratedInterceptor.php +++ b/src/Aop/Framework/GeneratedInterceptor.php @@ -39,26 +39,22 @@ public static function fromAdvice(string $advisorId, Advice $advice): self throw AspectException::unsupportedAdvice($advisorId, $advice); } - // A lazy interceptor knows its aspect method: reading the advice closure would create the aspect - if ($advice->aspectClass !== null && $advice->adviceMethodName !== null) { - return new self( - $advice->getType()->value, - $advice->aspectClass, - $advice->adviceMethodName, - $advice->adviceOrder, - $advisorId, - false, - ); + if ($advice->aspectMethod !== null) { + // An advice given by name: reading its closure would create the aspect + [$aspectClass, $adviceMethod] = $advice->aspectMethod; + $usesContainerAdvice = false; + } else { + $reflectionAdvice = new ReflectionFunction($advice->getRawAdvice()); + $scopeClass = $reflectionAdvice->getClosureScopeClass(); + $usesContainerAdvice = $scopeClass === null || !is_subclass_of($scopeClass->name, Aspect::class); + $aspectClass = $usesContainerAdvice ? null : $scopeClass->name; + $adviceMethod = $usesContainerAdvice ? null : $reflectionAdvice->name; } - $reflectionAdvice = new ReflectionFunction($advice->getRawAdvice()); - $scopeClass = $reflectionAdvice->getClosureScopeClass(); - $usesContainerAdvice = $scopeClass === null || !is_subclass_of($scopeClass->name, Aspect::class); - return new self( $advice->getType()->value, - $usesContainerAdvice ? null : $scopeClass->name, - $usesContainerAdvice ? null : $reflectionAdvice->name, + $aspectClass, + $adviceMethod, $advice->adviceOrder, $advisorId, $usesContainerAdvice, diff --git a/src/Aop/Framework/Interceptor.php b/src/Aop/Framework/Interceptor.php index 0fe9ceddd..aa2778fc7 100644 --- a/src/Aop/Framework/Interceptor.php +++ b/src/Aop/Framework/Interceptor.php @@ -16,7 +16,6 @@ use Go\Aop\Aspect; use Go\Aop\Exception\AspectException; use Go\Core\NativeLazyProxy; -use ReflectionProperty; /** * Factory facade for generated interceptor declarations, with two construction modes. @@ -38,13 +37,6 @@ */ final class Interceptor { - /** - * Properties describing the advice, set on the lazy proxies without initializing them - * - * @var array{ReflectionProperty, ReflectionProperty, ReflectionProperty, ReflectionProperty}|null - */ - private static ?array $descriptionProperties = null; - /** * @param class-string|Closure $aspectClassOrAdvice */ @@ -101,31 +93,15 @@ private static function createLazily( throw AspectException::adviceMethodNameRequired(); } + $aspectMethod = [$aspectClassOrAdvice, $methodName]; + // Interceptor classes are framework-owned and lazy-compatible by construction, // so they take the probe-free trusted path. - $interceptor = NativeLazyProxy::create( + return NativeLazyProxy::create( $interceptorClass, - static fn(): AbstractInterceptor => new $interceptorClass( - The::aspect($aspectClassOrAdvice)->$methodName(...), - $order, - $expression, - $aspectClassOrAdvice, - $methodName, - ), + static fn(): AbstractInterceptor => new $interceptorClass($aspectMethod, $order, $expression), + // What weaving, ordering and cache compilation read: the aspect is only created when the advice runs + ['adviceOrder' => $order, 'pointcutExpression' => $expression, 'aspectMethod' => $aspectMethod], ); - // The descriptive data is readable without initializing the proxy: weaving, ordering and cache compilation - // read it, the aspect is only created when the advice runs - [$orderProperty, $expressionProperty, $aspectProperty, $methodProperty] = self::$descriptionProperties ??= [ - new ReflectionProperty(AbstractInterceptor::class, 'adviceOrder'), - new ReflectionProperty(AbstractInterceptor::class, 'pointcutExpression'), - new ReflectionProperty(AbstractInterceptor::class, 'aspectClass'), - new ReflectionProperty(AbstractInterceptor::class, 'adviceMethodName'), - ]; - $orderProperty->setRawValueWithoutLazyInitialization($interceptor, $order); - $expressionProperty->setRawValueWithoutLazyInitialization($interceptor, $expression); - $aspectProperty->setRawValueWithoutLazyInitialization($interceptor, $aspectClassOrAdvice); - $methodProperty->setRawValueWithoutLazyInitialization($interceptor, $methodName); - - return $interceptor; } } diff --git a/src/Core/NativeLazyProxy.php b/src/Core/NativeLazyProxy.php index 226ece8fe..f1a6797c3 100644 --- a/src/Core/NativeLazyProxy.php +++ b/src/Core/NativeLazyProxy.php @@ -16,6 +16,7 @@ use Error; use ReflectionClass; use ReflectionException; +use ReflectionProperty; /** * Creates native lazy proxies ({@see ReflectionClass::newLazyProxy()}) through a single @@ -46,6 +47,11 @@ private function __construct() {} */ private static array $unsupported = []; + /** + * @var array> Properties set by {@see create()}, by class and name + */ + private static array $rawProperties = []; + /** * Creates a lazy proxy of the class, or returns null when PHP cannot make the class * lazy - the caller is expected to fall back to eager construction @@ -86,15 +92,33 @@ public static function tryCreate(string $className, Closure $initializer): ?obje * Creates a lazy proxy of a class known to be proxy-compatible by construction * (framework-owned classes), skipping the probe and the memo * + * The properties given in $rawValues are set on the proxy without initializing it, so they can be read + * without running the initializer. The initializer must give the real instance the same values: once the + * proxy is initialized, reads go to the real instance. At least one property has to stay unset, or PHP + * marks the proxy as initialized and the initializer never runs. + * * @template T of object * - * @param class-string $className - * @param Closure(T): T $initializer + * @param class-string $className + * @param Closure(T): T $initializer + * @param array $rawValues Values of properties, by property name * * @return T */ - public static function create(string $className, Closure $initializer): object + public static function create(string $className, Closure $initializer, array $rawValues = []): object { - return new ReflectionClass($className)->newLazyProxy($initializer); + $reflection = new ReflectionClass($className); + $proxy = $reflection->newLazyProxy($initializer); + foreach ($rawValues as $propertyName => $value) { + // Taken from the declaring class: PHP refuses to set an inherited private(set) property in the scope + // of a child class + $property = self::$rawProperties[$className][$propertyName] ??= new ReflectionProperty( + $reflection->getProperty($propertyName)->class, + $propertyName, + ); + $property->setRawValueWithoutLazyInitialization($proxy, $value); + } + + return $proxy; } } diff --git a/tests/Aop/Framework/InterceptorTest.php b/tests/Aop/Framework/InterceptorTest.php index 352781345..9dad9dbef 100644 --- a/tests/Aop/Framework/InterceptorTest.php +++ b/tests/Aop/Framework/InterceptorTest.php @@ -49,8 +49,7 @@ public function testLazyInterceptorDescribesItsAdviceWithoutCreatingTheAspect(): $interceptor = Interceptor::around(InterceptorTestAspect::class, 'recordInvocation', 7, 'execution(public Foo->bar(*))'); $reflection = new ReflectionClass(AroundInterceptor::class); - $this->assertSame(InterceptorTestAspect::class, $interceptor->aspectClass); - $this->assertSame('recordInvocation', $interceptor->adviceMethodName); + $this->assertSame([InterceptorTestAspect::class, 'recordInvocation'], $interceptor->aspectMethod); $this->assertSame(7, $interceptor->adviceOrder); $this->assertSame('execution(public Foo->bar(*))', $interceptor->pointcutExpression); diff --git a/tests/Core/AttributeAspectLoaderExtensionTest.php b/tests/Core/AttributeAspectLoaderExtensionTest.php index f82f0c0e6..12c39daeb 100644 --- a/tests/Core/AttributeAspectLoaderExtensionTest.php +++ b/tests/Core/AttributeAspectLoaderExtensionTest.php @@ -49,8 +49,7 @@ public function testLoadsAdvicesWithoutCreatingTheAspect(): void $this->assertInstanceOf(GenericPointcutAdvisor::class, $advisor); $interceptor = $advisor->getAdvice(); $this->assertInstanceOf(AbstractInterceptorClass::class, $interceptor); - $this->assertSame(AttributeAspectLoaderExtensionTestUncreatableAspect::class, $interceptor->aspectClass); - $this->assertSame('advice', $interceptor->adviceMethodName); + $this->assertSame([AttributeAspectLoaderExtensionTestUncreatableAspect::class, 'advice'], $interceptor->aspectMethod); $this->assertSame(3, $interceptor->adviceOrder); $this->assertTrue(new ReflectionClass($interceptor::class)->isUninitializedLazyObject($interceptor)); } diff --git a/tests/Core/NativeLazyProxyTest.php b/tests/Core/NativeLazyProxyTest.php index 020f711b9..5805aaf3f 100644 --- a/tests/Core/NativeLazyProxyTest.php +++ b/tests/Core/NativeLazyProxyTest.php @@ -87,6 +87,22 @@ public function testTrustedCreateSkipsProbe(): void $this->assertTrue((new ReflectionClass(LazyProxyFixture::class))->isUninitializedLazyObject($proxy)); $this->assertSame(7, $proxy->getValue()); } + + public function testTrustedCreateSetsRawValuesWithoutInitializing(): void + { + $proxy = NativeLazyProxy::create( + RawValueChildFixture::class, + static fn(): RawValueChildFixture => new RawValueChildFixture('described', 7), + ['description' => 'described'], + ); + $reflection = new ReflectionClass(RawValueChildFixture::class); + + // An inherited private(set) property, which PHP refuses to set in the scope of the child class + $this->assertSame('described', $proxy->description); + $this->assertTrue($reflection->isUninitializedLazyObject($proxy)); + $this->assertSame(7, $proxy->getValue()); + $this->assertFalse($reflection->isUninitializedLazyObject($proxy)); + } } class LazyProxyFixture @@ -99,6 +115,18 @@ public function getValue(): int } } +abstract class RawValueParentFixture +{ + public function __construct(public private(set) string $description, private readonly int $value) {} + + public function getValue(): int + { + return $this->value; + } +} + +final class RawValueChildFixture extends RawValueParentFixture {} + class PropertylessFixture { public function doNothing(): void {} From 6667d0e5fe01783c3d67c87e0bba9a153b865622 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 21:38:12 +0000 Subject: [PATCH 4/7] refactor(core): pass the aspect reflection to the aspect loaders getUnloadedAspects() returns one ReflectionClass per aspect, and load() and loadAndRegister() take it, so the loaders no longer reflect the same aspect again. The methods of AspectLoaderInterface are marked internal. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe --- src/Aop/AGENTS.md | 2 +- src/Console/Command/DebugAspectCommand.php | 8 ++--- src/Core/AGENTS.md | 2 +- src/Core/AspectLoader.php | 21 ++++++------ src/Core/AspectLoaderExtension.php | 4 +-- src/Core/AspectLoaderInterface.php | 21 ++++++++---- src/Core/Cache/CachedAspectLoader.php | 27 ++++++++------- .../Transformer/WeavingTransformer.php | 2 +- .../DebugAdvisorCommandInProcessTest.php | 2 +- tests/Core/AdvisorIndexTest.php | 9 ++--- tests/Core/AspectLoaderTest.php | 29 +++++++++++----- tests/Core/Cache/CachedAspectLoaderTest.php | 33 ++++++++++--------- .../Transformer/WeavingTransformerTest.php | 2 +- 13 files changed, 90 insertions(+), 72 deletions(-) diff --git a/src/Aop/AGENTS.md b/src/Aop/AGENTS.md index 7e1a02838..ccc76c0f0 100644 --- a/src/Aop/AGENTS.md +++ b/src/Aop/AGENTS.md @@ -48,7 +48,7 @@ when `$level > 0` and pops it in `finally`. Check every change with `composer te ## Advice wiring (src/Aop/Framework/) - The — proxy-code accessor: aspect(X::class) fetches aspect from container; advice('advisorId') resolves container-backed closure advice (unwraps Advisor/AbstractInterceptor to raw Closure) -- Interceptor — @internal factory facade with TWO construction modes: before()/after()/around()/afterThrowing(class-string|Closure, ?string $methodName=null, int $order=0, string $expression=''). With aspect class + method name it returns a native PHP 8.4 lazy proxy (via Go\Core\NativeLazyProxy::create) — interceptor construction, The::aspect() resolution and FCC creation all defer until the advice is invoked; adviceOrder, pointcutExpression, aspectClass and adviceMethodName are set on the proxy with setRawValueWithoutLazyInitialization(), so ordering, GeneratedInterceptor::fromAdvice() and compileToPhp() read them without initializing it. The aspect loader (AttributeAspectLoaderExtension) and compiled advisor cache files use this lazy form, so weaving never creates an aspect. A ready Closure constructs eagerly, and generated PROXY classes deliberately use the eager `The::aspect(X::class)->method(...)` FCC form (InterceptorListGenerator): the proxy method/hook is already executing, so the interceptor is needed right now and a lazy detour would be pure overhead; The::advice('') stays eager too. Free to change between releases +- Interceptor — @internal factory facade, before()/after()/around()/afterThrowing(Closure|class-string, ?methodName, order, expression). Aspect class + method name → native lazy proxy whose order, expression and aspectMethod are readable without initialization: the aspect loader and advisor cache files use it, so weaving never creates an aspect. A Closure constructs eagerly; generated proxies use the eager `The::aspect(X::class)->method(...)` form on purpose (reasons in the class docblock) - GeneratedInterceptor — internal descriptor built by AbstractJoinpoint::flatAndSortAdvices() via fromAdvice(); usesContainerAdvice=true when advice closure isn't scoped to an Aspect class - AdviceTypeEnum — Advice::getType() kind + sorting priority (before → after/afterThrowing → around → introduction); replaced AdviceBefore/AdviceAfter/AdviceAround marker interfaces - Advice methods MUST be public (FCC calls them on the aspect instance from generated code) diff --git a/src/Console/Command/DebugAspectCommand.php b/src/Console/Command/DebugAspectCommand.php index 243e2cee0..46ed566f4 100644 --- a/src/Console/Command/DebugAspectCommand.php +++ b/src/Console/Command/DebugAspectCommand.php @@ -106,22 +106,22 @@ private function showAspectInfo(SymfonyStyle $io, string $aspectClass): void if ($docComment) { $io->writeln($this->getPrettyText($docComment)); } - $this->showAspectPointcutsAndAdvisors($io, $aspectClass); + $this->showAspectPointcutsAndAdvisors($io, $refAspect); } /** * Shows an information about aspect pointcuts and advisors * - * @param class-string $aspectClass + * @param ReflectionClass $aspect */ - private function showAspectPointcutsAndAdvisors(SymfonyStyle $io, string $aspectClass): void + private function showAspectPointcutsAndAdvisors(SymfonyStyle $io, ReflectionClass $aspect): void { $container = $this->aspectKernel->getContainer(); $aspectLoader = $container->getService(AspectLoader::class); $io->writeln('Pointcuts and advices'); - $aspectItems = $aspectLoader->load($aspectClass); + $aspectItems = $aspectLoader->load($aspect); $aspectItemsInfo = []; foreach ($aspectItems as $itemId => $item) { $itemType = 'Unknown'; diff --git a/src/Core/AGENTS.md b/src/Core/AGENTS.md index 3684a2bb7..14e126ce4 100644 --- a/src/Core/AGENTS.md +++ b/src/Core/AGENTS.md @@ -11,7 +11,7 @@ ## Aspect loading - AspectLoader — scans aspect classes for pointcut/advice attributes → Advisor[]; CachedAspectLoader decorates it (both implement AspectLoaderInterface) -- Aspects are loaded BY CLASS, never by instance: getUnloadedAspects() lists ids via AspectContainer::getServiceIdsByInterface() (no factory runs), AspectLoaderExtension::load(ReflectionClass) reads attributes only, AttributeAspectLoaderExtension builds advices with the lazy Interceptor facade (aspectClass/adviceMethodName/order/expression readable without initialization). Weaving, warm-up and the advisor cache never create an aspect — also not one PHP can't make lazy (no instance properties); it is created on its first advice call (`The::aspect()`). Never reintroduce `getServicesByInterface(Aspect::class)` or `getClosure($aspect)` on the weaving path +- Aspects are loaded BY CLASS, never by instance: getUnloadedAspects() returns a ReflectionClass per unloaded id (AspectContainer::getServiceIdsByInterface(), no factory runs), which the loaders and extensions share; advices are built with the lazy Interceptor facade. Weaving, warm-up and the advisor cache never create an aspect, also not one PHP can't make lazy (no instance properties). Never reintroduce `getServicesByInterface(Aspect::class)` or `getClosure($aspect)` on the weaving path - 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 diff --git a/src/Core/AspectLoader.php b/src/Core/AspectLoader.php index f44e2129f..6b130cce5 100644 --- a/src/Core/AspectLoader.php +++ b/src/Core/AspectLoader.php @@ -49,17 +49,16 @@ public function __construct( * * @see loadAndRegister() method for registration * - * @param class-string $aspectClass + * @param ReflectionClass $aspect * * @return array */ - public function load(string $aspectClass): array + public function load(ReflectionClass $aspect): array { - $refAspect = new ReflectionClass($aspectClass); $loadedItems = []; foreach ($this->aspectLoaders as $loader) { - $loadedItems += $loader->load($refAspect); + $loadedItems += $loader->load($aspect); } return $loadedItems; @@ -68,21 +67,21 @@ public function load(string $aspectClass): array /** * Loads and register all items of aspect in the container * - * @param class-string $aspectClass + * @param ReflectionClass $aspect */ - public function loadAndRegister(string $aspectClass): void + public function loadAndRegister(ReflectionClass $aspect): void { - $loadedItems = $this->load($aspectClass); + $loadedItems = $this->load($aspect); foreach ($loadedItems as $itemId => $item) { $this->container->add($itemId, $item); } - $this->loadedAspects[$aspectClass] = $aspectClass; + $this->loadedAspects[$aspect->name] = $aspect->name; } /** - * Returns the classes of the aspects registered in the container that are not loaded yet + * Returns the aspects registered in the container that are not loaded yet * - * @return list> + * @return list> */ public function getUnloadedAspects(): array { @@ -90,7 +89,7 @@ public function getUnloadedAspects(): array foreach ($this->container->getServiceIdsByInterface(Aspect::class) as $aspectClass) { if (!isset($this->loadedAspects[$aspectClass]) && is_subclass_of($aspectClass, Aspect::class)) { - $unloadedAspects[] = $aspectClass; + $unloadedAspects[] = new ReflectionClass($aspectClass); } } diff --git a/src/Core/AspectLoaderExtension.php b/src/Core/AspectLoaderExtension.php index a9160933d..5e48c2ea2 100644 --- a/src/Core/AspectLoaderExtension.php +++ b/src/Core/AspectLoaderExtension.php @@ -28,11 +28,9 @@ interface AspectLoaderExtension * Only the class is read, the aspect is never created here: advices refer to it by class and method name, the * aspect is resolved from the container when one of them runs for the first time. * - * @param ReflectionClass $reflectionAspect Reflection of aspect + * @param ReflectionClass $reflectionAspect Reflection of aspect * * @return array - * - * @template T of Aspect */ public function load(ReflectionClass $reflectionAspect): array; } diff --git a/src/Core/AspectLoaderInterface.php b/src/Core/AspectLoaderInterface.php index 2b170c20d..4e2635c2b 100644 --- a/src/Core/AspectLoaderInterface.php +++ b/src/Core/AspectLoaderInterface.php @@ -15,6 +15,7 @@ use Go\Aop\Advisor; use Go\Aop\Aspect; use Go\Aop\Pointcut; +use ReflectionClass; /** * Contract for loading aspects into the container @@ -31,23 +32,31 @@ interface AspectLoaderInterface * * @see loadAndRegister() method for registration * - * @param class-string $aspectClass + * @internal + * + * @param ReflectionClass $aspect * * @return array */ - public function load(string $aspectClass): array; + public function load(ReflectionClass $aspect): array; /** * Loads and register all items of aspect in the container * - * @param class-string $aspectClass + * @internal + * + * @param ReflectionClass $aspect */ - public function loadAndRegister(string $aspectClass): void; + public function loadAndRegister(ReflectionClass $aspect): void; /** - * Returns the classes of the aspects registered in the container that are not loaded yet + * Returns the aspects registered in the container that are not loaded yet + * + * The reflection of each aspect is created once here and passed on to the loaders. + * + * @internal * - * @return list> + * @return list> */ public function getUnloadedAspects(): array; } diff --git a/src/Core/Cache/CachedAspectLoader.php b/src/Core/Cache/CachedAspectLoader.php index f2ef9c30b..da04515a6 100644 --- a/src/Core/Cache/CachedAspectLoader.php +++ b/src/Core/Cache/CachedAspectLoader.php @@ -94,18 +94,17 @@ public function __construct( $this->advisorCacheCompiler = new AdvisorCacheCompiler(); } - public function load(string $aspectClass): array + public function load(ReflectionClass $aspect): array { if ($this->cacheDir === null || $this->cacheDir === '') { - return $this->loader->load($aspectClass); + return $this->loader->load($aspect); } - $refAspect = new ReflectionClass($aspectClass); - $aspectFileName = $refAspect->getFileName(); + $aspectFileName = $aspect->getFileName(); $cacheFileName = $aspectFileName !== false ? $this->resolveCacheFileName($aspectFileName) : null; if ($aspectFileName === false || $cacheFileName === null) { // Aspects without a resolvable source file below the application root are not cached - return $this->loader->load($aspectClass); + return $this->loader->load($aspect); } // With a prebuilt cache an existing advisor cache file is trusted without any @@ -114,7 +113,7 @@ public function load(string $aspectClass): array if ($this->isPrebuiltCache) { $loadedItems = file_exists($cacheFileName) ? $this->loadFromCache($cacheFileName) : null; - return $loadedItems ?? $this->loader->load($aspectClass); + return $loadedItems ?? $this->loader->load($aspect); } // A fresh cache file is used when it is compatible, even when it holds no items (an aspect @@ -126,8 +125,8 @@ public function load(string $aspectClass): array } } - $loadedItems = $this->loader->load($aspectClass); - $this->saveToCache($refAspect->getName(), $loadedItems, $cacheFileName); + $loadedItems = $this->loader->load($aspect); + $this->saveToCache($aspect->name, $loadedItems, $cacheFileName); return $loadedItems; } @@ -135,19 +134,19 @@ public function load(string $aspectClass): array /** * Loads and register all items of aspect in the container */ - public function loadAndRegister(string $aspectClass): void + public function loadAndRegister(ReflectionClass $aspect): void { - $loadedItems = $this->load($aspectClass); + $loadedItems = $this->load($aspect); foreach ($loadedItems as $itemId => $item) { $this->container->add($itemId, $item); } - $this->loadedAspects[$aspectClass] = $aspectClass; + $this->loadedAspects[$aspect->name] = $aspect->name; } /** - * Returns the classes of the aspects registered in the container that are not loaded yet + * Returns the aspects registered in the container that are not loaded yet * - * @return list> + * @return list> */ public function getUnloadedAspects(): array { @@ -155,7 +154,7 @@ public function getUnloadedAspects(): array foreach ($this->container->getServiceIdsByInterface(Aspect::class) as $aspectClass) { if (!isset($this->loadedAspects[$aspectClass]) && is_subclass_of($aspectClass, Aspect::class)) { - $unloadedAspects[] = $aspectClass; + $unloadedAspects[] = new ReflectionClass($aspectClass); } } diff --git a/src/Instrument/Transformer/WeavingTransformer.php b/src/Instrument/Transformer/WeavingTransformer.php index f5239ee3a..d9550ef1b 100644 --- a/src/Instrument/Transformer/WeavingTransformer.php +++ b/src/Instrument/Transformer/WeavingTransformer.php @@ -1103,7 +1103,7 @@ private static function removeCachePath(string $path): void /** * Utility method to load and register unloaded aspects * - * @param list> $unloadedAspects Classes of the unloaded aspects + * @param list<\ReflectionClass> $unloadedAspects Reflections of the unloaded aspects */ private function loadAndRegisterAspects(array $unloadedAspects): void { diff --git a/tests/Console/Command/DebugAdvisorCommandInProcessTest.php b/tests/Console/Command/DebugAdvisorCommandInProcessTest.php index 3bf238b51..b9bcb7d3f 100644 --- a/tests/Console/Command/DebugAdvisorCommandInProcessTest.php +++ b/tests/Console/Command/DebugAdvisorCommandInProcessTest.php @@ -75,7 +75,7 @@ public function testExecuteListsRegisteredAdvisors(): void public function testShowAdvisorsListRegistersUnloadedAspects(): void { - $aspect = $this->createStub(\Go\Aop\Aspect::class)::class; + $aspect = new ReflectionClass($this->createStub(\Go\Aop\Aspect::class)); $aspectLoader = $this->createMock(CachedAspectLoader::class); $aspectLoader->method('getUnloadedAspects')->willReturn([$aspect]); diff --git a/tests/Core/AdvisorIndexTest.php b/tests/Core/AdvisorIndexTest.php index 2b5525090..6815725d2 100644 --- a/tests/Core/AdvisorIndexTest.php +++ b/tests/Core/AdvisorIndexTest.php @@ -23,6 +23,7 @@ use Go\Lang\Attribute\Before; use LogicException; use PHPUnit\Framework\TestCase; +use ReflectionClass; final class AdvisorIndexTest extends TestCase { @@ -78,14 +79,14 @@ public function __construct( private readonly GenericPointcutAdvisor $advisor, ) {} - public function load(string $aspectClass): array + public function load(ReflectionClass $aspect): array { return ['advisor.lazy' => $this->advisor]; } - public function loadAndRegister(string $aspectClass): void + public function loadAndRegister(ReflectionClass $aspect): void { - foreach ($this->load($aspectClass) as $itemId => $item) { + foreach ($this->load($aspect) as $itemId => $item) { $this->container->add($itemId, $item); } $this->isLoaded = true; @@ -93,7 +94,7 @@ public function loadAndRegister(string $aspectClass): void public function getUnloadedAspects(): array { - return $this->isLoaded ? [] : [$this->aspectClass]; + return $this->isLoaded ? [] : [new ReflectionClass($this->aspectClass)]; } }; $index = new AdvisorIndex($container, $loader); diff --git a/tests/Core/AspectLoaderTest.php b/tests/Core/AspectLoaderTest.php index b19f17378..b3d7e74f5 100644 --- a/tests/Core/AspectLoaderTest.php +++ b/tests/Core/AspectLoaderTest.php @@ -17,6 +17,7 @@ use Go\Aop\Pointcut; use LogicException; use PHPUnit\Framework\TestCase; +use ReflectionClass; #[\PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations] class AspectLoaderTest extends TestCase @@ -26,7 +27,7 @@ public function testLoadReturnsEmptyArrayWithoutRegisteredLoaders(): void $container = new Container(); $loader = new AspectLoader($container); - $items = $loader->load(AspectLoaderTestAspectOne::class); + $items = $loader->load(new ReflectionClass(AspectLoaderTestAspectOne::class)); $this->assertSame([], $items); } @@ -46,7 +47,7 @@ public function testLoadCombinesItemsFromAllLoaderExtensions(): void $loader = new AspectLoader($container, $extensionOne, $extensionTwo); - $items = $loader->load(AspectLoaderTestAspectOne::class); + $items = $loader->load(new ReflectionClass(AspectLoaderTestAspectOne::class)); $this->assertSame( ['item.one' => $pointcutOne, 'item.two' => $pointcutTwo], @@ -69,7 +70,7 @@ public function testLoadKeepsFirstLoaderItemWhenKeysCollide(): void $loader = new AspectLoader($container, $extensionOne, $extensionTwo); - $items = $loader->load(AspectLoaderTestAspectOne::class); + $items = $loader->load(new ReflectionClass(AspectLoaderTestAspectOne::class)); // array union (+=) semantics: the first loader's item for a colliding key wins $this->assertSame($pointcutFromFirst, $items['shared.key']); @@ -84,7 +85,7 @@ public function testLoadAndRegisterAddsLoadedItemsToContainer(): void $extension->method('load')->willReturn(['aspect.advisor' => $advisor]); $loader = new AspectLoader($container, $extension); - $loader->loadAndRegister(AspectLoaderTestAspectOne::class); + $loader->loadAndRegister(new ReflectionClass(AspectLoaderTestAspectOne::class)); $this->assertSame($advisor, $container->get('aspect.advisor')); } @@ -100,12 +101,12 @@ public function testGetUnloadedAspectsReturnsOnlyAspectsNotYetLoaded(): void // Before anything is loaded, both aspects are unloaded $this->assertSame( [AspectLoaderTestAspectOne::class, AspectLoaderTestAspectTwo::class], - $loader->getUnloadedAspects(), + self::names($loader->getUnloadedAspects()), ); - $loader->loadAndRegister(AspectLoaderTestAspectOne::class); + $loader->loadAndRegister(new ReflectionClass(AspectLoaderTestAspectOne::class)); - $this->assertSame([AspectLoaderTestAspectTwo::class], $loader->getUnloadedAspects()); + $this->assertSame([AspectLoaderTestAspectTwo::class], self::names($loader->getUnloadedAspects())); } public function testDeferredAspectsAreLoadedWithoutCreatingThem(): void @@ -119,11 +120,21 @@ public function testDeferredAspectsAreLoadedWithoutCreatingThem(): void $extension->method('load')->willReturn([]); $loader = new AspectLoader($container, $extension); - $this->assertSame([AspectLoaderTestAspectOne::class], $loader->getUnloadedAspects()); - $loader->loadAndRegister(AspectLoaderTestAspectOne::class); + $this->assertSame([AspectLoaderTestAspectOne::class], self::names($loader->getUnloadedAspects())); + $loader->loadAndRegister(new ReflectionClass(AspectLoaderTestAspectOne::class)); $this->assertSame([], $loader->getUnloadedAspects()); } + + /** + * @param list> $aspects + * + * @return list> + */ + private static function names(array $aspects): array + { + return array_map(static fn(ReflectionClass $aspect): string => $aspect->name, $aspects); + } } final class AspectLoaderTestAspectOne implements Aspect {} diff --git a/tests/Core/Cache/CachedAspectLoaderTest.php b/tests/Core/Cache/CachedAspectLoaderTest.php index fd0654bc6..b1cfe9bf1 100644 --- a/tests/Core/Cache/CachedAspectLoaderTest.php +++ b/tests/Core/Cache/CachedAspectLoaderTest.php @@ -24,6 +24,7 @@ use Go\Core\Container; use Go\VirtualFileSystem\FileSystem; use PHPUnit\Framework\TestCase; +use ReflectionClass; #[\PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations] class CachedAspectLoaderTest extends TestCase @@ -38,8 +39,8 @@ class CachedAspectLoaderTest extends TestCase private string $cacheDir; - /** @var class-string */ - private string $aspectClass; + /** @var ReflectionClass */ + private ReflectionClass $aspect; /** * Path of the fake aspect source file below the application root @@ -73,7 +74,7 @@ protected function setUp(): void require $this->aspectFileName; $className = 'Go\\Core\\TestFixture\\' . $shortClassName; assert(is_subclass_of($className, Aspect::class)); - $this->aspectClass = $className; + $this->aspect = new ReflectionClass($className); // The naming scheme: app-root prefix replaced by the cache dir, '.php' by '.cache.php' $this->cacheFileName = $this->cacheDir . '/src/Aspect/' . $shortClassName . '.cache.php'; @@ -132,7 +133,7 @@ public function testPrebuiltCacheTrustsExistingCacheFileWithoutFreshnessChecks() $this->writeCacheFile($this->validCacheFileContent(), stale: true); $this->innerLoader->expects($this->never())->method('load'); - $loadedItems = $loader->load($this->aspectClass); + $loadedItems = $loader->load($this->aspect); $this->assertArrayHasKey('pc', $loadedItems); $this->assertInstanceOf(TruePointcut::class, $loadedItems['pc']); @@ -148,7 +149,7 @@ public function testPrebuiltCacheCorruptFileThrowsWithoutRewriting(): void // A corrupt cache file is external interference and fails loudly - no fallback $thrownError = null; try { - $loader->load($this->aspectClass); + $loader->load($this->aspect); } catch (\ParseError $parseError) { $thrownError = $parseError; } @@ -166,7 +167,7 @@ public function testPrebuiltCacheFallsBackToLoaderOnWrongVersionWithoutRewriting $this->innerLoader->expects($this->once())->method('load')->willReturn([]); // A clean version mismatch is the expected upgrade path: silent direct-loader fallback - $this->assertSame([], $loader->load($this->aspectClass)); + $this->assertSame([], $loader->load($this->aspect)); $this->assertSame($wrongVersionContent, file_get_contents($this->cacheFileName)); } @@ -177,7 +178,7 @@ public function testStaleCacheFileIsRebuiltAndRewritten(): void $freshItems = ['pointcut.fresh' => new TruePointcut()]; $this->innerLoader->expects($this->once())->method('load')->willReturn($freshItems); - $this->assertSame($freshItems, $loader->load($this->aspectClass)); + $this->assertSame($freshItems, $loader->load($this->aspect)); // The stale file must have been replaced by a freshly compiled one that includes cleanly $rewrittenData = include $this->cacheFileName; @@ -198,7 +199,7 @@ public function testFreshButCorruptCacheFileThrowsWithoutRebuilding(): void $thrownError = null; try { - $loader->load($this->aspectClass); + $loader->load($this->aspect); } catch (\ParseError $parseError) { $thrownError = $parseError; } @@ -213,7 +214,7 @@ public function testFreshValidCacheFileIsUsedWithoutTouchingTheLoader(): void $this->writeCacheFile($this->validCacheFileContent(), stale: false); $this->innerLoader->expects($this->never())->method('load'); - $loadedItems = $loader->load($this->aspectClass); + $loadedItems = $loader->load($this->aspect); $this->assertArrayHasKey('pc', $loadedItems); $this->assertInstanceOf(TruePointcut::class, $loadedItems['pc']); @@ -232,7 +233,7 @@ public function testPayloadWithAnInvalidEntryIsRejectedAsAWhole(): void $this->innerLoader->expects($this->once())->method('load')->willReturn($freshItems); // Using the valid rest would silently lose advices: the file is rebuilt instead - $this->assertSame($freshItems, $loader->load($this->aspectClass)); + $this->assertSame($freshItems, $loader->load($this->aspect)); $this->assertStringContainsString('pointcut.fresh', (string) file_get_contents($this->cacheFileName)); } @@ -241,12 +242,12 @@ public function testAspectWithoutItemsIsWrittenOnceAndThenOnlyRead(): void $loader = $this->createLoader(0); $this->innerLoader->expects($this->once())->method('load')->willReturn([]); - $this->assertSame([], $loader->load($this->aspectClass)); + $this->assertSame([], $loader->load($this->aspect)); $this->assertFileExists($this->cacheFileName); $cacheModificationTime = filemtime($this->cacheFileName); // An empty compatible cache is a hit: the loader is not called again and nothing is rewritten - $this->assertSame([], $loader->load($this->aspectClass)); + $this->assertSame([], $loader->load($this->aspect)); clearstatcache(); $this->assertSame($cacheModificationTime, filemtime($this->cacheFileName)); } @@ -259,7 +260,7 @@ public function testPrebuiltCacheWithMissingFileLoadsDirectlyWithoutWriting(): v $freshItems = ['pointcut.fresh' => new TruePointcut()]; $this->innerLoader->expects($this->once())->method('load')->willReturn($freshItems); - $this->assertSame($freshItems, $loader->load($this->aspectClass)); + $this->assertSame($freshItems, $loader->load($this->aspect)); $this->assertFileDoesNotExist($this->cacheFileName); } @@ -284,7 +285,7 @@ public function compileToPhp(): \PhpParser\Node\Expr // failure propagates instead of degrading to silent uncached operation $thrownException = null; try { - $loader->load($this->aspectClass); + $loader->load($this->aspect); } catch (NotCompilableException $notCompilableException) { $thrownException = $notCompilableException; } @@ -301,7 +302,7 @@ public function testWithoutCacheDirectoryEveryLoadDelegatesToTheLoader(): void $loadedItems = ['pc' => new TruePointcut()]; $this->innerLoader->expects($this->once())->method('load')->willReturn($loadedItems); - $this->assertSame($loadedItems, $loader->load($this->aspectClass)); + $this->assertSame($loadedItems, $loader->load($this->aspect)); $this->assertFileDoesNotExist($this->cacheFileName); } @@ -313,7 +314,7 @@ public function testAspectOutsideApplicationRootIsLoadedDirectlyWithoutCaching() $loadedItems = ['pc' => new TruePointcut()]; $this->innerLoader->expects($this->once())->method('load')->willReturn($loadedItems); - $this->assertSame($loadedItems, $loader->load($foreignAspect::class)); + $this->assertSame($loadedItems, $loader->load(new ReflectionClass($foreignAspect))); // No shadow file may appear anywhere below the cache directory $cacheEntries = scandir($this->cacheDir); diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index 457af804a..d61356a3f 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -244,7 +244,7 @@ public function testUnadvisedClassWithUnknownParentIsNotTransformed(): void */ public function testLoadsUnloadedAspectsBeforeMatching(): void { - $aspect = $this->createStub(Aspect::class)::class; + $aspect = new ReflectionClass($this->createStub(Aspect::class)); $loader = $this->createMock(AspectLoader::class); $loader->method('getUnloadedAspects')->willReturn([$aspect]); $loader->expects($this->once())->method('loadAndRegister')->with($aspect); From bf0110ba2ad1197eb4486f2c56f257ec21e08c8b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 21:38:12 +0000 Subject: [PATCH 5/7] docs: shorten the notes on weaving without aspect instances Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe --- UPGRADE-4.0.md | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index 542e76b18..a405b5c65 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -114,13 +114,6 @@ object of the wrong type now fails on first use, not at registration. `AspectContainer::addResource()` is part of the interface. `onRegistration()` lets you react to deferred registrations of services implementing an interface. -Weaving never creates aspects: the aspect loader reads an aspect from its class, and each advice refers to it by -class and method name. A deferred aspect is created on the first call of one of its advices, including an aspect -without instance properties, which PHP can not make lazy (its factory used to run as soon as the weaver collected -the aspects). Register aspects under their class name: generated code resolves them with -`$container->getService(MonitorAspect::class)`. `AspectContainer::getServiceIdsByInterface()` lists the ids of the -services implementing an interface without creating them; a custom `containerClass` implements it too. - Custom aspect loader extensions get the reflection of the aspect only: ```php @@ -139,7 +132,8 @@ public function load(ReflectionClass $reflectionAspect): array that expects a PSR-11 container. `getValue()` is renamed to `get()`, and `has()` comes from the PSR interface with the same signature. A custom `containerClass` implements `get()` instead of `getValue()`, plus `setDelegate()`, which attaches the container of your application -(see [Aspects from the application container](docs/configuration.md#aspects-from-the-application-container)). +(see [Aspects from the application container](docs/configuration.md#aspects-from-the-application-container)), +and `getServiceIdsByInterface()`, which lists service ids without creating the services. ```php // before From ef138564c498bf27d82ccb54c7e32704aa4409aa Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 21:47:28 +0000 Subject: [PATCH 6/7] refactor(core): pass the aspect reflection to parsePointcut() and getAdvice() Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe --- CHANGELOG.md | 2 +- UPGRADE-4.0.md | 2 +- src/Core/AbstractAspectLoaderExtension.php | 12 +++++----- src/Core/AttributeAspectLoaderExtension.php | 18 +++++++-------- src/Core/IntroductionAspectExtension.php | 2 +- .../AbstractAspectLoaderExtensionTest.php | 22 +++++++++---------- 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d80b9461..fff1f77aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ See [UPGRADE-4.0.md](UPGRADE-4.0.md) for the migration guide. * [Feature] [BC BREAK] **Interface-keyed registration listeners** — new `AspectContainer::onRegistration(string $interfaceFQCN, Closure $listener)`: the listener receives the id (class-name) and the container whenever a deferred service whose id implements the interface is registered via `addLazyService()`; matching never autoloads, so an id whose class is not loaded yet reaches the listener as a candidate. Values are never touched, so laziness is preserved; with no listeners armed (production) registration stays a pure array write. The kernel arms one `Aspect::class` listener in debug mode to track aspect source files as freshness resources at registration time, resolving them through composer without loading the aspects (previously hardwired into `registerAspect()`). `AspectContainer::addResource()` is now part of the interface and `final public` on `Container` (was `final protected`). * [Feature] [BC BREAK] **PSR-11 container** — `AspectContainer` extends `Psr\Container\ContainerInterface`: `getValue()` is renamed to `get()`, `has()` is inherited from the PSR interface. `ServiceNotFoundException` implements `NotFoundExceptionInterface`, and a container entry of the wrong type in `getService()` throws the new `Go\Aop\Exception\ContainerException` (an `InvalidConfigurationException` implementing `ContainerExceptionInterface`). * [Feature] **Aspects from the application container** — `AspectContainer::setDelegate(ContainerInterface $delegate)` attaches the PSR-11 container of the application: `get()`/`getService()`/`has()` fall back to it for unknown ids. Aspects are registered in `configureAop()` with `addLazyService()` as before, and their factories resolve dependencies from the delegate on first use, once the application has built its container; weaving and cache warm-up never need it. The delegate is never enumerated (PSR-11 has no lookup by interface or tag), framework bridges can collect tagged services and register them. -* [Feature] [BC BREAK] **Weaving never creates aspects** — the aspect loader reads aspects from their classes and builds every aspect-method advice in the lazy static-data form of the `Interceptor` facade, which exposes the aspect class, the advice method, the order and the expression without resolving the aspect. An aspect is created on the first call of one of its advices, also one without instance properties, whose factory used to run as soon as the weaver collected the aspects (before an application container attached with `setDelegate()` existed). New `AspectContainer::getServiceIdsByInterface()` lists service ids without creating the services. `AspectLoaderExtension::load()` takes the `ReflectionClass` of the aspect only, `AbstractAspectLoaderExtension::parsePointcut()` the aspect class name. +* [Feature] [BC BREAK] **Weaving never creates aspects** — the aspect loader reads aspects from their classes and builds every aspect-method advice in the lazy static-data form of the `Interceptor` facade, which exposes the aspect class, the advice method, the order and the expression without resolving the aspect. An aspect is created on the first call of one of its advices, also one without instance properties, whose factory used to run as soon as the weaver collected the aspects (before an application container attached with `setDelegate()` existed). New `AspectContainer::getServiceIdsByInterface()` lists service ids without creating the services. `AspectLoaderExtension::load()` takes the `ReflectionClass` of the aspect only, `AbstractAspectLoaderExtension::parsePointcut()` the `ReflectionClass` of the aspect instead of the instance. * [Feature] Console commands declare their name, description and help with `#[AsCommand]`, `bin/aspect` loads them lazily, and every command returns `Command::SUCCESS`/`Command::FAILURE` (`debug:weaving` used to return the raw error count). `cache:warmup:aop` stops cleanly on SIGINT/SIGTERM. * [Feature] [BC BREAK] **Doctrine bridge rewritten for the trait-based engine (#671)** — new `Go\Bridge\Doctrine\WovenEntityClassLocator` lets Doctrine mapping drivers (ORM 3.6+) discover entities through the autoloader, so the woven class is mapped instead of the original source, and entities loaded before discovery are no longer missed. `MetadataLoadInterceptor` no longer rewrites metadata (the old mapped-superclass workaround only matched the extend-based engine); it now reports woven entities with intercepted mapped properties when Doctrine native lazy objects are disabled. It is registered as a plain listener: `$eventManager->addEventListener(Events::loadClassMetadata, new MetadataLoadInterceptor())` (it is no longer an `EventSubscriber`). diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index a405b5c65..04ee49e38 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -124,7 +124,7 @@ public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array public function load(ReflectionClass $reflectionAspect): array ``` -`AbstractAspectLoaderExtension::parsePointcut()` takes the aspect class name instead of the aspect instance. +`AbstractAspectLoaderExtension::parsePointcut()` takes the `ReflectionClass` of the aspect instead of the aspect instance. ### PSR-11 container diff --git a/src/Core/AbstractAspectLoaderExtension.php b/src/Core/AbstractAspectLoaderExtension.php index 4e6d1a5a3..2ae8f7527 100644 --- a/src/Core/AbstractAspectLoaderExtension.php +++ b/src/Core/AbstractAspectLoaderExtension.php @@ -41,16 +41,16 @@ public function __construct( * General method for parsing pointcuts * * @throws PointcutSyntaxException if there was an error during parsing - * @param class-string $aspectClass Aspect the expression belongs to, `$this` in it + * @param ReflectionClass $aspect Aspect the expression belongs to, `$this` in it * @param ReflectionMethod|ReflectionProperty|ReflectionClass $reflection * @template T of Aspect */ final protected function parsePointcut( - string $aspectClass, + ReflectionClass $aspect, ReflectionMethod|ReflectionProperty|ReflectionClass $reflection, string $pointcutExpression, ): Pointcut { - $stream = $this->makeLexicalAnalyze($aspectClass, $reflection, $pointcutExpression); + $stream = $this->makeLexicalAnalyze($aspect, $reflection, $pointcutExpression); return $this->parseTokenStream($reflection, $pointcutExpression, $stream); } @@ -58,19 +58,19 @@ final protected function parsePointcut( /** * Performs lexical analyze of pointcut * - * @param class-string $aspectClass + * @param ReflectionClass $aspect * @param ReflectionMethod|ReflectionProperty|ReflectionClass $reflection * @template T of Aspect * * @throws PointcutSyntaxException */ private function makeLexicalAnalyze( - string $aspectClass, + ReflectionClass $aspect, ReflectionMethod|ReflectionProperty|ReflectionClass $reflection, string $pointcutExpression, ): TokenStream { try { - $resolvedThisPointcut = str_replace('$this', $aspectClass, $pointcutExpression); + $resolvedThisPointcut = str_replace('$this', $aspect->name, $pointcutExpression); $stream = $this->pointcutLexer->lex($resolvedThisPointcut); } catch (RecognitionException $e) { throw PointcutSyntaxException::unrecognizedLexicalStructure($pointcutExpression, $reflection, $e); diff --git a/src/Core/AttributeAspectLoaderExtension.php b/src/Core/AttributeAspectLoaderExtension.php index 6529af1e3..994ac596a 100644 --- a/src/Core/AttributeAspectLoaderExtension.php +++ b/src/Core/AttributeAspectLoaderExtension.php @@ -40,7 +40,6 @@ final class AttributeAspectLoaderExtension extends AbstractAspectLoaderExtension public function load(ReflectionClass $reflectionAspect): array { - $aspectClass = $reflectionAspect->getName(); $loadedItems = []; foreach ($reflectionAspect->getMethods() as $aspectMethod) { $methodId = $reflectionAspect->getName() . '->' . $aspectMethod->getName(); @@ -54,12 +53,12 @@ public function load(ReflectionClass $reflectionAspect): array foreach ($pointcutAttributes as $reflectionAttribute) { $attribute = $reflectionAttribute->newInstance(); - $loadedItems[$methodId] = $this->parsePointcut($aspectClass, $aspectMethod, $attribute->expression); + $loadedItems[$methodId] = $this->parsePointcut($reflectionAspect, $aspectMethod, $attribute->expression); } foreach ($adviceAttributes as $reflectionAttribute) { $attribute = $reflectionAttribute->newInstance(); - $pointcut = $this->parsePointcut($aspectClass, $aspectMethod, $attribute->expression); - $interceptor = $this->getAdvice($attribute, $aspectClass, $aspectMethod); + $pointcut = $this->parsePointcut($reflectionAspect, $aspectMethod, $attribute->expression); + $interceptor = $this->getAdvice($attribute, $reflectionAspect, $aspectMethod); $advisorId = $pointcutAttributes === [] ? $methodId : $methodId . self::ADVISOR_ID_SUFFIX; $loadedItems[$advisorId] = new GenericPointcutAdvisor($pointcut, $interceptor); @@ -75,22 +74,23 @@ public function load(ReflectionClass $reflectionAspect): array * The interceptor is the lazy form of the {@see Interceptor} facade: it carries the aspect class, the method name, * the order and the expression as plain data, and resolves the aspect from the container on first use only. * - * @param class-string $aspectClass + * @param ReflectionClass $aspect * * @throws AspectException If the advice method is not public or the attribute is unsupported */ protected function getAdvice( AbstractInterceptor $interceptorAttribute, - string $aspectClass, + ReflectionClass $aspect, ReflectionMethod $aspectMethod, ): InterceptorInterface { if (!$aspectMethod->isPublic()) { throw AspectException::nonPublicAdviceMethod($aspectMethod); } - $methodName = $aspectMethod->getName(); - $order = $interceptorAttribute->order; - $expression = $interceptorAttribute->expression; + $aspectClass = $aspect->name; + $methodName = $aspectMethod->getName(); + $order = $interceptorAttribute->order; + $expression = $interceptorAttribute->expression; return match (true) { $interceptorAttribute instanceof Before => Interceptor::before($aspectClass, $methodName, $order, $expression), diff --git a/src/Core/IntroductionAspectExtension.php b/src/Core/IntroductionAspectExtension.php index 5c3939543..0be6d4226 100644 --- a/src/Core/IntroductionAspectExtension.php +++ b/src/Core/IntroductionAspectExtension.php @@ -43,7 +43,7 @@ public function load(ReflectionClass $reflectionAspect): array ? $reflectionAttribute->newInstance() : null; if ($attribute instanceof DeclareParents) { - $pointcut = $this->parsePointcut($reflectionAspect->getName(), $aspectProperty, $attribute->expression); + $pointcut = $this->parsePointcut($reflectionAspect, $aspectProperty, $attribute->expression); // Introduction doesn't have own syntax and uses any suitable class-filter $pointcut = new Pointcut\AndPointcut( Pointcut::KIND_INTRODUCTION | Pointcut::KIND_CLASS, diff --git a/tests/Core/AbstractAspectLoaderExtensionTest.php b/tests/Core/AbstractAspectLoaderExtensionTest.php index 4010e62c8..fb7efb26f 100644 --- a/tests/Core/AbstractAspectLoaderExtensionTest.php +++ b/tests/Core/AbstractAspectLoaderExtensionTest.php @@ -40,7 +40,7 @@ public function testParsesValidPointcutExpressionForMethodReflection(): void $aspect = new AbstractAspectLoaderExtensionTestAspect(); $reflection = new ReflectionMethod($aspect, 'someMethod'); - $pointcut = $this->extension->doParsePointcut($aspect::class, $reflection, 'execution(public Foo->bar(*))'); + $pointcut = $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'execution(public Foo->bar(*))'); // @phpstan-ignore method.alreadyNarrowedType (runtime double-check of the declared return type) $this->assertInstanceOf(Pointcut::class, $pointcut); @@ -53,7 +53,7 @@ public function testResolvesThisInPointcutExpressionToAspectClassName(): void // "$this" must be replaced with the aspect's own class name before lexing $pointcut = $this->extension->doParsePointcut( - $aspect::class, + new ReflectionClass($aspect), $reflection, 'execution(public $this->someMethod(*))', ); @@ -70,7 +70,7 @@ public function testThrowsUnexpectedValueExceptionOnLexicalErrorForMethodReflect $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Can not recognize the lexical structure/'); - $this->extension->doParsePointcut($aspect::class, $reflection, 'execution(public Foo->@#$%^bar(*))'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'execution(public Foo->@#$%^bar(*))'); } public function testThrowsUnexpectedValueExceptionOnLexicalErrorForPropertyReflection(): void @@ -81,7 +81,7 @@ public function testThrowsUnexpectedValueExceptionOnLexicalErrorForPropertyRefle $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Can not recognize the lexical structure/'); - $this->extension->doParsePointcut($aspect::class, $reflection, 'execution(public Foo->@#$%^bar(*))'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'execution(public Foo->@#$%^bar(*))'); } public function testThrowsUnexpectedValueExceptionOnLexicalErrorForClassReflection(): void @@ -92,7 +92,7 @@ public function testThrowsUnexpectedValueExceptionOnLexicalErrorForClassReflecti $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Can not recognize the lexical structure/'); - $this->extension->doParsePointcut($aspect::class, $reflection, 'execution(public Foo->@#$%^bar(*))'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'execution(public Foo->@#$%^bar(*))'); } public function testThrowsUnexpectedValueExceptionOnParserErrorForMethodReflection(): void @@ -103,7 +103,7 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForMethodReflecti $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Unexpected token/'); - $this->extension->doParsePointcut($aspect::class, $reflection, 'public execution(*)'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'public execution(*)'); } public function testThrowsUnexpectedValueExceptionOnParserErrorForPropertyReflection(): void @@ -114,7 +114,7 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForPropertyReflec $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Unexpected token/'); - $this->extension->doParsePointcut($aspect::class, $reflection, 'public execution(*)'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'public execution(*)'); } public function testThrowsUnexpectedValueExceptionOnParserErrorForClassReflection(): void @@ -125,7 +125,7 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForClassReflectio $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Unexpected token/'); - $this->extension->doParsePointcut($aspect::class, $reflection, 'public execution(*)'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'public execution(*)'); } } @@ -141,15 +141,15 @@ public function load(ReflectionClass $reflectionAspect): array /** * @template T of Aspect - * @param class-string $aspectClass + * @param ReflectionClass $aspect * @param ReflectionMethod|ReflectionProperty|ReflectionClass $reflection */ public function doParsePointcut( - string $aspectClass, + ReflectionClass $aspect, ReflectionMethod|ReflectionProperty|ReflectionClass $reflection, string $pointcutExpression, ): Pointcut { - return $this->parsePointcut($aspectClass, $reflection, $pointcutExpression); + return $this->parsePointcut($aspect, $reflection, $pointcutExpression); } } From 0479d806d21f4559256a56a9b2476addaffbc0be Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 22:15:50 +0000 Subject: [PATCH 7/7] docs: drop the upgrade notes of weaving without aspect instances Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe --- UPGRADE-4.0.md | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index 04ee49e38..7bad58a02 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -114,26 +114,13 @@ object of the wrong type now fails on first use, not at registration. `AspectContainer::addResource()` is part of the interface. `onRegistration()` lets you react to deferred registrations of services implementing an interface. -Custom aspect loader extensions get the reflection of the aspect only: - -```php -// before -public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array - -// 4.0 -public function load(ReflectionClass $reflectionAspect): array -``` - -`AbstractAspectLoaderExtension::parsePointcut()` takes the `ReflectionClass` of the aspect instead of the aspect instance. - ### PSR-11 container `AspectContainer` extends `Psr\Container\ContainerInterface`, so the container can be passed to any code that expects a PSR-11 container. `getValue()` is renamed to `get()`, and `has()` comes from the PSR interface with the same signature. A custom `containerClass` implements `get()` instead of `getValue()`, plus `setDelegate()`, which attaches the container of your application -(see [Aspects from the application container](docs/configuration.md#aspects-from-the-application-container)), -and `getServiceIdsByInterface()`, which lists service ids without creating the services. +(see [Aspects from the application container](docs/configuration.md#aspects-from-the-application-container)). ```php // before