diff --git a/docs/php84-limitations.md b/docs/php84-limitations.md index c70f2cb2..dda3cb4a 100644 --- a/docs/php84-limitations.md +++ b/docs/php84-limitations.md @@ -33,7 +33,9 @@ Asymmetric visibility on **non-readonly** properties _is_ preserved in generated ## Lazy Objects -PHP 8.4 introduced `ReflectionClass::newLazyProxy()` and `ReflectionClass::newLazyGhost()` for [lazy object initialization](https://wiki.php.net/rfc/lazy-objects). While the framework's `Container` uses lazy objects internally, **lazy object initialization itself is not interceptable** by the framework. There is no join point for the moment a lazy proxy materializes its backing instance. +PHP 8.4 introduced `ReflectionClass::newLazyProxy()` and `ReflectionClass::newLazyGhost()` for [lazy object initialization](https://wiki.php.net/rfc/lazy-objects). The framework's `Container` uses native lazy proxies for its own deferred services and for aspects registered by class name: retrieving such a service hands out a typed, `instanceof`-correct `newLazyProxy()` instance, and the registered factory only runs on the first real interaction with that object (classes PHP cannot make lazy — internal classes and their non-`stdClass` subclasses, abstract classes, enums, and readonly classes before PHP 8.5 — fall back to eager construction). + +However, **lazy object initialization itself is not interceptable** by the framework. There is no join point for the moment a lazy proxy materializes its backing instance. ## Summary Table diff --git a/src/Aop/Pointcut/AndPointcut.php b/src/Aop/Pointcut/AndPointcut.php index e9545b20..e3de54ae 100644 --- a/src/Aop/Pointcut/AndPointcut.php +++ b/src/Aop/Pointcut/AndPointcut.php @@ -56,13 +56,10 @@ public function matches( ReflectionClass|ReflectionFileNamespace $context, ReflectionMethod|ReflectionProperty|ReflectionFunction|null $reflector = null ): bool { - foreach ($this->pointcuts as $singlePointcut) { - if (!$singlePointcut->matches($context, $reflector)) { - return false; - } - } - - return true; + return array_all( + $this->pointcuts, + fn(Pointcut $singlePointcut): bool => $singlePointcut->matches($context, $reflector) + ); } public function getKind(): int diff --git a/src/Aop/Pointcut/OrPointcut.php b/src/Aop/Pointcut/OrPointcut.php index 9a9457f2..ffaab6f5 100644 --- a/src/Aop/Pointcut/OrPointcut.php +++ b/src/Aop/Pointcut/OrPointcut.php @@ -53,13 +53,10 @@ public function matches( ReflectionClass|ReflectionFileNamespace $context, ReflectionMethod|ReflectionProperty|ReflectionFunction|null $reflector = null ): bool { - foreach ($this->pointcuts as $singlePointcut) { - if ($singlePointcut->matches($context, $reflector)) { - return true; - } - } - - return false; + return array_any( + $this->pointcuts, + fn(Pointcut $singlePointcut): bool => $singlePointcut->matches($context, $reflector) + ); } public function getKind(): int diff --git a/src/Core/AdviceMatcher.php b/src/Core/AdviceMatcher.php index 7e1c3129..2464ce23 100644 --- a/src/Core/AdviceMatcher.php +++ b/src/Core/AdviceMatcher.php @@ -87,7 +87,7 @@ public function getAdvicesForClass(ReflectionClass $class, array $advisors): arr $parentClass = $class->getParentClass(); $originalClass = $class; - if ($parentClass && strpos($parentClass->name, AspectContainer::AOP_PROXIED_SUFFIX) !== false) { + if ($parentClass && str_contains($parentClass->name, AspectContainer::AOP_PROXIED_SUFFIX)) { $originalClass = $parentClass; } diff --git a/src/Core/AspectContainer.php b/src/Core/AspectContainer.php index 0fd1c2f7..afd926b5 100644 --- a/src/Core/AspectContainer.php +++ b/src/Core/AspectContainer.php @@ -69,7 +69,8 @@ interface AspectContainer /** * Returns a service from the container. * - * Services registered via addLazyService() are constructed by their factory on first retrieval. + * Services registered via addLazyService() are returned as typed, instanceof-correct + * native lazy objects where possible; their factory runs on first actual use. * * @param class-string $className Class-name of service to retrieve from the container * @return T @@ -113,10 +114,11 @@ public function has(string $id): bool; * Passing an aspect instance registers it immediately, exactly as before. * * Passing a class-name defers construction until the aspect is first needed (first - * advice hit, or aspect enumeration during weaving), keeping it off the hot boot path. + * advice hit, or first real interaction with the lazy object handed out during aspect + * enumeration on the weaving path), keeping it off the hot boot path. * An aspect with required constructor arguments must also pass a factory closure that * creates the instance; without a factory the class must be default-constructible, - * which is validated when the aspect materializes. + * which is validated when the aspect materializes into its lazy object. * * @param Aspect|class-string $aspectOrClassName Aspect instance or its class-name * @param null|Closure(AspectContainer $container): Aspect $aspectFactory Factory for deferred @@ -142,9 +144,11 @@ public function add(string $id, mixed $value): void; /** * Adds a deferred service definition to the container. * - * Nothing is autoloaded or constructed at registration time: the factory closure is - * invoked once, on the first retrieval of the service (or when the service matches a - * getServicesByInterface() query), and the result is stored as a regular container entry. + * Nothing is autoloaded or constructed at registration time. On the first retrieval of + * the service (or when the service matches a getServicesByInterface() query) the entry + * materializes as a native lazy object of the service class, and the factory closure is + * invoked once, on the first actual interaction with that object. For classes that PHP + * cannot make lazy the factory is invoked at materialization time instead. * * @param class-string $id Identifier of value to store, must be equal to the class-name * @param Closure(AspectContainer $container): T $lazyInitializationClosure diff --git a/src/Core/CachedAspectLoader.php b/src/Core/CachedAspectLoader.php index 4d79b90f..f58f81b1 100644 --- a/src/Core/CachedAspectLoader.php +++ b/src/Core/CachedAspectLoader.php @@ -12,8 +12,6 @@ namespace Go\Core; -use AllowDynamicProperties; -use RuntimeException; use Go\Aop\Advisor; use Go\Aop\Aspect; use Go\Aop\Features; @@ -23,12 +21,17 @@ /** * Cached loader is responsible for faster initialization of pointcuts/advisors for concrete aspect * - * @property AspectLoader $loader * @phpstan-import-type KernelOptions from AspectKernel */ -#[AllowDynamicProperties] class CachedAspectLoader extends AspectLoader { + /** + * Original loader, resolved from the container on first access and memoized in the backing store + */ + private AspectLoader $loader { + get => $this->loader ??= $this->container->getService($this->loaderId); + } + /** * Path to the cache directory */ @@ -99,17 +102,6 @@ public function load(Aspect $aspect): array return $loadedItems; } - public function __get(string $name): AspectLoader - { - if ($name === 'loader') { - $this->loader = $this->container->getService($this->loaderId); - - return $this->loader; - } - throw new RuntimeException('Not implemented'); - } - - /** * Loads pointcuts and advisors from the file * diff --git a/src/Core/Container.php b/src/Core/Container.php index 919c0787..a489e22b 100644 --- a/src/Core/Container.php +++ b/src/Core/Container.php @@ -38,6 +38,12 @@ class Container implements AspectContainer */ private array $factories = []; + /** + * @var array Optional eager validators for deferred services, run when the + * lazy object is created (first retrieval) - before the factory itself runs (first actual use) + */ + private array $factoryValidators = []; + /** * @var array> Holds information about mapping of interface tags into identifiers */ @@ -113,12 +119,20 @@ final public function registerAspect(Aspect|string $aspectOrClassName, ?Closure } // Deferred registration by class-name: the aspect is constructed on first use - // (first advice hit, or aspect enumeration on the weaving path), so a hot-cache - // request never pays for aspects it does not touch. + // (first advice hit, or first real interaction with the lazy object handed out + // on the weaving path), so a hot-cache request never pays for aspects it does + // not touch. $this->addLazyService($aspectOrClassName, function () use ($aspectOrClassName, $aspectFactory): Aspect { return $this->materializeAspect($aspectOrClassName, $aspectFactory); }); + // Cheap aspect declaration checks (implements Aspect, constructibility) run as soon + // as the service materializes into a lazy object, so misconfiguration surfaces on + // retrieval - construction itself stays deferred until first actual use. + $this->factoryValidators[$aspectOrClassName] = function () use ($aspectOrClassName, $aspectFactory): void { + $this->validateAspectRegistration($aspectOrClassName, $aspectFactory); + }; + // In debug mode the aspect's source file must be tracked as a resource right away: // SourceTransformingLoader consults resource freshness before any aspect materializes. // Production skips this - its warm path never checks freshness, and a cache miss @@ -142,9 +156,9 @@ final public function registerAspect(Aspect|string $aspectOrClassName, ?Closure */ private function materializeAspect(string $aspectClassName, ?Closure $aspectFactory): Aspect { - if (!is_subclass_of($aspectClassName, Aspect::class)) { - throw new AspectException("Aspect class $aspectClassName must implement " . Aspect::class); - } + $this->validateAspectRegistration($aspectClassName, $aspectFactory); + assert(is_subclass_of($aspectClassName, Aspect::class)); + if ($aspectFactory !== null) { $aspect = $aspectFactory($this); if (!$aspect instanceof $aspectClassName) { @@ -154,17 +168,32 @@ private function materializeAspect(string $aspectClassName, ?Closure $aspectFact return $aspect; } - $constructor = (new ReflectionClass($aspectClassName))->getConstructor(); - if ($constructor !== null && $constructor->getNumberOfRequiredParameters() > 0) { - throw new AspectException( - "Aspect $aspectClassName has required constructor arguments, " - . "pass a factory closure to registerAspect() to create it" - ); - } - return new $aspectClassName(); } + /** + * Validates a deferred aspect registration without constructing the aspect + * + * @param null|Closure(AspectContainer): Aspect $aspectFactory + * + * @throws AspectException if the class is not an aspect or cannot be default-constructed + */ + private function validateAspectRegistration(string $aspectClassName, ?Closure $aspectFactory): void + { + if (!is_subclass_of($aspectClassName, Aspect::class)) { + throw new AspectException("Aspect class $aspectClassName must implement " . Aspect::class); + } + if ($aspectFactory === null) { + $constructor = (new ReflectionClass($aspectClassName))->getConstructor(); + if ($constructor !== null && $constructor->getNumberOfRequiredParameters() > 0) { + throw new AspectException( + "Aspect $aspectClassName has required constructor arguments, " + . "pass a factory closure to registerAspect() to create it" + ); + } + } + } + /** * Whether the kernel that owns this container runs in debug mode */ @@ -203,6 +232,7 @@ final public function addLazyService(string $id, Closure $lazyInitializationClos throw new AspectException("Lazy service id must be a valid class name, \"$id\" given"); } $this->factories[$id] = $lazyInitializationClosure; + unset($this->factoryValidators[$id]); } final public function getService(string $className): object @@ -240,10 +270,10 @@ final public function has(string $id): bool final public function getServicesByInterface(string $interfaceTagClassName): array { - // Deferred services are only tagged once constructed, so materialize the - // pending ones that implement the requested interface first. This path is - // only taken during weaving/console runs, never on a hot request. - // Both the is_subclass_of() autoload and the factory invocation can re-enter + // Deferred services are only tagged once materialized (as lazy objects), so + // materialize the pending ones that implement the requested interface first. + // This path is only taken during weaving/console runs, never on a hot request. + // The is_subclass_of() autoload (and an eager fallback factory) can re-enter // this method (an aspect class autoloaded here goes through the weaving // pipeline, which enumerates aspects again), consuming pending factories from // under this loop - hence the existence re-check and the tolerant materialization. @@ -262,7 +292,14 @@ final public function getServicesByInterface(string $interfaceTagClassName): arr } /** - * Constructs a deferred service from its registered factory and stores it in the container + * Materializes a deferred service into a container entry and tags it by its interfaces. + * + * Where the class supports it, the entry becomes a native lazy proxy + * ({@see ReflectionClass::newLazyProxy()}): a typed, instanceof-correct instance of the + * service class whose factory only runs on first actual interaction with the object. + * Classes that PHP cannot make lazy (internal classes and their non-stdClass subclasses, + * abstract classes, enums, readonly classes before PHP 8.5) and ids that are not loadable + * classes fall back to invoking the factory eagerly, as before. */ private function materializeService(string $id): void { @@ -271,9 +308,78 @@ private function materializeService(string $id): void // Already materialized by a re-entrant call (e.g. triggered through autoloading) return; } - // Unset before invoking the factory so a re-entrant lookup cannot run it twice + // Unset before touching the class: autoloading (class_exists/reflection below) or an + // eager fallback factory can re-enter the container and must not materialize $id twice unset($this->factories[$id]); - $this->add($id, $factory($this)); + + $validator = $this->factoryValidators[$id] ?? null; + unset($this->factoryValidators[$id]); + $validator?->__invoke(); + + $this->add($id, $this->createLazyService($id, $factory)); + } + + /** + * Creates the container entry for a deferred service: a native lazy proxy when possible, + * otherwise the eagerly invoked factory result + * + * @param Closure(AspectContainer): object $factory + */ + private function createLazyService(string $id, Closure $factory): object + { + if (!class_exists($id)) { + return $factory($this); + } + $reflection = new ReflectionClass($id); + if (!self::isLazyProxyCompatible($reflection)) { + return $factory($this); + } + + return $reflection->newLazyProxy(function () use ($id, $factory): object { + $instance = $factory($this); + if (!$instance instanceof $id) { + throw new AspectException("Service $id is not properly registered"); + } + + return $instance; + }); + } + + /** + * Whether PHP can create a native lazy proxy for the given class + * + * @param ReflectionClass $reflection + */ + private static function isLazyProxyCompatible(ReflectionClass $reflection): bool + { + if ($reflection->isInternal() || $reflection->isAbstract() || $reflection->isEnum()) { + return false; + } + // Lazy objects for readonly classes are only supported since PHP 8.5 + if (PHP_VERSION_ID < 80500 && $reflection->isReadOnly()) { + return false; + } + // PHP creates lazy objects of classes without instance properties as already + // initialized, so the initializer (and with it the service factory) would never + // run - such services keep the eager construction path + $hasInstanceProperties = false; + foreach ($reflection->getProperties() as $property) { + if (!$property->isStatic() && !$property->isVirtual()) { + $hasInstanceProperties = true; + break; + } + } + if (!$hasInstanceProperties) { + return false; + } + // Subclasses of internal classes (other than stdClass) cannot be lazy + for ($parent = $reflection->getParentClass(); $parent !== false; $parent = $parent->getParentClass()) { + if ($parent->isInternal()) { + return $parent->getName() === 'stdClass'; + } + } + + return true; } final public function hasAnyResourceChangedSince(int $timestamp): bool diff --git a/src/Core/LazyAdvisorAccessor.php b/src/Core/LazyAdvisorAccessor.php index 316607d9..a2e5ae35 100644 --- a/src/Core/LazyAdvisorAccessor.php +++ b/src/Core/LazyAdvisorAccessor.php @@ -12,7 +12,6 @@ namespace Go\Core; -use AllowDynamicProperties; use Go\Aop\Advisor; use Go\Aop\Aspect; use Go\Aop\AspectException; @@ -22,36 +21,37 @@ /** * Provides an interface for loading of advisors from the container */ -#[AllowDynamicProperties] final class LazyAdvisorAccessor { + /** + * @var array Resolved interceptors, keyed by advisor identifier + */ + private array $interceptors = []; + /** * Accessor constructor */ public function __construct( - protected readonly AspectContainer $container, - protected readonly AspectLoader $loader + private readonly AspectContainer $container, + private readonly AspectLoader $loader ) {} /** - * Returns the Interceptor for the given advisor name, loading and caching it on first access. - * - * Prefer this over the magic property accessor when the name is a variable — PHP's `__get()` is - * identical in behaviour, but static-analysis tools cannot track its return type for variable keys. + * Returns the Interceptor for the given advisor name, loading and caching it on first access * * @throws InvalidArgumentException if referenced value is not an advisor or its advice is not an Interceptor */ public function getInterceptor(string $name): Interceptor { - return $this->__get($name); + return $this->interceptors[$name] ??= $this->loadInterceptor($name); } /** - * Magic advice accessor + * Resolves an interceptor from the container, loading the owning aspect on demand * * @throws InvalidArgumentException if referenced value is not an advisor or its advice is not an Interceptor */ - public function __get(string $name): Interceptor + private function loadInterceptor(string $name): Interceptor { if (!$this->container->has($name)) { [$aspectName] = explode('->', $name, 2); @@ -60,7 +60,6 @@ public function __get(string $name): Interceptor } $aspectInstance = $this->container->getService($aspectName); $this->loader->loadAndRegister($aspectInstance); - } $advisor = $this->container->getValue($name); if (!$advisor instanceof Advisor) { @@ -70,8 +69,7 @@ public function __get(string $name): Interceptor if (!$advice instanceof Interceptor) { throw new InvalidArgumentException("Advice {$name} is not an Interceptor"); } - $this->$name = $advice; - return $this->$name; + return $advice; } } diff --git a/src/Instrument/FileSystem/Enumerator.php b/src/Instrument/FileSystem/Enumerator.php index 8dffa19a..2264cfd1 100644 --- a/src/Instrument/FileSystem/Enumerator.php +++ b/src/Instrument/FileSystem/Enumerator.php @@ -81,7 +81,7 @@ public function enumerate(): Iterator $iterator = $finder->getIterator(); // on Windows platform the default iterator is unable to rewind, not sure why - if (strpos(PHP_OS, 'WIN') === 0) { + if (PHP_OS_FAMILY === 'Windows') { $iterator = new ArrayIterator(iterator_to_array($iterator)); } @@ -105,30 +105,17 @@ public function getFilter(): Closure $fullPath = $this->getFileFullPath($file); // Do not touch files that not under rootDirectory - if (strpos($fullPath, $rootDirectory) !== 0) { + if (!str_starts_with($fullPath, $rootDirectory)) { return false; } - if (!empty($includePaths)) { - $found = false; - foreach ($includePaths as $includePattern) { - if (fnmatch("{$includePattern}*", $fullPath, FNM_NOESCAPE)) { - $found = true; - break; - } - } - if (!$found) { - return false; - } - } + $matchesPattern = fn(string $pattern): bool => fnmatch("{$pattern}*", $fullPath, FNM_NOESCAPE); - foreach ($excludePaths as $excludePattern) { - if (fnmatch("{$excludePattern}*", $fullPath, FNM_NOESCAPE)) { - return false; - } + if (!empty($includePaths) && !array_any($includePaths, $matchesPattern)) { + return false; } - return true; + return !array_any($excludePaths, $matchesPattern); }; } @@ -155,7 +142,9 @@ private function getInPaths(): array $inPaths = []; foreach ($this->includePaths as $path) { - if (strpos($path, $this->rootDirectory, 0) === false) { + // Include paths must be below the root directory: this is a prefix check, + // a path merely containing the root somewhere else must be rejected + if (!str_starts_with($path, $this->rootDirectory)) { throw new UnexpectedValueException(sprintf('Path %s is not in %s', $path, $this->rootDirectory)); } diff --git a/src/Instrument/PathResolver.php b/src/Instrument/PathResolver.php index 8e09f588..b6bf97d7 100644 --- a/src/Instrument/PathResolver.php +++ b/src/Instrument/PathResolver.php @@ -63,7 +63,7 @@ public static function realpath(string|array $somePath, bool $shouldCheckExisten // resolve path parts (single dot, double dot and double delimiters) $path = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $path); - if (strpos($path, '.') !== false) { + if (str_contains($path, '.')) { $parts = explode(DIRECTORY_SEPARATOR, $path); $absolutes = []; foreach ($parts as $part) { diff --git a/src/Instrument/Transformer/StreamMetaData.php b/src/Instrument/Transformer/StreamMetaData.php index 29fca079..065590f2 100644 --- a/src/Instrument/Transformer/StreamMetaData.php +++ b/src/Instrument/Transformer/StreamMetaData.php @@ -21,11 +21,33 @@ /** * Stream metadata object - * - * @property-read string $source */ class StreamMetaData { + /** + * Source code represented by the token stream. + * + * Reading rebuilds the source directly from {@see self::$tokenStream}. Writing is + * deprecated: it re-tokenizes the given source into the token stream instead - use + * {@see self::setTokenStreamFromRawTokens()} directly. + */ + public string $source { + get { + $transformedSource = ''; + foreach ($this->tokenStream as $token) { + if ($token->id !== 0) { + $transformedSource .= $token->text; + } + } + + return $transformedSource; + } + set { + trigger_error('Setting StreamMetaData->source is deprecated, use tokenStream instead', E_USER_DEPRECATED); + $this->setTokenStreamFromRawTokens(...PhpToken::tokenize($value)); + } + } + /** * Mapping between array keys and properties * @@ -107,57 +129,6 @@ public function __construct($stream, ?string $source = null) $this->setTokenStreamFromRawTokens(...ReflectionEngine::getParser()->getTokens()); } - /** - * @inheritDoc - */ - public function __get(string $name): mixed - { - if ($name === 'source') { - return $this->getSource(); - } - - return null; - } - - /** - * @inheritDoc - */ - public function __set(string $name, mixed $value): void - { - if ($name === 'source' && is_string($value)) { - trigger_error('Setting StreamMetaData->source is deprecated, use tokenStream instead', E_USER_DEPRECATED); - $this->setSource($value); - } - } - - /** - * Returns source code directly from tokens - */ - private function getSource(): string - { - $transformedSource = ''; - foreach ($this->tokenStream as $token) { - if ($token->id !== 0) { - $transformedSource .= $token->text; - } - } - - return $transformedSource; - } - - /** - * Sets the new source for this file - * - * @TODO: Unfortunately, AST won't be changed, so please be accurate during transformation - * - * @param string $newSource - */ - private function setSource(string $newSource): void - { - $rawTokens = PhpToken::tokenize($newSource); - $this->setTokenStreamFromRawTokens(...$rawTokens); - } - /** * Sets an array of token identifiers for this file */ diff --git a/src/Lang/Attribute/AbstractAttribute.php b/src/Lang/Attribute/AbstractAttribute.php index 7dd3f43f..038bfe5c 100644 --- a/src/Lang/Attribute/AbstractAttribute.php +++ b/src/Lang/Attribute/AbstractAttribute.php @@ -12,8 +12,6 @@ namespace Go\Lang\Attribute; -use BadMethodCallException; - abstract class AbstractAttribute { /** @@ -24,26 +22,4 @@ public function __construct( readonly public string $expression = '', readonly public int $order = 0, ) {} - - /** - * Error handler for unknown property accessor in attribute class. - */ - public function __get(string $name): never - { - throw new BadMethodCallException( - sprintf("Unknown property '%s' on attribute '%s'.", $name, static::class) - ); - } - - /** - * Error handler for unknown property mutator in attribute class. - * - * @param mixed $value Property value - */ - public function __set(string $name, mixed $value): never - { - throw new BadMethodCallException( - sprintf("Unknown property '%s' on attribute '%s'.", $name, static::class) - ); - } } diff --git a/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php b/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php index 03ac3dbd..00e966d6 100644 --- a/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php +++ b/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php @@ -98,11 +98,10 @@ protected function isArrayTypedProperty(): bool return $type->getName() === 'array'; } if ($type instanceof ReflectionUnionType) { - foreach ($type->getTypes() as $unionType) { - if ($unionType instanceof ReflectionNamedType && $unionType->getName() === 'array') { - return true; - } - } + return array_any( + $type->getTypes(), + fn($unionType): bool => $unionType instanceof ReflectionNamedType && $unionType->getName() === 'array' + ); } return false; diff --git a/tests/Core/ContainerTest.php b/tests/Core/ContainerTest.php index 3caff0a2..18b1ee81 100644 --- a/tests/Core/ContainerTest.php +++ b/tests/Core/ContainerTest.php @@ -178,7 +178,7 @@ public function testGetServiceEnsuresThatKeyAndReturnedTypeMatches(): void $this->container->getService(First::class); } - public function testLazyServiceIsNotConstructedUntilFirstRetrieval(): void + public function testLazyServiceIsNotConstructedUntilFirstUse(): void { $initialized = false; $container = new Container(); @@ -191,10 +191,17 @@ public function testLazyServiceIsNotConstructedUntilFirstRetrieval(): void $this->assertTrue($container->has(PointcutLexer::class)); $this->assertFalse($initialized, 'Factory should not have been called yet'); - // First retrieval constructs the service exactly once + // Retrieval hands out a typed, instanceof-correct lazy proxy without running the factory $value = $container->getService(PointcutLexer::class); $this->assertInstanceOf(PointcutLexer::class, $value); - $this->assertTrue($initialized, 'Factory should have been called on first retrieval'); + $this->assertFalse($initialized, 'Factory should not run on retrieval'); + $this->assertTrue((new \ReflectionClass(PointcutLexer::class))->isUninitializedLazyObject($value)); + $this->assertSame($value, $container->getService(PointcutLexer::class)); + + // First real interaction with the object runs the factory exactly once + $value->lex('public'); + $this->assertTrue($initialized, 'Factory should have been called on first use'); + $this->assertFalse((new \ReflectionClass(PointcutLexer::class))->isUninitializedLazyObject($value)); $this->assertSame($value, $container->getService(PointcutLexer::class)); } @@ -226,8 +233,13 @@ function () use (&$constructed): LoggingAspect { $this->assertTrue($this->container->has(LoggingAspect::class)); $this->assertFalse($constructed, 'Aspect should not have been constructed at registration'); + // Retrieval returns an instanceof-correct lazy object, construction is still deferred $aspect = $this->container->getService(LoggingAspect::class); $this->assertInstanceOf(LoggingAspect::class, $aspect); + $this->assertFalse($constructed, 'Aspect should not have been constructed by retrieval'); + + // First real interaction with the aspect object triggers the factory + (new \ReflectionClass(LoggingAspect::class))->initializeLazyObject($aspect); $this->assertTrue($constructed); } @@ -240,6 +252,52 @@ public function testLazyAspectAppearsInAspectInterfaceQuery(): void $this->assertInstanceOf(DoSomethingAspect::class, $aspects[DoSomethingAspect::class]); } + public function testLazyAspectEnumerationHandsOutUninitializedLazyObjects(): void + { + $constructed = false; + $this->container->registerAspect( + StatefulTestAspect::class, + function () use (&$constructed): StatefulTestAspect { + $constructed = true; + + return new StatefulTestAspect(42); + } + ); + + $aspects = $this->container->getServicesByInterface(Aspect::class); + $aspect = $aspects[StatefulTestAspect::class]; + + // instanceof is correct before initialization, and the factory has not run yet + $this->assertInstanceOf(Aspect::class, $aspect); + $this->assertInstanceOf(StatefulTestAspect::class, $aspect); + $this->assertTrue((new \ReflectionClass(StatefulTestAspect::class))->isUninitializedLazyObject($aspect)); + $this->assertFalse($constructed, 'Enumeration should not construct the aspect'); + + // A real method call transparently initializes the lazy object and delegates to it + $this->assertSame(42, $aspect->getState()); + $this->assertTrue($constructed, 'First method call should construct the aspect'); + $this->assertFalse((new \ReflectionClass(StatefulTestAspect::class))->isUninitializedLazyObject($aspect)); + } + + public function testPropertylessServiceFallsBackToEagerConstruction(): void + { + // PHP creates lazy objects of property-less classes as already initialized, + // which would silently skip the factory - the container must construct these eagerly + $constructed = false; + $this->container->registerAspect( + DoSomethingAspect::class, + function () use (&$constructed): DoSomethingAspect { + $constructed = true; + + return new DoSomethingAspect(); + } + ); + + $aspect = $this->container->getService(DoSomethingAspect::class); + $this->assertInstanceOf(DoSomethingAspect::class, $aspect); + $this->assertTrue($constructed, 'Factory of a property-less service must run at materialization'); + } + public function testLazyAspectWithRequiredConstructorArgsNeedsFactory(): void { // LoggingAspect requires a LoggerInterface constructor argument @@ -275,6 +333,8 @@ public function testReRegisteringPendingFactoryKeepsTagOrderAndReplacesFactory() $aspects = $this->container->getServicesByInterface(Aspect::class); $this->assertSame([DoSomethingAspect::class, EnumMethodAspect::class], array_keys($aspects)); + // DoSomethingAspect has no instance properties, so it materializes eagerly + // through the re-registered factory (see testPropertylessServiceFallsBackToEagerConstruction) $this->assertTrue($replaced, 'Re-registered factory should have been used'); } @@ -298,3 +358,16 @@ function (AspectContainer $container): DoSomethingAspect { $this->assertArrayHasKey(EnumMethodAspect::class, $aspects); } } + +/** + * Stateful aspect fixture: has an instance property, so PHP can create a true lazy proxy for it + */ +class StatefulTestAspect implements Aspect +{ + public function __construct(private readonly int $state) {} + + public function getState(): int + { + return $this->state; + } +} diff --git a/tests/Functional/ReflectionFilenameTest.php b/tests/Functional/ReflectionFilenameTest.php index c2419e4c..201ffe17 100644 --- a/tests/Functional/ReflectionFilenameTest.php +++ b/tests/Functional/ReflectionFilenameTest.php @@ -46,7 +46,8 @@ public function tearDown(): void parent::tearDown(); $reflectedClass = new \ReflectionClass(FilterInjectorTransformer::class); $reflectedProperty = $reflectedClass->getProperty('kernel'); - $reflectedProperty->setValue(null); + // Static property: the two-argument form (null object) is the non-deprecated way + $reflectedProperty->setValue(null, null); } public function testReflectionFilenameIsCorrect() diff --git a/tests/Instrument/FileSystem/EnumeratorTest.php b/tests/Instrument/FileSystem/EnumeratorTest.php index ec71100b..87382609 100644 --- a/tests/Instrument/FileSystem/EnumeratorTest.php +++ b/tests/Instrument/FileSystem/EnumeratorTest.php @@ -125,4 +125,33 @@ public function testExclude(array $expectedPaths, array $includePaths, array $ex $this->assertEquals($expectedPaths, $testPaths); } + + /** + * Regression test: the include-path check must be a prefix test. + * + * The former `strpos($path, $rootDirectory, 0) === false` was a substring-anywhere + * test, so an include path that merely CONTAINED the root directory somewhere in the + * middle (here: '/somewhere/base/other' contains root '/base') was wrongly accepted + * instead of being rejected as outside the root. + */ + public function testIncludePathMerelyContainingRootDirectoryIsRejected(): void + { + $enumerator = new Enumerator('/base', ['/somewhere/base/other']); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('Path /somewhere/base/other is not in /base'); + $enumerator->enumerate(); + } + + /** + * Sanity check for the fixed prefix test: include paths below the root are accepted + * (no UnexpectedValueException; Finder then fails on the nonexistent directory itself) + */ + public function testIncludePathBelowRootDirectoryPassesTheRootCheck(): void + { + $enumerator = new Enumerator('vfs://base', ['vfs://base/sub']); + + $files = iterator_to_array($enumerator->enumerate()); + $this->assertNotEmpty($files); + } } diff --git a/tests/Instrument/Transformer/StreamMetaDataTest.php b/tests/Instrument/Transformer/StreamMetaDataTest.php new file mode 100644 index 00000000..f924b56a --- /dev/null +++ b/tests/Instrument/Transformer/StreamMetaDataTest.php @@ -0,0 +1,52 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Instrument\Transformer; + +use PHPUnit\Framework\TestCase; + +class StreamMetaDataTest extends TestCase +{ + public function testSourceIsRebuiltFromTokenStream(): void + { + $source = ''; + $metadata = new StreamMetaData(fopen('php://input', 'rb'), $source); + + $this->assertSame($source, $metadata->source); + + // Mutating the token stream is reflected by subsequent reads + foreach ($metadata->tokenStream as $token) { + $token->text = str_replace('hello', 'brave new', $token->text); + } + $this->assertSame('', $metadata->source); + } + + public function testSettingSourceIsDeprecatedButRetokenizes(): void + { + $metadata = new StreamMetaData(fopen('php://input', 'rb'), ''); + + $deprecations = []; + set_error_handler(function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, E_USER_DEPRECATED); + try { + $metadata->source = ''; + } finally { + restore_error_handler(); + } + + $this->assertSame(['Setting StreamMetaData->source is deprecated, use tokenStream instead'], $deprecations); + $this->assertSame('', $metadata->source); + } +} diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index db606afc..b695ff94 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -133,7 +133,8 @@ public function testWeaverForTypeHint(): void $this->assertEquals($expected, $actual); $proxyContent = file_get_contents($this->cachePathManager->getCacheDir() . '/Transformer/_files/class-typehint.php'); - $this->assertFalse(strpos($proxyContent, '\\\\Exception')); + $this->assertNotFalse($proxyContent); + $this->assertStringNotContainsString('\\\\Exception', $proxyContent); } /**