From 865929ac82d64fdc01c10e3a7b30e45f8c11e5fa Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 22:40:42 +0000 Subject: [PATCH 1/3] fix(weaving): write all proxies of a source file into one cache file A source file declaring several woven class-likes wrote every proxy to the same cache path, so the last proxy overwrote the others and all classes but the last one were never declared ("Class not found"). - processSingleClass() converts the class and returns a WovenProxy instead of writing; transform() collects them in a local list (it is re-entered mid-transform) and writeProxies() writes one proxy file and appends ONE include_once. - ProxyFileGenerator: a single proxy stays byte-identical to the previous output; several proxies get `declare(strict_types=1);` once and one braced namespace block per proxy (`namespace {}` for global classes), each with its own imports, a proxy extending another proxy of the file after it. Class/Enum/TraitGenerator::getStmts(), ClassProxyGenerator::generateStmts(), GeneratedCodePrinter option `bracedNamespaces`. - ProxyIncludePlanner: the include goes after the last woven class-like whose woven trait keeps trait uses (declared at runtime) or that extends/implements an unwoven class-like of the file, else after the first one (trait-free traits are declared at compile time). Code before that point that needs a woven class-like (a class-like extending/implementing/using one, a top-level statement loading an earlier one) is rejected with a WeavingException naming the file, the classes and the lines. - The runtime class map already mapped every class of the file to its woven file, CachePathManager is unchanged; no cache format version bump. Transformation performance group, base vs head interleaved, 7 runs each, median ratios: rewriter default rules 0.0958 -> 0.0979 (1.02), all rules 1.3456 -> 1.3401 (1.00), WeavingTransformer 0.2424 -> 0.2319 (0.96). Closes #760 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KXjD75egSqDNujmNS5kr5c --- CHANGELOG.md | 1 + src/Instrument/AGENTS.md | 19 ++ .../Transformer/LoadedClassNameCollector.php | 105 +++++++ .../Transformer/ProxyIncludePlanner.php | 263 ++++++++++++++++ .../Transformer/WeavingTransformer.php | 93 +++--- src/Instrument/Transformer/WovenProxy.php | 42 +++ src/Proxy/AGENTS.md | 4 + src/Proxy/ClassProxyGenerator.php | 25 +- src/Proxy/EnumProxyGenerator.php | 5 + src/Proxy/Generator/ClassGenerator.php | 17 +- src/Proxy/Generator/EnumGenerator.php | 17 +- src/Proxy/Generator/GeneratedCodePrinter.php | 30 ++ src/Proxy/Generator/GeneratorInterface.php | 9 + src/Proxy/Generator/TraitGenerator.php | 17 +- src/Proxy/ProxyFileGenerator.php | 83 +++++ src/Proxy/TraitProxyGenerator.php | 5 + .../src/Application/MultiClassHolder.php | 50 +++ .../project/src/Aspect/MultiClassAspect.php | 23 ++ .../src/Kernel/DefaultAspectKernel.php | 2 + tests/Fixtures/project/web/index.php | 9 + tests/Functional/MultiClassFileTest.php | 83 +++++ .../ProductionMultiClassFileTest.php | 32 ++ .../Transformer/WeavingTransformerTest.php | 287 +++++++++++++++++- .../Transformer/_files/class-proxy.php | 3 +- .../_files/class-static-init-proxy.php | 125 ++++++++ .../_files/multiple-classes-proxy.php | 79 +++++ .../_files/multiple-classes-woven.php | 2 - .../Transformer/_files/multiple-gap.php | 41 +++ .../_files/multiple-mixed-proxy.php | 106 +++++++ .../_files/multiple-mixed-woven.php | 50 +++ .../Transformer/_files/multiple-mixed.php | 49 +++ .../Transformer/_files/multiple-no-gap.php | 38 +++ .../Transformer/_files/multiple-ns-proxy.php | 53 ++++ .../Transformer/_files/multiple-ns-woven.php | 1 - .../_files/multiple-parent-after-child.php | 20 ++ .../_files/php80-attribute-class-woven.php | 2 - .../_files/php80-class-attribute-woven.php | 1 - .../Transformer/_files/php81-enum-proxy.php | 3 +- 38 files changed, 1738 insertions(+), 56 deletions(-) create mode 100644 src/Instrument/Transformer/LoadedClassNameCollector.php create mode 100644 src/Instrument/Transformer/ProxyIncludePlanner.php create mode 100644 src/Instrument/Transformer/WovenProxy.php create mode 100644 src/Proxy/ProxyFileGenerator.php create mode 100644 tests/Fixtures/project/src/Application/MultiClassHolder.php create mode 100644 tests/Fixtures/project/src/Aspect/MultiClassAspect.php create mode 100644 tests/Functional/MultiClassFileTest.php create mode 100644 tests/Functional/ProductionMultiClassFileTest.php create mode 100644 tests/Instrument/Transformer/_files/class-static-init-proxy.php create mode 100644 tests/Instrument/Transformer/_files/multiple-classes-proxy.php create mode 100644 tests/Instrument/Transformer/_files/multiple-gap.php create mode 100644 tests/Instrument/Transformer/_files/multiple-mixed-proxy.php create mode 100644 tests/Instrument/Transformer/_files/multiple-mixed-woven.php create mode 100644 tests/Instrument/Transformer/_files/multiple-mixed.php create mode 100644 tests/Instrument/Transformer/_files/multiple-no-gap.php create mode 100644 tests/Instrument/Transformer/_files/multiple-ns-proxy.php create mode 100644 tests/Instrument/Transformer/_files/multiple-parent-after-child.php diff --git a/CHANGELOG.md b/CHANGELOG.md index c8bdff5b2..22d4bf546 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,7 @@ See [UPGRADE-4.0.md](UPGRADE-4.0.md) for the migration guide. * [BC BREAK] Interceptors are no longer `serialize()`-able: `AbstractInterceptor::__serialize()`/`__unserialize()` and the protected `serializeAdvice()`/`unserializeAdvice()` hooks are removed; advisor caches restore advices via first-class callables on aspect instances instead. **Fixed** +* [Fixed] A file declaring several woven classes writes all their proxies into its one proxy file (a braced namespace block per proxy) and includes it once; the proxies used to overwrite each other, so all classes but the last were missing. Code running before that include that needs one of the woven classes is rejected with a `WeavingException` (#760). * [Fixed] Generated proxies keep readable short `use` imports, and imports that collide with names of the original file (its imports, the class itself, names used in its body) or with each other (aspects sharing a short name) get a distinct alias automatically, e.g. `use Go\Aop\Framework\Interceptor as AopInterceptor;`. Such collisions used to make the proxy fail to compile (#668). * [Fixed] Every cache file (woven sources, class and function proxies, metadata maps, advisor caches) is written atomically through `CacheFileWriter` (temporary file + rename, opcache invalidation), so a concurrent request can no longer include a half-written proxy (#670). Cache directories get search permissions matching the read bits of `cacheFileMode`, and an invalid `cacheFileMode` (outside `0600..0777` or without owner read/write) is rejected by the kernel. * [Fixed] Intercepted property declarations spanning several lines (multi-line defaults, attributes on their own lines) are removed from the woven trait completely, and grouped declarations such as `public int $a, $b;` keep their non-intercepted items; line numbers are preserved (#669). diff --git a/src/Instrument/AGENTS.md b/src/Instrument/AGENTS.md index 79d4e6d92..f1fe22995 100644 --- a/src/Instrument/AGENTS.md +++ b/src/Instrument/AGENTS.md @@ -59,6 +59,25 @@ Short names above are `use` aliases managed by ProxyImports: a name colliding wi file's imports or body gets an adjusted alias (e.g. `use Go\Aop\Framework\Interceptor as AopInterceptor;`), see src/Proxy/AGENTS.md. WeavingTransformer passes the original imports to the proxy generator constructors. +### Several woven class-likes in one file (#760) +One proxy file per source file, never one per class: processSingleClass() only converts the class and returns a +WovenProxy; transform() collects them in a local list (transform() is re-entered mid-transform) and writeProxies() +writes them once (ProxyFileGenerator) and appends ONE include_once. +- One proxy: byte-identical to the single-class output (` + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Instrument\Transformer; + +use PhpParser\Node; +use PhpParser\Node\Expr; +use PhpParser\Node\Identifier; +use PhpParser\Node\Name; +use PhpParser\Node\Stmt; +use PhpParser\NodeVisitor; +use PhpParser\NodeVisitorAbstract; + +/** + * Collects the names of the classes code loads when it runs: instantiations, static members, class constants (not + * `X::class`) and the parents, interfaces and traits of the classes it declares. Bodies of functions, closures, arrow + * functions and declared classes run later and are skipped; `instanceof`, type declarations and other references + * that do not load a class are ignored. + * + * @internal + */ +final class LoadedClassNameCollector extends NodeVisitorAbstract +{ + /** + * Fully qualified names of the loaded classes + * + * @var list + */ + public array $names = []; + + public function enterNode(Node $node): ?int + { + if ($node instanceof Stmt\Function_ || $node instanceof Expr\Closure || $node instanceof Expr\ArrowFunction) { + return NodeVisitor::DONT_TRAVERSE_CHILDREN; + } + if ($node instanceof Stmt\ClassLike) { + array_push($this->names, ...self::getInheritedNames($node), ...self::getTraitUseNames($node)); + + return NodeVisitor::DONT_TRAVERSE_CHILDREN; + } + $isLoadingReference = $node instanceof Expr\New_ + || $node instanceof Expr\StaticCall + || $node instanceof Expr\StaticPropertyFetch + || ($node instanceof Expr\ClassConstFetch + && !($node->name instanceof Identifier && $node->name->toLowerString() === 'class')); + if ($isLoadingReference && $node->class instanceof Name) { + $this->names[] = self::resolveName($node->class); + } + + return null; + } + + /** + * Names of the class-likes the declaration extends or implements + * + * @return list + */ + public static function getInheritedNames(Stmt\ClassLike $node): array + { + $names = match (true) { + $node instanceof Stmt\Class_ => $node->extends !== null ? [$node->extends, ...$node->implements] : $node->implements, + $node instanceof Stmt\Enum_ => $node->implements, + $node instanceof Stmt\Interface_ => $node->extends, + default => [], + }; + + return array_values(array_map(self::resolveName(...), $names)); + } + + /** + * Names of the traits used by the declaration + * + * @return list + */ + public static function getTraitUseNames(Stmt\ClassLike $node): array + { + $names = []; + foreach ($node->getTraitUses() as $traitUse) { + foreach ($traitUse->traits as $trait) { + $names[] = self::resolveName($trait); + } + } + + return $names; + } + + /** + * Fully qualified name of a class reference, resolved by the name resolver of the parser + */ + public static function resolveName(Name $name): string + { + $resolvedName = $name->getAttribute('resolvedName'); + + return $resolvedName instanceof Name ? $resolvedName->toString() : $name->toString(); + } +} diff --git a/src/Instrument/Transformer/ProxyIncludePlanner.php b/src/Instrument/Transformer/ProxyIncludePlanner.php new file mode 100644 index 000000000..10e6b1edd --- /dev/null +++ b/src/Instrument/Transformer/ProxyIncludePlanner.php @@ -0,0 +1,263 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Instrument\Transformer; + +use Go\Aop\Exception\WeavingException; +use PhpParser\Node; +use PhpParser\Node\Stmt; +use PhpParser\NodeTraverser; + +/** + * Places the proxies of a source file declaring several woven class-likes: the order of the proxies in the proxy file + * and the declaration after which the woven source includes that file. + * + * A proxy always uses its `…OriginalTrait`, so PHP declares it at runtime, when the include runs. A woven trait without + * trait uses is declared at compile time, before any code of the file runs; one with trait uses is declared at runtime, + * at its position. The include goes after the last woven class-like whose proxy depends on such a position (its trait + * uses something, or it extends/implements a class-like of the file that is not woven), else after the first one. + * + * Code that runs between the first woven class-like and that point can not use the woven class-likes before it, as it + * could with one proxy per class: the file is rejected with a WeavingException instead of failing with "Class not found" + * while it is loaded. Detected: + * - a class-like declared up to the include point that needs a woven class-like of the file: an unwoven one extending, + * implementing or using it, a woven one using it (its trait uses stay in its `…OriginalTrait`); + * - a top-level statement between the first woven class-like and the include point that loads a woven class-like + * declared before it: `new X`, `X::method()`, `X::$property`, `X::CONSTANT` (not `X::class`) or a class declared + * by it extending/implementing/using X. Bodies of closures, arrow functions and functions are not looked into, + * `instanceof`, type declarations and other references that do not load a class are ignored. + * + * @internal + */ +final class ProxyIncludePlanner +{ + /** + * Sorts the proxies for the proxy file: source order, a proxy extending another proxy of the file comes after it + * + * @param non-empty-list $proxies Proxies in source order + * + * @return non-empty-list + */ + public static function sortForProxyFile(array $proxies): array + { + $byName = []; + foreach ($proxies as $proxy) { + $byName[strtolower($proxy->class->getName())] = $proxy; + } + $placed = []; + $sorted = []; + foreach ($proxies as $proxy) { + self::place($proxy, $byName, $placed, $sorted); + } + assert($sorted !== []); + + return $sorted; + } + + /** + * Returns the proxy after whose declaration the proxy file is included + * + * @param array $syntaxTree Syntax tree of the source file + * @param non-empty-list $proxies Proxies in source order + * @param string $fileName Source file name, for the error message + * + * @throws WeavingException When code running before the include point needs a woven class-like + */ + public static function findIncludePoint(array $syntaxTree, array $proxies, string $fileName): WovenProxy + { + $includePoint = $proxies[0]; + if (count($proxies) === 1) { + return $includePoint; + } + + $statements = self::getTopLevelStatements($syntaxTree); + $wovenNames = []; + $wovenNodes = []; + foreach ($proxies as $proxy) { + $wovenNames[strtolower($proxy->class->getName())] = $proxy; + $wovenNodes[spl_object_id($proxy->node)] = true; + } + $unwovenNames = []; + foreach ($statements as $statement) { + if ($statement instanceof Stmt\ClassLike && $statement->namespacedName !== null) { + $name = strtolower($statement->namespacedName->toString()); + if (!isset($wovenNames[$name])) { + $unwovenNames[$name] = true; + } + } + } + foreach ($proxies as $proxy) { + if (self::isPositionBound($proxy->node, $unwovenNames)) { + $includePoint = $proxy; + } + } + + $problems = self::findLoadTimeDependencies($statements, $proxies, $wovenNames, $wovenNodes, $includePoint); + if ($problems !== []) { + $wovenClassNames = array_map(static fn(WovenProxy $proxy): string => $proxy->class->getName(), $proxies); + + throw new WeavingException(sprintf( + 'Can not weave %s: the proxies of its woven class-likes %s are declared by one proxy file, included ' + . 'after %s (line %d), the first point where all of them can be declared. Code running before that ' + . 'point needs a woven class-like: %s. Move that code or the woven class-likes so that the code runs ' + . 'after %s, split the file into one class per file, or exclude the class-likes from the pointcut.', + $fileName, + implode(', ', $wovenClassNames), + $includePoint->class->getName(), + $includePoint->node->getEndLine(), + implode('; ', $problems), + $includePoint->class->getName(), + )); + } + + return $includePoint; + } + + /** + * @param list $statements Top-level statements of the file + * @param non-empty-list $proxies Proxies in source order + * @param array $wovenNames Woven class-likes by lowercase name + * @param array $wovenNodes Object ids of the woven declarations + * + * @return list Descriptions of the code needing a woven class-like before the include point + */ + private static function findLoadTimeDependencies( + array $statements, + array $proxies, + array $wovenNames, + array $wovenNodes, + WovenProxy $includePoint, + ): array { + $problems = []; + $firstWovenEnd = $proxies[0]->endTokenPosition; + foreach ($statements as $statement) { + if ($statement->getEndTokenPos() > $includePoint->endTokenPosition) { + continue; + } + if ($statement instanceof Stmt\ClassLike) { + // Extends/implements of a woven class-like move to its proxy, its trait uses stay in the woven trait + $neededNames = LoadedClassNameCollector::getTraitUseNames($statement); + if (!isset($wovenNodes[spl_object_id($statement)])) { + $neededNames = [...LoadedClassNameCollector::getInheritedNames($statement), ...$neededNames]; + } + foreach ($neededNames as $neededName) { + if (isset($wovenNames[strtolower($neededName)])) { + $problems[] = sprintf( + '%s (line %d) needs %s', + $statement->namespacedName?->toString() ?? 'class', + $statement->getStartLine(), + $wovenNames[strtolower($neededName)]->class->getName(), + ); + } + } + continue; + } + $statementStart = $statement->getStartTokenPos(); + if ($statementStart <= $firstWovenEnd || self::isDeclarationOnly($statement)) { + continue; + } + $collector = new LoadedClassNameCollector(); + $traverser = new NodeTraverser($collector); + $traverser->traverse([$statement]); + foreach ($collector->names as $reference) { + $woven = $wovenNames[strtolower($reference)] ?? null; + if ($woven !== null && $woven->endTokenPosition < $statementStart) { + $problems[] = sprintf( + 'the statement on line %d uses %s', + $statement->getStartLine(), + $woven->class->getName(), + ); + } + } + } + + return array_values(array_unique($problems)); + } + + /** + * A proxy depends on the position of its declaration when its woven trait keeps trait uses (PHP declares such a + * trait at runtime) or when it extends/implements a class-like of the file that is not woven + * + * @param array $unwovenNames Lowercase names of the class-likes of the file that are not woven + */ + private static function isPositionBound(Stmt\ClassLike $node, array $unwovenNames): bool + { + if ($node->getTraitUses() !== []) { + return true; + } + foreach (LoadedClassNameCollector::getInheritedNames($node) as $inheritedName) { + if (isset($unwovenNames[strtolower($inheritedName)])) { + return true; + } + } + + return false; + } + + /** + * @param array $byName Proxies by lowercase class name + * @param array $placed Lowercase names of the visited proxies + * @param list $sorted + */ + private static function place(WovenProxy $proxy, array $byName, array &$placed, array &$sorted): void + { + $key = strtolower($proxy->class->getName()); + if (isset($placed[$key])) { + return; + } + $placed[$key] = true; + if ($proxy->node instanceof Stmt\Class_ && $proxy->node->extends !== null) { + $parent = $byName[strtolower(LoadedClassNameCollector::resolveName($proxy->node->extends))] ?? null; + if ($parent !== null) { + self::place($parent, $byName, $placed, $sorted); + } + } + $sorted[] = $proxy; + } + + /** + * Statements directly in the file or in its namespace blocks + * + * @param array $syntaxTree + * + * @return list + */ + private static function getTopLevelStatements(array $syntaxTree): array + { + $statements = []; + foreach ($syntaxTree as $node) { + if ($node instanceof Stmt\Namespace_) { + array_push($statements, ...$node->stmts); + } elseif ($node instanceof Stmt\Declare_) { + array_push($statements, ...($node->stmts ?? [])); + } elseif ($node instanceof Stmt) { + $statements[] = $node; + } + } + + return $statements; + } + + /** + * Statements that run no code while the file is loaded + */ + private static function isDeclarationOnly(Stmt $statement): bool + { + return $statement instanceof Stmt\Function_ + || $statement instanceof Stmt\Use_ + || $statement instanceof Stmt\GroupUse + || $statement instanceof Stmt\Nop + || $statement instanceof Stmt\InlineHTML + || $statement instanceof Stmt\HaltCompiler + || $statement instanceof Stmt\Declare_; + } +} diff --git a/src/Instrument/Transformer/WeavingTransformer.php b/src/Instrument/Transformer/WeavingTransformer.php index 20f025fa3..a7d8a9c7d 100644 --- a/src/Instrument/Transformer/WeavingTransformer.php +++ b/src/Instrument/Transformer/WeavingTransformer.php @@ -29,6 +29,7 @@ use Go\Proxy\ClassProxyGenerator; use Go\Proxy\EnumProxyGenerator; use Go\Proxy\FunctionProxyGenerator; +use Go\Proxy\ProxyFileGenerator; use Go\Proxy\TraitProxyGenerator; use Closure; use PhpParser\Node\Attribute; @@ -92,6 +93,9 @@ public function transform(StreamMetaData $metadata): TransformerResult $advisors = $this->container->getServicesByInterface(Advisor::class); $namespaces = $parsedSource->getFileNamespaces(); + // Proxies of the woven class-likes of this file, written together once the whole file is woven. A local + // list, not a property: weaving loads other files, which re-enters transform() for them mid-transform + $wovenProxies = []; foreach ($namespaces as $namespace) { $classes = $namespace->getClasses(); @@ -105,47 +109,44 @@ public function transform(StreamMetaData $metadata): TransformerResult if ($class->isInterface()) { continue; } - $wasClassProcessed = $this->processSingleClass( - $advisors, - $metadata, - $class, - $namespace, - $parsedSource->isStrictMode(), - ); - $totalTransformations += (int) $wasClassProcessed; + $wovenProxy = $this->processSingleClass($advisors, $metadata, $class, $namespace); + if ($wovenProxy !== null) { + $wovenProxies[] = $wovenProxy; + ++$totalTransformations; + } } $wasFunctionsProcessed = $this->processFunctions($advisors, $metadata, $namespace); $totalTransformations += (int) $wasFunctionsProcessed; } + if ($wovenProxies !== []) { + $this->writeProxies($metadata, $parsedSource, $wovenProxies); + } + $result = ($totalTransformations > 0) ? TransformerResult::Transformed : TransformerResult::Abstain; return $result; } /** - * Performs weaving of single class if needed, returns true if the class was processed - * - * @param Advisor[] $advisors List of advisors - * @param StreamMetaData $metadata - * @param ReflectionClass $class - * @param ReflectionFileNamespace $namespace Namespace block of the file that declares the class - * @param bool $useStrictMode If the source file used strict mode, the proxy should too - * @return bool + * Performs weaving of single class if needed: converts it to its trait in the woven source and returns its proxy, + * written to the proxy file of the source by {@see writeProxies()}. Returns null if the class was not woven. + * + * @param Advisor[] $advisors List of advisors + * @param ReflectionFileNamespace $namespace Namespace block of the file that declares the class */ private function processSingleClass( array $advisors, StreamMetaData $metadata, ReflectionClass $class, ReflectionFileNamespace $namespace, - bool $useStrictMode, - ): bool { + ): ?WovenProxy { try { $advices = $this->adviceMatcher->getAdvicesForClass($class, $advisors); } catch (Throwable $matchingError) { // Aspects used to be skipped before matching, so matching one must not fail the file if ($this->isAspectSafe($class)) { - return false; + return null; } throw $matchingError; } @@ -153,12 +154,18 @@ private function processSingleClass( if (empty($advices)) { // Fast return if there aren't any advices for that class. It comes before the aspect check, // as that one reflects (and parses) every ancestor and fails when one can't be located - return false; + return null; } // Aspects are never woven if ($class->implementsInterface(Aspect::class)) { - return false; + return null; + } + + // The include of the proxy file goes after the last token of a woven class + $lastClassToken = $class->getNode()->getAttribute('endTokenPos'); + if (!is_int($lastClassToken)) { + return null; } // Sort advices in advance to keep the correct order in cache, and leave only keys for the cache @@ -191,24 +198,33 @@ private function processSingleClass( $childProxyGenerator = new ClassProxyGenerator($class, $newFqcn, $advices, $originalImports, $methods); } - $childCode = $childProxyGenerator->generate(); - - if ($useStrictMode) { - $childCode = 'declare(strict_types=1);' . PHP_EOL . $childCode; - } + return new WovenProxy($class, $childProxyGenerator, $lastClassToken); + } - $contentToInclude = $this->saveProxyToCache($class, $childCode); + /** + * Writes the proxies of all woven class-likes of the file to its proxy file and includes that file once + * + * A single proxy is included right after its class. With several proxies the include goes after the last + * class-like whose proxy depends on its position, see {@see ProxyIncludePlanner}. + * + * @param non-empty-list $wovenProxies + */ + private function writeProxies(StreamMetaData $metadata, ReflectionFile $parsedSource, array $wovenProxies): void + { + // Namespaces are visited by name, a namespace declared in several blocks would break the source order + usort( + $wovenProxies, + static fn(WovenProxy $left, WovenProxy $right): int => $left->endTokenPosition <=> $right->endTokenPosition, + ); + $includePoint = ProxyIncludePlanner::findIncludePoint($metadata->syntaxTree, $wovenProxies, $metadata->uri); - // Get last token for this class - $classNode = $class->getNode(); - $lastClassToken = $classNode->getAttribute('endTokenPos'); - if (!is_int($lastClassToken)) { - return false; + $proxyFile = new ProxyFileGenerator($parsedSource->isStrictMode()); + foreach (ProxyIncludePlanner::sortForProxyFile($wovenProxies) as $wovenProxy) { + $proxyFile->addProxy($wovenProxy->generator, $wovenProxy->class->getNamespaceName()); } + $contentToInclude = $this->saveProxyToCache($includePoint->class, $proxyFile); - $metadata->tokenStream[$lastClassToken]->text .= PHP_EOL . $contentToInclude; - - return true; + $metadata->tokenStream[$includePoint->endTokenPosition]->text .= PHP_EOL . $contentToInclude; } /** @@ -969,9 +985,10 @@ private function processFunctions( } /** - * Save AOP proxy to the separate file anr returns the php source code for inclusion + * Saves the proxies of the source file declaring the class to its proxy file and returns the php source code + * for inclusion */ - private function saveProxyToCache(ReflectionClass $class, string $childCode): string + private function saveProxyToCache(ReflectionClass $class, ProxyFileGenerator $proxyFile): string { $cacheRootDir = $this->cachePathManager->getCacheDir(); if ($cacheRootDir === null) { @@ -987,7 +1004,7 @@ private function saveProxyToCache(ReflectionClass $class, string $childCode): st $proxyFileName = $cacheRootDir . '/' . $proxyRelativePath; // Atomic write: a concurrent request including the proxy never sees a partial file - $this->cachePathManager->getCacheFileWriter()->write($proxyFileName, 'cachePathManager->getCacheFileWriter()->write($proxyFileName, $proxyFile->generate()); return 'include_once AOP_CACHE_DIR . ' . var_export('/' . $proxyRelativePath, true) . ';'; } diff --git a/src/Instrument/Transformer/WovenProxy.php b/src/Instrument/Transformer/WovenProxy.php new file mode 100644 index 000000000..5f16430c4 --- /dev/null +++ b/src/Instrument/Transformer/WovenProxy.php @@ -0,0 +1,42 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Instrument\Transformer; + +use Go\ParserReflection\ReflectionClass; +use Go\Proxy\ClassProxyGenerator; +use PhpParser\Node\Stmt\ClassLike; + +/** + * Proxy of a class-like woven in the current file. The proxies of all class-likes of a file are written to one proxy + * file once the whole file was woven. + * + * @internal + */ +final readonly class WovenProxy +{ + /** + * Declaration of the woven class-like, converted to its `…OriginalTrait` in the woven source + */ + public ClassLike $node; + + /** + * @param int $endTokenPosition Position of the last token of the declaration + */ + public function __construct( + public ReflectionClass $class, + public ClassProxyGenerator $generator, + public int $endTokenPosition, + ) { + $this->node = $class->getNode(); + } +} diff --git a/src/Proxy/AGENTS.md b/src/Proxy/AGENTS.md index 72ca83762..a9f7f781c 100644 --- a/src/Proxy/AGENTS.md +++ b/src/Proxy/AGENTS.md @@ -7,6 +7,10 @@ - FunctionProxyGenerator — function wrappers - TraitProxyGenerator — trait proxies - EnumProxyGenerator — enum proxies (trait extraction + case re-declaration) +- ProxyFileGenerator (@internal) — the proxy file of one source file: one proxy → `generate()` output unchanged; + several → one braced namespace block per proxy from `generateStmts()` (Class/Enum/TraitGenerator::getStmts() = uses + + declaration, static initialization as an Expression node), printed by GeneratedCodePrinter with + `bracedNamespaces` (see src/Instrument/AGENTS.md, #760) ## Imports in generated code (issue #668) - Generated code stays clean and readable: framework and aspect classes are referenced through short `use` aliases diff --git a/src/Proxy/ClassProxyGenerator.php b/src/Proxy/ClassProxyGenerator.php index 09ba7b02d..e89866ca2 100644 --- a/src/Proxy/ClassProxyGenerator.php +++ b/src/Proxy/ClassProxyGenerator.php @@ -266,15 +266,36 @@ public function addUse(string $use, ?string $useAlias = null): void public function generate(): string { $classCode = $this->generator->generate(); - $staticInitializationAdvices = $this->adviceNames[AspectContainer::STATIC_INIT_PREFIX]['root'] ?? []; - if ($staticInitializationAdvices !== []) { + if ($this->hasStaticInitialization()) { $classCode .= "\n" . $this->generator->name . '::__staticInitialization();'; } return $classCode; } + /** + * Generates the statements of the proxy below its namespace declaration: use statements, the proxy and the + * static initialization call. Used to combine the proxies of several classes declared in one file. + * + * @return list + */ + public function generateStmts(): array + { + $stmts = $this->generator->getStmts(); + + if ($this->hasStaticInitialization()) { + $stmts[] = new Expression(new StaticCall(new Name($this->generator->name), '__staticInitialization')); + } + + return $stmts; + } + + private function hasStaticInitialization(): bool + { + return ($this->adviceNames[AspectContainer::STATIC_INIT_PREFIX]['root'] ?? []) !== []; + } + /** * Returns list of intercepted method generators for class by method names * diff --git a/src/Proxy/EnumProxyGenerator.php b/src/Proxy/EnumProxyGenerator.php index 1fcb3c925..6515b49a2 100644 --- a/src/Proxy/EnumProxyGenerator.php +++ b/src/Proxy/EnumProxyGenerator.php @@ -199,6 +199,11 @@ public function generate(): string return $this->generator->generate(); } + public function generateStmts(): array + { + return $this->generator->getStmts(); + } + /** * Every intercepted method keeps its joinpoint in its own `static $__joinPoint` variable, like in * a trait proxy: all intercepted enum methods have `OriginalAlias` aliases from the enum's diff --git a/src/Proxy/Generator/ClassGenerator.php b/src/Proxy/Generator/ClassGenerator.php index 7a2e8aee4..cc6a17899 100644 --- a/src/Proxy/Generator/ClassGenerator.php +++ b/src/Proxy/Generator/ClassGenerator.php @@ -17,6 +17,7 @@ use PhpParser\Node\AttributeGroup; use PhpParser\Node\Identifier; use PhpParser\Node\Name; +use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Class_ as ClassNode; use PhpParser\Node\Stmt\TraitUse; use PhpParser\Node\Stmt\TraitUseAdaptation; @@ -219,6 +220,19 @@ public function generate(): string $stmts[] = self::getFactory()->namespace($this->namespace)->getNode(); } + return self::getPrinter()->prettyPrint([...$stmts, ...$this->getStmts()]); + } + + /** + * Returns the statements of the generated file below its namespace declaration: use statements and the class. + * + * Several generated declarations can be combined into one file by wrapping each one into its own namespace block. + * + * @return list + */ + public function getStmts(): array + { + $stmts = []; foreach ($this->uses as $use => $alias) { $useBuilder = self::getFactory()->use($use); if ($alias !== null) { @@ -226,10 +240,9 @@ public function generate(): string } $stmts[] = $useBuilder->getNode(); } - $stmts[] = $this->getNode(); - return self::getPrinter()->prettyPrint($stmts); + return $stmts; } private static function getPrinter(): Standard diff --git a/src/Proxy/Generator/EnumGenerator.php b/src/Proxy/Generator/EnumGenerator.php index 0aa42743f..322b676e0 100644 --- a/src/Proxy/Generator/EnumGenerator.php +++ b/src/Proxy/Generator/EnumGenerator.php @@ -16,6 +16,7 @@ use PhpParser\Node\Expr; use PhpParser\Node\Identifier; use PhpParser\Node\Name; +use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Enum_ as EnumNode; use PhpParser\Node\Stmt\TraitUse; use PhpParser\Node\Stmt\TraitUseAdaptation; @@ -214,6 +215,19 @@ public function generate(): string $stmts[] = self::getFactory()->namespace($this->namespace)->getNode(); } + return self::getPrinter()->prettyPrint([...$stmts, ...$this->getStmts()]); + } + + /** + * Returns the statements of the generated file below its namespace declaration: use statements and the enum. + * + * Several generated declarations can be combined into one file by wrapping each one into its own namespace block. + * + * @return list + */ + public function getStmts(): array + { + $stmts = []; foreach ($this->uses as $use => $alias) { $useBuilder = self::getFactory()->use($use); if ($alias !== null) { @@ -221,10 +235,9 @@ public function generate(): string } $stmts[] = $useBuilder->getNode(); } - $stmts[] = $this->getNode(); - return self::getPrinter()->prettyPrint($stmts); + return $stmts; } private static function getPrinter(): Standard diff --git a/src/Proxy/Generator/GeneratedCodePrinter.php b/src/Proxy/Generator/GeneratedCodePrinter.php index 118bbd533..af77a26f2 100644 --- a/src/Proxy/Generator/GeneratedCodePrinter.php +++ b/src/Proxy/Generator/GeneratedCodePrinter.php @@ -15,6 +15,7 @@ use PhpParser\Node\Expr; use PhpParser\Node\Identifier; use PhpParser\Node\Name; +use PhpParser\PhpVersion; use PhpParser\PrettyPrinter\Standard; /** @@ -37,6 +38,35 @@ class GeneratedCodePrinter extends Standard */ private const array INTERCEPTOR_METHODS = ['before', 'after', 'around', 'afterThrowing']; + /** + * Whether namespaces are always printed as braced blocks (`namespace X { ... }`) + */ + private readonly bool $bracedNamespaces; + + /** + * Accepts the options of the standard printer plus `bracedNamespaces`: print every namespace as a braced + * block, so a file combining several namespace blocks stays valid when one of them is the global namespace + * + * @param array{ + * phpVersion?: PhpVersion, newline?: string, indent?: string, shortArraySyntax?: bool, + * bracedNamespaces?: bool + * } $options + */ + public function __construct(array $options = []) + { + $this->bracedNamespaces = $options['bracedNamespaces'] ?? false; + unset($options['bracedNamespaces']); + parent::__construct($options); + } + + protected function preprocessNodes(array $nodes): void + { + parent::preprocessNodes($nodes); + if ($this->bracedNamespaces) { + $this->canUseSemicolonNamespaces = false; + } + } + protected function pExpr_Array(Expr\Array_ $node): string { if (empty($node->items)) { diff --git a/src/Proxy/Generator/GeneratorInterface.php b/src/Proxy/Generator/GeneratorInterface.php index 69298a307..f10ec7dcd 100644 --- a/src/Proxy/Generator/GeneratorInterface.php +++ b/src/Proxy/Generator/GeneratorInterface.php @@ -12,6 +12,8 @@ namespace Go\Proxy\Generator; +use PhpParser\Node\Stmt; + /** * Common contract for all top-level code generators (class, trait, file, etc.). */ @@ -26,4 +28,11 @@ interface GeneratorInterface * Returns the generated PHP source code as a string. */ public function generate(): string; + + /** + * Returns the generated statements below the namespace declaration (use statements and the declaration). + * + * @return list + */ + public function getStmts(): array; } diff --git a/src/Proxy/Generator/TraitGenerator.php b/src/Proxy/Generator/TraitGenerator.php index 74c3454b3..463cb9703 100644 --- a/src/Proxy/Generator/TraitGenerator.php +++ b/src/Proxy/Generator/TraitGenerator.php @@ -15,6 +15,7 @@ use PhpParser\BuilderFactory; use PhpParser\Comment\Doc; use PhpParser\Node\Name; +use PhpParser\Node\Stmt; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Stmt\Property as PropertyNode; use PhpParser\Node\Stmt\Trait_ as TraitNode; @@ -158,6 +159,19 @@ public function generate(): string $stmts[] = self::getFactory()->namespace($this->namespace)->getNode(); } + return self::getPrinter()->prettyPrint([...$stmts, ...$this->getStmts()]); + } + + /** + * Returns the statements of the generated file below its namespace declaration: use statements and the trait. + * + * Several generated declarations can be combined into one file by wrapping each one into its own namespace block. + * + * @return list + */ + public function getStmts(): array + { + $stmts = []; foreach ($this->uses as $use => $alias) { $useBuilder = self::getFactory()->use($use); if ($alias !== null) { @@ -165,10 +179,9 @@ public function generate(): string } $stmts[] = $useBuilder->getNode(); } - $stmts[] = $this->getNode(); - return self::getPrinter()->prettyPrint($stmts); + return $stmts; } private static function getPrinter(): Standard diff --git a/src/Proxy/ProxyFileGenerator.php b/src/Proxy/ProxyFileGenerator.php new file mode 100644 index 000000000..7cad948b3 --- /dev/null +++ b/src/Proxy/ProxyFileGenerator.php @@ -0,0 +1,83 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Proxy; + +use Go\Proxy\Generator\GeneratedCodePrinter; +use PhpParser\Node\Name; +use PhpParser\Node\Stmt\Namespace_; +use PhpParser\PrettyPrinter\Standard; + +/** + * Generates the cache file holding the proxies of all woven class-likes declared in one source file. + * + * A single proxy is emitted exactly like before (`namespace X;` + imports + declaration). Several proxies are emitted + * as one braced namespace block per proxy (`namespace X { ... }`, `namespace { ... }` for the global namespace): + * every proxy keeps its own imports, which could collide when merged into one block of the same namespace. + * + * @internal + */ +final class ProxyFileGenerator +{ + private static ?Standard $printer = null; + + /** + * Proxy generators with the namespace of their class, in the order they are emitted + * + * @var list + */ + private array $proxies = []; + + /** + * @param bool $useStrictMode If the source file used strict mode, the proxies should too + */ + public function __construct(private readonly bool $useStrictMode) {} + + /** + * Adds a proxy to the file, proxies are emitted in the order they are added + * + * @param string $namespaceName Namespace of the proxied class, an empty string for the global namespace + */ + public function addProxy(ClassProxyGenerator $generator, string $namespaceName): void + { + $this->proxies[] = [$generator, $namespaceName]; + } + + /** + * Generates the full PHP source of the proxy file + */ + public function generate(): string + { + $code = 'useStrictMode) { + $code .= 'declare(strict_types=1);' . PHP_EOL; + } + + if (count($this->proxies) === 1) { + return $code . $this->proxies[0][0]->generate(); + } + + $blocks = []; + foreach ($this->proxies as [$generator, $namespaceName]) { + $block = new Namespace_($namespaceName !== '' ? new Name($namespaceName) : null, $generator->generateStmts()); + $blocks[] = self::getPrinter()->prettyPrint([$block]); + } + + // Blocks are separated by a blank line to keep the file readable + return $code . implode("\n\n", $blocks); + } + + private static function getPrinter(): Standard + { + return self::$printer ??= new GeneratedCodePrinter(['shortArraySyntax' => true, 'bracedNamespaces' => true]); + } +} diff --git a/src/Proxy/TraitProxyGenerator.php b/src/Proxy/TraitProxyGenerator.php index bade10538..1453aa253 100644 --- a/src/Proxy/TraitProxyGenerator.php +++ b/src/Proxy/TraitProxyGenerator.php @@ -135,4 +135,9 @@ public function generate(): string { return $this->generator->generate(); } + + public function generateStmts(): array + { + return $this->generator->getStmts(); + } } diff --git a/tests/Fixtures/project/src/Application/MultiClassHolder.php b/tests/Fixtures/project/src/Application/MultiClassHolder.php new file mode 100644 index 000000000..f6f93c906 --- /dev/null +++ b/tests/Fixtures/project/src/Application/MultiClassHolder.php @@ -0,0 +1,50 @@ +greet(); + } + } +} + +namespace Go\Tests\TestProject\Application\Sub { + /** + * Woven class of another namespace block of the file + */ + class MultiClassThird + { + public function hello(): string + { + return 'third'; + } + } +} diff --git a/tests/Fixtures/project/src/Aspect/MultiClassAspect.php b/tests/Fixtures/project/src/Aspect/MultiClassAspect.php new file mode 100644 index 000000000..40659ff1d --- /dev/null +++ b/tests/Fixtures/project/src/Aspect/MultiClassAspect.php @@ -0,0 +1,23 @@ +hello(*))" + . " || execution(public Go\Tests\TestProject\Application\Sub\MultiClass*->hello(*))", + )] + public function beforeHello(MethodInvocation $invocation): void + { + echo 'advised:'; + } +} diff --git a/tests/Fixtures/project/src/Kernel/DefaultAspectKernel.php b/tests/Fixtures/project/src/Kernel/DefaultAspectKernel.php index c83ac06f4..2f85244dd 100644 --- a/tests/Fixtures/project/src/Kernel/DefaultAspectKernel.php +++ b/tests/Fixtures/project/src/Kernel/DefaultAspectKernel.php @@ -15,6 +15,7 @@ use Go\Tests\TestProject\Aspect\InitializationAspect; use Go\Tests\TestProject\Aspect\Issue293Aspect; use Go\Tests\TestProject\Aspect\LoggingAspect; +use Go\Tests\TestProject\Aspect\MultiClassAspect; use Go\Tests\TestProject\Aspect\NullablePropertyInterceptAspect; use Go\Tests\TestProject\Aspect\PromotedPropertyInterceptAspect; use Go\Tests\TestProject\Aspect\PropertyInterceptAspect; @@ -44,5 +45,6 @@ protected function configureAop(AspectContainer $container): void $container->addLazyService(AlphaCollisionAspect::class, fn(): AlphaCollisionAspect => new AlphaCollisionAspect()); $container->addLazyService(BetaCollisionAspect::class, fn(): BetaCollisionAspect => new BetaCollisionAspect()); $container->addLazyService(EdgeCaseAspect::class, fn(): EdgeCaseAspect => new EdgeCaseAspect()); + $container->addLazyService(MultiClassAspect::class, fn(): MultiClassAspect => new MultiClassAspect()); } } diff --git a/tests/Fixtures/project/web/index.php b/tests/Fixtures/project/web/index.php index 5cb2025ef..00214bfc9 100644 --- a/tests/Fixtures/project/web/index.php +++ b/tests/Fixtures/project/web/index.php @@ -3,6 +3,15 @@ include_once __DIR__ . '/../../../../vendor/autoload.php'; +// MultiClassHolder.php declares several classes (issue #760): the woven MultiClassSecond extends MultiClassPlain of +// that file. Weaving reflects the parent through composer, so it is registered the way a classmap autoloaded +// project knows it (PSR-4 can not locate a class declared in a file named after another class) +foreach (\Composer\Autoload\ClassLoader::getRegisteredLoaders() as $composerLoader) { + $composerLoader->addClassMap([ + \Go\Tests\TestProject\Application\MultiClassPlain::class => __DIR__ . '/../src/Application/MultiClassHolder.php', + ]); +} + $configuration = ($env = getenv('GO_AOP_CONFIGURATION')) ? $env : 'default' ; $settings = require __DIR__.'/configuration.php'; diff --git a/tests/Functional/MultiClassFileTest.php b/tests/Functional/MultiClassFileTest.php new file mode 100644 index 000000000..67f40bd08 --- /dev/null +++ b/tests/Functional/MultiClassFileTest.php @@ -0,0 +1,83 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Functional; + +use Go\Tests\TestProject\Application\MultiClassHolder; +use Go\Tests\TestProject\Application\MultiClassPlain; +use Go\Tests\TestProject\Application\MultiClassSecond; +use Go\Tests\TestProject\Application\Sub\MultiClassThird; +use Symfony\Component\Process\PhpExecutableFinder; +use Symfony\Component\Process\Process; + +/** + * Several woven classes declared in one file, in two namespace blocks, share one proxy file (issue #760): every + * class is declared once the file is loaded and runs its advice, the unwoven class of the file stays as it is + */ +class MultiClassFileTest extends BaseFunctionalTestCase +{ + public function testAllWovenClassesOfFileAreDeclaredAndAdvised(): void + { + $this->assertClassIsWoven(MultiClassHolder::class); + + $output = $this->runScript( + $this->getFirstLoadedClass(), + sprintf( + 'echo (new \\%s())->hello(), "|", (new \\%s())->hello(), "|", (new \\%s())->hello(), "|", (new \\%s())->greet();', + MultiClassHolder::class, + MultiClassSecond::class, + MultiClassThird::class, + MultiClassPlain::class, + ), + ); + + $this->assertSame('advised:holder|advised:second:plain|advised:third|plain', $output); + } + + /** + * Class loaded first: in debug mode composer locates the classes, so it is the one named after the file + */ + protected function getFirstLoadedClass(): string + { + return MultiClassHolder::class; + } + + private function runScript(string $firstLoadedClass, string $code): string + { + $phpExecutable = (new PhpExecutableFinder())->find(); + $this->assertIsString($phpExecutable); + $script = sprintf( + 'include %s; new \\%s(); %s', + var_export($this->configuration['frontController'], true), + $firstLoadedClass, + $code, + ); + $process = new Process( + [$phpExecutable, '-r', $script], + null, + ['GO_AOP_CONFIGURATION' => $this->getConfigurationName()], + ); + $process->run(); + + $this->assertTrue( + $process->isSuccessful(), + 'Loading the woven classes failed: ' . $process->getOutput() . $process->getErrorOutput(), + ); + + return trim($process->getOutput()); + } + + protected function getConfigurationName(): string + { + return 'default'; + } +} diff --git a/tests/Functional/ProductionMultiClassFileTest.php b/tests/Functional/ProductionMultiClassFileTest.php new file mode 100644 index 000000000..934537f46 --- /dev/null +++ b/tests/Functional/ProductionMultiClassFileTest.php @@ -0,0 +1,32 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Functional; + +use Go\Tests\TestProject\Application\Sub\MultiClassThird; + +/** + * Same as MultiClassFileTest with debug mode off: the runtime class map maps every woven class of the file to its + * woven file, so a class composer can not locate by its name is loaded first + */ +class ProductionMultiClassFileTest extends MultiClassFileTest +{ + protected function getFirstLoadedClass(): string + { + return MultiClassThird::class; + } + + protected function getConfigurationName(): string + { + return 'production'; + } +} diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index 97abddac6..3f6b4cfad 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -20,9 +20,13 @@ use Go\Core\AspectKernel; use Go\Core\AspectLoader; use Go\Instrument\ClassLoading\CachePathManager; +use Go\ParserReflection\Locator\CallableLocator; +use Go\ParserReflection\LocatorInterface; +use Go\ParserReflection\ReflectionEngine; use Go\VirtualFileSystem\FileSystem; use PHPUnit\Framework\MockObject\MockObject; use PhpParser\Node\Stmt\Class_; +use PhpParser\PhpVersion; use Go\Instrument\Transformer\Stubs\MultiLinePropertiesClass; use Go\Instrument\Transformer\Stubs\TokenSurgeryClass; use Go\PhpUnit\AssertsCompilablePhp; @@ -91,7 +95,8 @@ public function setUp(): void } /** - * It's a caution check that multiple namespaces are not yet supported + * Classes declared in several namespaces of one file share one proxy file (issue #760): every proxy keeps its + * namespace in a braced block of its own, the file is included once, in the block of the first class */ public function testMultipleNamespacesInOneFile(): void { @@ -101,6 +106,202 @@ public function testMultipleNamespacesInOneFile(): void $actual = $this->normalizeWhitespaces($metadata->source); $expected = $this->normalizeWhitespaces($this->loadTestMetadata('multiple-ns-woven')->source); $this->assertEquals($expected, $actual); + + $proxyContent = $this->getSingleIncludedProxy($metadata->source); + $this->assertEquals( + $this->normalizeWhitespaces($this->loadTestMetadata('multiple-ns-proxy')->source), + $this->normalizeWhitespaces($proxyContent), + ); + $this->assertPhpCompiles($proxyContent); + } + + /** + * A combined proxy file keeps the static initialization call of a proxy inside the namespace block of the proxy + */ + public function testCombinedProxyFileKeepsStaticInitializationInNamespaceBlockOfProxy(): void + { + $advice = new BeforeInterceptor(static function (): void {}); + $transformer = $this->createTransformerWithAdvices([ + AspectContainer::METHOD_PREFIX => ['test' => ['advisor.test' => $advice]], + AspectContainer::STATIC_INIT_PREFIX => ['root' => ['advisor.static' => $advice]], + ]); + $metadata = $this->loadTestMetadata('multiple-ns'); + $transformer->transform($metadata); + + $proxyContent = $this->getSingleIncludedProxy($metadata->source); + $this->assertStringContainsString("\n TestClass1::__staticInitialization();\n}\n\nnamespace Test\\ns2 {", $proxyContent); + $this->assertStringEndsWith("\n TestClass2::__staticInitialization();\n}", $proxyContent); + $this->assertPhpCompiles($proxyContent); + } + + /** + * Interfaces, enums, classes extending a class of the file and the global namespace in one file: unwoven + * class-likes stay untouched, the proxy file is included once after the last class-like whose proxy depends on + * its position (MixedChild extends the unwoven MixedPlain), the global namespace gets a `namespace {}` block + */ + public function testClassLikesOfMixedKindsShareOneProxyFile(): void + { + $metadata = $this->transformWithInFileReflection('multiple-mixed'); + + $actual = $this->normalizeWhitespaces($metadata->source); + $expected = $this->normalizeWhitespaces($this->loadTestMetadata('multiple-mixed-woven')->source); + $this->assertEquals($expected, $actual); + $this->assertPhpCompiles($metadata->source); + + $proxyContent = $this->getSingleIncludedProxy($metadata->source); + $this->assertEquals( + $this->normalizeWhitespaces($this->loadTestMetadata('multiple-mixed-proxy')->source), + $this->normalizeWhitespaces($proxyContent), + ); + $this->assertPhpCompiles($proxyContent); + } + + /** + * The woven file of mixed class-likes loads: the trait of a woven class-like without trait uses is declared at + * compile time, so proxies of class-likes declared after the include point find their trait + */ + public function testWovenFileOfMixedClassLikesLoadsAtRuntime(): void + { + $metadata = $this->transformWithInFileReflection('multiple-mixed'); + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $metadata->source, $matches)); + $proxyContent = file_get_contents('vfs://' . $matches[1]); + $this->assertIsString($proxyContent); + + // The virtual file system supports include, so the woven code runs without touching the disk + $runtimeDir = static::$fileSystem->path('/runtime-mixed'); + if (!is_dir($runtimeDir)) { + mkdir($runtimeDir, 0777, true); + } + $wovenFile = $runtimeDir . '/woven.php'; + $proxyFile = $runtimeDir . '/proxy.php'; + // The include stays in place, only the cache directory constant is replaced + $wovenSource = str_replace("AOP_CACHE_DIR . '{$matches[1]}'", var_export($proxyFile, true), $metadata->source); + try { + file_put_contents($wovenFile, $wovenSource); + file_put_contents($proxyFile, $proxyContent); + include $wovenFile; + + $this->assertTrue(class_exists('Test\mixed\MixedFirst', false)); + $this->assertTrue(class_exists('Test\mixed\MixedChild', false)); + $this->assertTrue(class_exists('MixedGlobal', false)); + $this->assertTrue(is_subclass_of('Test\mixed\MixedChild', 'Test\mixed\MixedPlain')); + $this->assertTrue(is_subclass_of('Test\mixed\MixedFirst', 'Test\mixed\MixedContract')); + $this->assertTrue(enum_exists('Test\mixed\MixedSuit', false)); + $hearts = constant('Test\mixed\MixedSuit::Hearts'); + $this->assertInstanceOf(\BackedEnum::class, $hearts); + $this->assertSame('H', $hearts->value); + } finally { + unlink($wovenFile); + unlink($proxyFile); + } + } + + /** + * A proxy extending another proxy of the file follows it in the proxy file, even when the source declares the + * child first + */ + public function testProxyOfParentDeclaredInSameFileComesFirst(): void + { + $metadata = $this->transformWithInFileReflection('multiple-parent-after-child'); + + $proxyContent = $this->getSingleIncludedProxy($metadata->source); + $parentOffset = strpos($proxyContent, 'class SortParent'); + $childOffset = strpos($proxyContent, 'class SortChild extends \\Test\\sort\\SortParent'); + $this->assertIsInt($parentOffset); + $this->assertIsInt($childOffset); + $this->assertLessThan($childOffset, $parentOffset); + $this->assertPhpCompiles($proxyContent); + } + + /** + * With several woven class-likes the proxy file is included after the last one whose proxy depends on its + * position. Code before that point needing a woven class-like is rejected with an actionable message instead + * of failing with "Class not found" when the file is loaded + */ + public function testCodeNeedingWovenClassBeforeProxyFileIncludeIsRejected(): void + { + try { + $this->transformWithInFileReflection('multiple-gap', [ + 'Test\gap\GapFirst' => ['hello'], + 'Test\gap\GapTrait' => ['traitHello'], + 'Test\gap\GapSecond' => ['hello'], + ]); + $this->fail('Code using a woven class before the include of the proxy file must be rejected'); + } catch (WeavingException $exception) { + $message = $exception->getMessage(); + $this->assertStringContainsString('_files/multiple-gap.php', $message); + $this->assertStringContainsString('Test\gap\GapFirst, Test\gap\GapTrait, Test\gap\GapSecond', $message); + $this->assertStringContainsString('included after Test\gap\GapSecond (line 41)', $message); + $this->assertStringContainsString('the statement on line 18 uses Test\gap\GapFirst', $message); + $this->assertStringContainsString('Test\gap\GapChild (line 20) needs Test\gap\GapFirst', $message); + $this->assertStringContainsString('Test\gap\GapSecond (line 32) needs Test\gap\GapTrait', $message); + $this->assertStringContainsString('exclude the class-likes from the pointcut', $message); + } + } + + /** + * References that do not load a class (instanceof, ::class, type declarations) and code running later (function + * and arrow function bodies) may use woven class-likes before the include of the proxy file + */ + public function testCodeNotLoadingWovenClassBeforeProxyFileIncludeIsAccepted(): void + { + $metadata = $this->transformWithInFileReflection('multiple-no-gap'); + + $source = $metadata->source; + $this->getSingleIncludedProxy($source); + $includeOffset = strpos($source, 'include_once AOP_CACHE_DIR'); + $secondOffset = strpos($source, 'trait NoGapSecondOriginalTrait'); + $usageOffset = strpos($source, '$first = new NoGapFirst();'); + $this->assertIsInt($includeOffset); + $this->assertIsInt($secondOffset); + $this->assertIsInt($usageOffset); + $this->assertGreaterThan($secondOffset, $includeOffset); + $this->assertLessThan($usageOffset, $includeOffset); + $this->assertPhpCompiles($source); + } + + /** + * The proxy file of a single woven class is byte-identical to the one generated before proxies were combined + */ + public function testSingleClassProxyIsByteIdentical(): void + { + foreach (['class' => 'class-proxy', 'php81-enum' => 'php81-enum-proxy'] as $fixture => $expectedProxy) { + $metadata = $this->loadTestMetadata($fixture); + $this->transformer->transform($metadata); + + $this->assertSame( + file_get_contents(__DIR__ . '/_files/' . $expectedProxy . '.php'), + $this->getSingleIncludedProxy($metadata->source), + ); + } + } + + public function testSingleClassProxyWithStaticInitializationIsByteIdentical(): void + { + $adviceMatcher = $this->createMock(AdviceMatcherInterface::class); + $adviceMatcher + ->method('getAdvicesForClass') + ->willReturnCallback(static function (ReflectionClass $refClass): array { + $methodAdvices = []; + foreach ($refClass->getMethods() as $method) { + $advisorId = "advisor.{$refClass->name}->{$method->name}"; + $methodAdvices[$method->name][$advisorId] = new BeforeInterceptor(static function (): void {}); + } + $staticAdvisorId = "advisor.{$refClass->name}->static"; + + return [ + AspectContainer::METHOD_PREFIX => $methodAdvices, + AspectContainer::STATIC_INIT_PREFIX => ['root' => [$staticAdvisorId => new BeforeInterceptor(static function (): void {})]], + ]; + }); + $adviceMatcher->method('getAdvicesForFunctions')->willReturn([]); + $metadata = $this->loadTestMetadata('class'); + $this->createTransformer($adviceMatcher)->transform($metadata); + + $this->assertSame( + file_get_contents(__DIR__ . '/_files/class-static-init-proxy.php'), + $this->getSingleIncludedProxy($metadata->source), + ); } /** @@ -795,7 +996,7 @@ public function testWeaverStripsAttributeClassMarkersFromWovenTrait(): void // The compatible part of the grouped attribute must survive $this->assertStringContainsString('#[\FakeMarkerAttr]', $actual); - // The proxy (last class in the file wins the shared cache path) must keep #[\Attribute(...)] + // The proxy file holds the proxies of all three classes, TestCustomAttribute keeps #[\Attribute(...)] $matches = []; $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $actual, $matches)); $proxyContent = (string) file_get_contents('vfs://' . $matches[1]); @@ -1507,6 +1708,88 @@ public function testMultipleClasses(): void $actual = $this->normalizeWhitespaces($metadata->source); $expected = $this->normalizeWhitespaces($this->loadTestMetadata('multiple-classes-woven')->source); $this->assertEquals($expected, $actual); + + // All proxies of the file live in one proxy file (issue #760), each in a namespace block of its own + $proxyContent = $this->getSingleIncludedProxy($metadata->source); + $this->assertEquals( + $this->normalizeWhitespaces($this->loadTestMetadata('multiple-classes-proxy')->source), + $this->normalizeWhitespaces($proxyContent), + ); + $this->assertPhpCompiles($proxyContent); + } + + /** + * Asserts that the woven source includes exactly one proxy file and returns its content + */ + private function getSingleIncludedProxy(string $wovenSource): string + { + $this->assertSame(1, substr_count($wovenSource, 'AOP_CACHE_DIR'), 'The proxy file must be included once'); + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $wovenSource, $matches)); + $proxyContent = file_get_contents('vfs://' . $matches[1]); + $this->assertIsString($proxyContent); + + return $proxyContent; + } + + /** + * Weaves a fixture whose classes extend, implement or use class-likes declared in the same fixture: the fixture + * is not autoloadable, so parser-reflection locates them in the fixture while it is woven + * + * @param array>|null $advisedMethods Advised methods by class name, null advises every method + */ + private function transformWithInFileReflection(string $name, ?array $advisedMethods = null): StreamMetaData + { + $fixtureFile = __DIR__ . '/_files/' . $name . '.php'; + // The engine has no getters: the current locator and grammar are read to wrap and restore them + $previousLocator = (new \ReflectionProperty(ReflectionEngine::class, 'locator'))->getValue(); + $phpVersion = (new \ReflectionProperty(ReflectionEngine::class, 'phpVersion'))->getValue(); + assert($previousLocator instanceof LocatorInterface); + assert($phpVersion === null || $phpVersion instanceof PhpVersion); + ReflectionEngine::init( + new CallableLocator( + static function (string $className) use ($previousLocator, $fixtureFile): string { + $classFile = $previousLocator->locateClass($className); + + return $classFile !== false ? $classFile : $fixtureFile; + }, + ), + $phpVersion, + ); + try { + $transformer = $this->transformer; + if ($advisedMethods !== null) { + $adviceMatcher = $this->createMock(AdviceMatcherInterface::class); + $adviceMatcher + ->method('getAdvicesForClass') + ->willReturnCallback(static function (ReflectionClass $refClass) use ($advisedMethods): array { + $advices = []; + foreach ($advisedMethods[$refClass->name] ?? [] as $methodName) { + $advisorId = "advisor.{$refClass->name}->{$methodName}"; + $advices[AspectContainer::METHOD_PREFIX][$methodName][$advisorId] = new BeforeInterceptor(static function (): void {}); + } + + return $advices; + }); + $adviceMatcher->method('getAdvicesForFunctions')->willReturn([]); + $transformer = $this->createTransformer($adviceMatcher); + } + $metadata = $this->loadTestMetadata($name); + $transformer->transform($metadata); + + return $metadata; + } finally { + ReflectionEngine::init($previousLocator, $phpVersion); + } + } + + private function createTransformer(AdviceMatcherInterface $adviceMatcher): WeavingTransformer + { + $loader = $this + ->getMockBuilder(AspectLoader::class) + ->setConstructorArgs([$this->getContainerMock()]) + ->getMock(); + + return new WeavingTransformer($this->kernel, $adviceMatcher, $this->cachePathManager, $loader); } /** diff --git a/tests/Instrument/Transformer/_files/class-proxy.php b/tests/Instrument/Transformer/_files/class-proxy.php index 57a0460b0..f08029d8a 100644 --- a/tests/Instrument/Transformer/_files/class-proxy.php +++ b/tests/Instrument/Transformer/_files/class-proxy.php @@ -1,6 +1,7 @@ __invoke($this, [$instance]); } -} +} \ No newline at end of file diff --git a/tests/Instrument/Transformer/_files/class-static-init-proxy.php b/tests/Instrument/Transformer/_files/class-static-init-proxy.php new file mode 100644 index 000000000..a5d9084be --- /dev/null +++ b/tests/Instrument/Transformer/_files/class-static-init-proxy.php @@ -0,0 +1,125 @@ + $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'publicMethod', + [ + Interceptor::before(The::advice('advisor.Test\ns1\TestClass->publicMethod')), + ], + $this->publicMethodOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } + protected function protectedMethod() + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'protectedMethod', + [ + Interceptor::before(The::advice('advisor.Test\ns1\TestClass->protectedMethod')), + ], + $this->protectedMethodOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } + public static function publicStaticMethod() + { + /** @var StaticMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forStaticMethod( + self::class, + 'publicStaticMethod', + [ + Interceptor::before(The::advice('advisor.Test\ns1\TestClass->publicStaticMethod')), + ], + self::publicStaticMethodOriginalAlias(...), + ); + return $__joinPoint->__invoke(static::class); + } + protected static function protectedStaticMethod() + { + /** @var StaticMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forStaticMethod( + self::class, + 'protectedStaticMethod', + [ + Interceptor::before(The::advice('advisor.Test\ns1\TestClass->protectedStaticMethod')), + ], + self::protectedStaticMethodOriginalAlias(...), + ); + return $__joinPoint->__invoke(static::class); + } + public function publicMethodDynamicArguments($a, &$b) + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'publicMethodDynamicArguments', + [ + Interceptor::before(The::advice('advisor.Test\ns1\TestClass->publicMethodDynamicArguments')), + ], + $this->publicMethodDynamicArgumentsOriginalAlias(...), + ); + return $__joinPoint->__invoke($this, [$a, &$b]); + } + public function publicMethodFixedArguments($a, $b, $c = null) + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'publicMethodFixedArguments', + [ + Interceptor::before(The::advice('advisor.Test\ns1\TestClass->publicMethodFixedArguments')), + ], + $this->publicMethodFixedArgumentsOriginalAlias(...), + ); + return $__joinPoint->__invoke($this, \array_slice([$a, $b, $c], 0, \func_num_args())); + } + public function methodWithSpecialTypeArguments(self $instance) + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'methodWithSpecialTypeArguments', + [ + Interceptor::before(The::advice('advisor.Test\ns1\TestClass->methodWithSpecialTypeArguments')), + ], + $this->methodWithSpecialTypeArgumentsOriginalAlias(...), + ); + return $__joinPoint->__invoke($this, [$instance]); + } + public static function __staticInitialization(): void + { + /** @var ClassJoinpoint $__joinPoint */ + static $__joinPoint = InterceptorInjector::forStaticInitialization( + self::class, + [ + Interceptor::before(The::advice('advisor.Test\ns1\TestClass->static')), + ], + ); + $__joinPoint(static::class); + } +} +TestClass::__staticInitialization(); \ No newline at end of file diff --git a/tests/Instrument/Transformer/_files/multiple-classes-proxy.php b/tests/Instrument/Transformer/_files/multiple-classes-proxy.php new file mode 100644 index 000000000..6945f376d --- /dev/null +++ b/tests/Instrument/Transformer/_files/multiple-classes-proxy.php @@ -0,0 +1,79 @@ + $__joinPoint */ + static $__joinPoint = InterceptorInjector::forStaticMethod( + self::class, + 'test', + [ + Interceptor::before(The::advice('advisor.Test\ns3\TestClass1->test')), + ], + self::testOriginalAlias(...), + ); + return $__joinPoint->__invoke(static::class); + } + } +} + +namespace Test\ns3 { + use Go\Aop\Framework\InterceptorInjector; + use Go\Aop\Framework\Interceptor; + use Go\Aop\Framework\The; + use Go\Aop\Intercept\StaticMethodInvocation; + class TestClass11 implements \Go\Aop\Proxy + { + use TestClass11OriginalTrait { + TestClass11OriginalTrait::test as private testOriginalAlias; + } + public static function test() + { + /** @var StaticMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forStaticMethod( + self::class, + 'test', + [ + Interceptor::before(The::advice('advisor.Test\ns3\TestClass11->test')), + ], + self::testOriginalAlias(...), + ); + return $__joinPoint->__invoke(static::class); + } + } +} + +namespace Test\ns3 { + use Go\Aop\Framework\InterceptorInjector; + use Go\Aop\Framework\Interceptor; + use Go\Aop\Framework\The; + use Go\Aop\Intercept\StaticMethodInvocation; + class TestClass2 implements \Go\Aop\Proxy + { + use TestClass2OriginalTrait { + TestClass2OriginalTrait::test as private testOriginalAlias; + } + public static function test() + { + /** @var StaticMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forStaticMethod( + self::class, + 'test', + [ + Interceptor::before(The::advice('advisor.Test\ns3\TestClass2->test')), + ], + self::testOriginalAlias(...), + ); + return $__joinPoint->__invoke(static::class); + } + } +} \ No newline at end of file diff --git a/tests/Instrument/Transformer/_files/multiple-classes-woven.php b/tests/Instrument/Transformer/_files/multiple-classes-woven.php index 08a37adea..62dbb027e 100644 --- a/tests/Instrument/Transformer/_files/multiple-classes-woven.php +++ b/tests/Instrument/Transformer/_files/multiple-classes-woven.php @@ -10,10 +10,8 @@ public static function test() {} trait TestClass11OriginalTrait { public static function test() {} } -include_once AOP_CACHE_DIR . '/Transformer/_files/multiple-classes.php'; TestClass11::test(); trait TestClass2OriginalTrait { public static function test() {} } -include_once AOP_CACHE_DIR . '/Transformer/_files/multiple-classes.php'; TestClass2::test(); diff --git a/tests/Instrument/Transformer/_files/multiple-gap.php b/tests/Instrument/Transformer/_files/multiple-gap.php new file mode 100644 index 000000000..ac31619b5 --- /dev/null +++ b/tests/Instrument/Transformer/_files/multiple-gap.php @@ -0,0 +1,41 @@ + $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'run', + [ + Interceptor::before(The::advice('advisor.Test\mixed\MixedFirst->run')), + ], + $this->runOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } + } +} + +namespace Test\mixed { + use Go\Aop\Framework\InterceptorInjector; + use Go\Aop\Framework\Interceptor; + use Go\Aop\Framework\The; + use Go\Aop\Intercept\DynamicMethodInvocation; + enum MixedSuit : string implements \Go\Aop\Proxy + { + use MixedSuitOriginalTrait { + MixedSuitOriginalTrait::label as private labelOriginalAlias; + } + case Hearts = 'H'; + public function label(): string + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'label', + [ + Interceptor::before(The::advice('advisor.Test\mixed\MixedSuit->label')), + ], + $this->labelOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } + } +} + +namespace Test\mixed { + use Go\Aop\Framework\InterceptorInjector; + use Go\Aop\Framework\Interceptor; + use Go\Aop\Framework\The; + use Go\Aop\Intercept\DynamicMethodInvocation; + class MixedChild extends \Test\mixed\MixedPlain implements \Go\Aop\Proxy + { + use MixedChildOriginalTrait { + MixedChildOriginalTrait::hello as private helloOriginalAlias; + } + public function hello(): string + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'hello', + [ + Interceptor::before(The::advice('advisor.Test\mixed\MixedChild->hello')), + ], + $this->helloOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } + } +} + +namespace { + use Go\Aop\Framework\InterceptorInjector; + use Go\Aop\Framework\Interceptor; + use Go\Aop\Framework\The; + use Go\Aop\Intercept\DynamicMethodInvocation; + class MixedGlobal implements \Go\Aop\Proxy + { + use MixedGlobalOriginalTrait { + MixedGlobalOriginalTrait::hello as private helloOriginalAlias; + } + public function hello(): string + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'hello', + [ + Interceptor::before(The::advice('advisor.MixedGlobal->hello')), + ], + $this->helloOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } + } +} \ No newline at end of file diff --git a/tests/Instrument/Transformer/_files/multiple-mixed-woven.php b/tests/Instrument/Transformer/_files/multiple-mixed-woven.php new file mode 100644 index 000000000..f0eacda1c --- /dev/null +++ b/tests/Instrument/Transformer/_files/multiple-mixed-woven.php @@ -0,0 +1,50 @@ +name); + } + } + + trait MixedChildOriginalTrait + { + public function hello(): string + { + return 'child'; + } + } +include_once AOP_CACHE_DIR . '/Transformer/_files/multiple-mixed.php'; +} + +namespace { + trait MixedGlobalOriginalTrait + { + public function hello(): string + { + return 'global'; + } + } +} diff --git a/tests/Instrument/Transformer/_files/multiple-mixed.php b/tests/Instrument/Transformer/_files/multiple-mixed.php new file mode 100644 index 000000000..5158664e3 --- /dev/null +++ b/tests/Instrument/Transformer/_files/multiple-mixed.php @@ -0,0 +1,49 @@ +name); + } + } + + class MixedChild extends MixedPlain + { + public function hello(): string + { + return 'child'; + } + } +} + +namespace { + class MixedGlobal + { + public function hello(): string + { + return 'global'; + } + } +} diff --git a/tests/Instrument/Transformer/_files/multiple-no-gap.php b/tests/Instrument/Transformer/_files/multiple-no-gap.php new file mode 100644 index 000000000..770b28535 --- /dev/null +++ b/tests/Instrument/Transformer/_files/multiple-no-gap.php @@ -0,0 +1,38 @@ + new NoGapFirst(); + +function createFirst(): NoGapFirst +{ + return new NoGapFirst(); +} + +class NoGapSecond +{ + use NoGapHelper; + + public function hello(): string + { + return 'second'; + } +} + +$first = new NoGapFirst(); +$second = new NoGapSecond(); diff --git a/tests/Instrument/Transformer/_files/multiple-ns-proxy.php b/tests/Instrument/Transformer/_files/multiple-ns-proxy.php new file mode 100644 index 000000000..d86358254 --- /dev/null +++ b/tests/Instrument/Transformer/_files/multiple-ns-proxy.php @@ -0,0 +1,53 @@ + $__joinPoint */ + static $__joinPoint = InterceptorInjector::forStaticMethod( + self::class, + 'test', + [ + Interceptor::before(The::advice('advisor.Test\ns1\TestClass1->test')), + ], + self::testOriginalAlias(...), + ); + return $__joinPoint->__invoke(static::class); + } + } +} + +namespace Test\ns2 { + use Go\Aop\Framework\InterceptorInjector; + use Go\Aop\Framework\Interceptor; + use Go\Aop\Framework\The; + use Go\Aop\Intercept\StaticMethodInvocation; + class TestClass2 implements \Go\Aop\Proxy + { + use TestClass2OriginalTrait { + TestClass2OriginalTrait::test as private testOriginalAlias; + } + public static function test() + { + /** @var StaticMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forStaticMethod( + self::class, + 'test', + [ + Interceptor::before(The::advice('advisor.Test\ns2\TestClass2->test')), + ], + self::testOriginalAlias(...), + ); + return $__joinPoint->__invoke(static::class); + } + } +} \ No newline at end of file diff --git a/tests/Instrument/Transformer/_files/multiple-ns-woven.php b/tests/Instrument/Transformer/_files/multiple-ns-woven.php index 62ded0ad7..daeafd6b9 100644 --- a/tests/Instrument/Transformer/_files/multiple-ns-woven.php +++ b/tests/Instrument/Transformer/_files/multiple-ns-woven.php @@ -10,5 +10,4 @@ public static function test() {} trait TestClass2OriginalTrait { public static function test() {} } -include_once AOP_CACHE_DIR . '/Transformer/_files/multiple-ns.php'; } diff --git a/tests/Instrument/Transformer/_files/multiple-parent-after-child.php b/tests/Instrument/Transformer/_files/multiple-parent-after-child.php new file mode 100644 index 000000000..fc0277a16 --- /dev/null +++ b/tests/Instrument/Transformer/_files/multiple-parent-after-child.php @@ -0,0 +1,20 @@ +reason; } } -include_once AOP_CACHE_DIR . '/Transformer/_files/php80-attribute-class.php'; diff --git a/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php b/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php index 01a68ab34..da5e46b53 100644 --- a/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php +++ b/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php @@ -28,4 +28,3 @@ public function doSomethingElse(): string return 'else'; } } -include_once AOP_CACHE_DIR . '/Transformer/_files/php80-class-attribute.php'; diff --git a/tests/Instrument/Transformer/_files/php81-enum-proxy.php b/tests/Instrument/Transformer/_files/php81-enum-proxy.php index de2803339..1884b67b4 100644 --- a/tests/Instrument/Transformer/_files/php81-enum-proxy.php +++ b/tests/Instrument/Transformer/_files/php81-enum-proxy.php @@ -1,6 +1,7 @@ __invoke($this); } -} +} \ No newline at end of file From 22a41b2a9b0c2a7bcbfe42548fee403693e315d8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 22:43:45 +0000 Subject: [PATCH 2/3] test(weaving): reference the in-file fixture class by name for static analysis Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KXjD75egSqDNujmNS5kr5c --- tests/Functional/ProductionMultiClassFileTest.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/Functional/ProductionMultiClassFileTest.php b/tests/Functional/ProductionMultiClassFileTest.php index 934537f46..33d0e4774 100644 --- a/tests/Functional/ProductionMultiClassFileTest.php +++ b/tests/Functional/ProductionMultiClassFileTest.php @@ -12,8 +12,6 @@ namespace Go\Functional; -use Go\Tests\TestProject\Application\Sub\MultiClassThird; - /** * Same as MultiClassFileTest with debug mode off: the runtime class map maps every woven class of the file to its * woven file, so a class composer can not locate by its name is loaded first @@ -22,7 +20,8 @@ class ProductionMultiClassFileTest extends MultiClassFileTest { protected function getFirstLoadedClass(): string { - return MultiClassThird::class; + // Declared in MultiClassHolder.php: not discoverable by name for static analysis + return 'Go\\Tests\\TestProject\\Application\\Sub\\MultiClassThird'; } protected function getConfigurationName(): string From cd84761eb86c47599be0a945e88c6620842d8f32 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 22:51:08 +0000 Subject: [PATCH 3/3] test(weaving): cover combined proxy edge cases - Cover a file without namespace declaration (one `namespace {}` block per proxy, loaded at runtime), two woven traits sharing a proxy file (child trait proxies via TraitProxyGenerator::generateStmts(), loaded at runtime) and a class declared by a top-level statement in the gap region. - ProxyIncludePlanner: drop the unexecuted assert and the unreachable top-level statement branch (parser-reflection wraps files without namespace into a global namespace block). WeavingTransformer reads the end of a woven declaration through getDeclarationTokenRange(), which rejects nodes without token positions like the rest of the weaver, instead of a separate branch. - Platform-neutral tests for Windows: separator-neutral path assertion, byte-exact proxy snapshots with a PHP_EOL header, and the attribute test reads the include from the raw woven source. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KXjD75egSqDNujmNS5kr5c --- .../Transformer/ProxyIncludePlanner.php | 10 +- .../Transformer/WeavingTransformer.php | 5 +- .../Transformer/WeavingTransformerTest.php | 146 +++++++++++++----- .../Transformer/_files/multiple-gap.php | 6 + .../_files/multiple-global-proxy.php | 53 +++++++ .../Transformer/_files/multiple-global.php | 20 +++ .../_files/multiple-traits-proxy.php | 53 +++++++ .../Transformer/_files/multiple-traits.php | 20 +++ 8 files changed, 266 insertions(+), 47 deletions(-) create mode 100644 tests/Instrument/Transformer/_files/multiple-global-proxy.php create mode 100644 tests/Instrument/Transformer/_files/multiple-global.php create mode 100644 tests/Instrument/Transformer/_files/multiple-traits-proxy.php create mode 100644 tests/Instrument/Transformer/_files/multiple-traits.php diff --git a/src/Instrument/Transformer/ProxyIncludePlanner.php b/src/Instrument/Transformer/ProxyIncludePlanner.php index 10e6b1edd..fba49c4f5 100644 --- a/src/Instrument/Transformer/ProxyIncludePlanner.php +++ b/src/Instrument/Transformer/ProxyIncludePlanner.php @@ -45,7 +45,7 @@ final class ProxyIncludePlanner * * @param non-empty-list $proxies Proxies in source order * - * @return non-empty-list + * @return list */ public static function sortForProxyFile(array $proxies): array { @@ -58,7 +58,6 @@ public static function sortForProxyFile(array $proxies): array foreach ($proxies as $proxy) { self::place($proxy, $byName, $placed, $sorted); } - assert($sorted !== []); return $sorted; } @@ -225,7 +224,8 @@ private static function place(WovenProxy $proxy, array $byName, array &$placed, } /** - * Statements directly in the file or in its namespace blocks + * Statements of the namespace blocks of the file. Parser-reflection wraps the statements of a file without a + * namespace declaration into a global namespace block, only declare statements stay outside of the blocks. * * @param array $syntaxTree * @@ -237,10 +237,6 @@ private static function getTopLevelStatements(array $syntaxTree): array foreach ($syntaxTree as $node) { if ($node instanceof Stmt\Namespace_) { array_push($statements, ...$node->stmts); - } elseif ($node instanceof Stmt\Declare_) { - array_push($statements, ...($node->stmts ?? [])); - } elseif ($node instanceof Stmt) { - $statements[] = $node; } } diff --git a/src/Instrument/Transformer/WeavingTransformer.php b/src/Instrument/Transformer/WeavingTransformer.php index a7d8a9c7d..cd316fa87 100644 --- a/src/Instrument/Transformer/WeavingTransformer.php +++ b/src/Instrument/Transformer/WeavingTransformer.php @@ -163,10 +163,7 @@ private function processSingleClass( } // The include of the proxy file goes after the last token of a woven class - $lastClassToken = $class->getNode()->getAttribute('endTokenPos'); - if (!is_int($lastClassToken)) { - return null; - } + [, $lastClassToken] = $this->getDeclarationTokenRange($class->getNode()); // Sort advices in advance to keep the correct order in cache, and leave only keys for the cache $advices = AbstractJoinpoint::flatAndSortAdvices($advices); diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index 3f6b4cfad..b894a8d83 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -163,36 +163,71 @@ public function testClassLikesOfMixedKindsShareOneProxyFile(): void public function testWovenFileOfMixedClassLikesLoadsAtRuntime(): void { $metadata = $this->transformWithInFileReflection('multiple-mixed'); - $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $metadata->source, $matches)); - $proxyContent = file_get_contents('vfs://' . $matches[1]); - $this->assertIsString($proxyContent); + $this->includeWovenFile($metadata, 'runtime-mixed'); - // The virtual file system supports include, so the woven code runs without touching the disk - $runtimeDir = static::$fileSystem->path('/runtime-mixed'); - if (!is_dir($runtimeDir)) { - mkdir($runtimeDir, 0777, true); - } - $wovenFile = $runtimeDir . '/woven.php'; - $proxyFile = $runtimeDir . '/proxy.php'; - // The include stays in place, only the cache directory constant is replaced - $wovenSource = str_replace("AOP_CACHE_DIR . '{$matches[1]}'", var_export($proxyFile, true), $metadata->source); - try { - file_put_contents($wovenFile, $wovenSource); - file_put_contents($proxyFile, $proxyContent); - include $wovenFile; + $this->assertTrue(class_exists('Test\mixed\MixedFirst', false)); + $this->assertTrue(class_exists('Test\mixed\MixedChild', false)); + $this->assertTrue(class_exists('MixedGlobal', false)); + $this->assertTrue(is_subclass_of('Test\mixed\MixedChild', 'Test\mixed\MixedPlain')); + $this->assertTrue(is_subclass_of('Test\mixed\MixedFirst', 'Test\mixed\MixedContract')); + $this->assertTrue(enum_exists('Test\mixed\MixedSuit', false)); + $hearts = constant('Test\mixed\MixedSuit::Hearts'); + $this->assertInstanceOf(\BackedEnum::class, $hearts); + $this->assertSame('H', $hearts->value); + } - $this->assertTrue(class_exists('Test\mixed\MixedFirst', false)); - $this->assertTrue(class_exists('Test\mixed\MixedChild', false)); - $this->assertTrue(class_exists('MixedGlobal', false)); - $this->assertTrue(is_subclass_of('Test\mixed\MixedChild', 'Test\mixed\MixedPlain')); - $this->assertTrue(is_subclass_of('Test\mixed\MixedFirst', 'Test\mixed\MixedContract')); - $this->assertTrue(enum_exists('Test\mixed\MixedSuit', false)); - $hearts = constant('Test\mixed\MixedSuit::Hearts'); - $this->assertInstanceOf(\BackedEnum::class, $hearts); - $this->assertSame('H', $hearts->value); - } finally { - unlink($wovenFile); - unlink($proxyFile); + /** + * Classes of a file without a namespace declaration get one `namespace {}` block per proxy + */ + public function testClassesWithoutNamespaceShareOneProxyFile(): void + { + $metadata = $this->loadTestMetadata('multiple-global'); + $this->transformer->transform($metadata); + + $proxyContent = $this->getSingleIncludedProxy($metadata->source); + $this->assertEquals( + $this->normalizeWhitespaces($this->loadTestMetadata('multiple-global-proxy')->source), + $this->normalizeWhitespaces($proxyContent), + ); + $this->assertPhpCompiles($proxyContent); + + // The include follows the first class, before the code of the file + $includeOffset = strpos($metadata->source, 'include_once AOP_CACHE_DIR'); + $this->assertIsInt($includeOffset); + $this->assertGreaterThan(strpos($metadata->source, 'trait GlobalMultiFirstOriginalTrait'), $includeOffset); + $this->assertLessThan(strpos($metadata->source, '$globalMultiName'), $includeOffset); + + $this->includeWovenFile($metadata, 'runtime-global'); + $this->assertTrue(class_exists('GlobalMultiFirst', false)); + $this->assertTrue(class_exists('GlobalMultiSecond', false)); + } + + /** + * Two woven traits of one file: both child trait proxies share the proxy file + */ + public function testTraitsShareOneProxyFile(): void + { + $metadata = $this->loadTestMetadata('multiple-traits'); + $this->transformer->transform($metadata); + + $proxyContent = $this->getSingleIncludedProxy($metadata->source); + $this->assertEquals( + $this->normalizeWhitespaces($this->loadTestMetadata('multiple-traits-proxy')->source), + $this->normalizeWhitespaces($proxyContent), + ); + $this->assertPhpCompiles($proxyContent); + + $this->includeWovenFile($metadata, 'runtime-traits'); + $this->assertTrue(trait_exists('Test\traits\FirstMultiTrait', false)); + $this->assertTrue(trait_exists('Test\traits\SecondMultiTrait', false)); + // Each proxy trait composes its own original trait + $proxyTraits = array_values(array_filter( + get_declared_traits(), + static fn(string $trait): bool => str_starts_with($trait, 'Test\traits\\') && !str_ends_with($trait, 'OriginalTrait'), + )); + $this->assertCount(2, $proxyTraits); + foreach ($proxyTraits as $proxyTrait) { + $this->assertSame([$proxyTrait . 'OriginalTrait'], (new \ReflectionClass($proxyTrait))->getTraitNames()); } } @@ -229,12 +264,14 @@ public function testCodeNeedingWovenClassBeforeProxyFileIncludeIsRejected(): voi $this->fail('Code using a woven class before the include of the proxy file must be rejected'); } catch (WeavingException $exception) { $message = $exception->getMessage(); - $this->assertStringContainsString('_files/multiple-gap.php', $message); + $this->assertStringContainsString('_files/multiple-gap.php', str_replace('\\', '/', $message)); $this->assertStringContainsString('Test\gap\GapFirst, Test\gap\GapTrait, Test\gap\GapSecond', $message); - $this->assertStringContainsString('included after Test\gap\GapSecond (line 41)', $message); + $this->assertStringContainsString('included after Test\gap\GapSecond (line 47)', $message); $this->assertStringContainsString('the statement on line 18 uses Test\gap\GapFirst', $message); $this->assertStringContainsString('Test\gap\GapChild (line 20) needs Test\gap\GapFirst', $message); - $this->assertStringContainsString('Test\gap\GapSecond (line 32) needs Test\gap\GapTrait', $message); + // A class declared by a top-level statement is declared when the statement runs + $this->assertStringContainsString('the statement on line 24 uses Test\gap\GapFirst', $message); + $this->assertStringContainsString('Test\gap\GapSecond (line 38) needs Test\gap\GapTrait', $message); $this->assertStringContainsString('exclude the class-likes from the pointcut', $message); } } @@ -270,7 +307,7 @@ public function testSingleClassProxyIsByteIdentical(): void $this->transformer->transform($metadata); $this->assertSame( - file_get_contents(__DIR__ . '/_files/' . $expectedProxy . '.php'), + $this->loadExactProxySnapshot($expectedProxy), $this->getSingleIncludedProxy($metadata->source), ); } @@ -299,11 +336,25 @@ public function testSingleClassProxyWithStaticInitializationIsByteIdentical(): v $this->createTransformer($adviceMatcher)->transform($metadata); $this->assertSame( - file_get_contents(__DIR__ . '/_files/class-static-init-proxy.php'), + $this->loadExactProxySnapshot('class-static-init-proxy'), $this->getSingleIncludedProxy($metadata->source), ); } + /** + * Returns a byte-exact proxy snapshot (stored with LF line endings) as the weaver writes it on this platform: the + * `assertIsString($snapshot); + $this->assertStringStartsWith($header, $snapshot); + + return 'assertStringContainsString('#[\FakeMarkerAttr]', $actual); // The proxy file holds the proxies of all three classes, TestCustomAttribute keeps #[\Attribute(...)] - $matches = []; - $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $actual, $matches)); - $proxyContent = (string) file_get_contents('vfs://' . $matches[1]); + $proxyContent = $this->getSingleIncludedProxy($metadata->source); $this->assertStringContainsString( '#[\Attribute(\Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]', $proxyContent, @@ -1718,6 +1767,31 @@ public function testMultipleClasses(): void $this->assertPhpCompiles($proxyContent); } + /** + * Includes the woven source with its proxy file from the virtual file system: the include stays in place, only + * the cache directory constant is replaced + */ + private function includeWovenFile(StreamMetaData $metadata, string $directoryName): void + { + $proxyContent = $this->getSingleIncludedProxy($metadata->source); + $this->assertSame(1, preg_match("/AOP_CACHE_DIR \\. ('[^']+')/", $metadata->source, $matches)); + + $runtimeDir = static::$fileSystem->path('/' . $directoryName); + if (!is_dir($runtimeDir)) { + mkdir($runtimeDir, 0777, true); + } + $wovenFile = $runtimeDir . '/woven.php'; + $proxyFile = $runtimeDir . '/proxy.php'; + try { + file_put_contents($wovenFile, str_replace("AOP_CACHE_DIR . {$matches[1]}", var_export($proxyFile, true), $metadata->source)); + file_put_contents($proxyFile, $proxyContent); + include $wovenFile; + } finally { + unlink($wovenFile); + unlink($proxyFile); + } + } + /** * Asserts that the woven source includes exactly one proxy file and returns its content */ diff --git a/tests/Instrument/Transformer/_files/multiple-gap.php b/tests/Instrument/Transformer/_files/multiple-gap.php index ac31619b5..fb4771f86 100644 --- a/tests/Instrument/Transformer/_files/multiple-gap.php +++ b/tests/Instrument/Transformer/_files/multiple-gap.php @@ -21,6 +21,12 @@ class GapChild extends GapFirst { } +if (!class_exists(GapNested::class, false)) { + class GapNested extends GapFirst + { + } +} + trait GapTrait { public function traitHello(): string diff --git a/tests/Instrument/Transformer/_files/multiple-global-proxy.php b/tests/Instrument/Transformer/_files/multiple-global-proxy.php new file mode 100644 index 000000000..1e7b3d752 --- /dev/null +++ b/tests/Instrument/Transformer/_files/multiple-global-proxy.php @@ -0,0 +1,53 @@ + $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'hello', + [ + Interceptor::before(The::advice('advisor.GlobalMultiFirst->hello')), + ], + $this->helloOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } + } +} + +namespace { + use Go\Aop\Framework\InterceptorInjector; + use Go\Aop\Framework\Interceptor; + use Go\Aop\Framework\The; + use Go\Aop\Intercept\DynamicMethodInvocation; + class GlobalMultiSecond implements \Go\Aop\Proxy + { + use GlobalMultiSecondOriginalTrait { + GlobalMultiSecondOriginalTrait::hello as private helloOriginalAlias; + } + public function hello(): string + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'hello', + [ + Interceptor::before(The::advice('advisor.GlobalMultiSecond->hello')), + ], + $this->helloOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } + } +} \ No newline at end of file diff --git a/tests/Instrument/Transformer/_files/multiple-global.php b/tests/Instrument/Transformer/_files/multiple-global.php new file mode 100644 index 000000000..abd2d3d14 --- /dev/null +++ b/tests/Instrument/Transformer/_files/multiple-global.php @@ -0,0 +1,20 @@ + $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'first', + [ + Interceptor::before(The::advice('advisor.Test\traits\FirstMultiTrait->first')), + ], + $this->firstOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } + } +} + +namespace Test\traits { + use Go\Aop\Framework\InterceptorInjector; + use Go\Aop\Framework\Interceptor; + use Go\Aop\Framework\The; + use Go\Aop\Intercept\DynamicMethodInvocation; + trait SecondMultiTrait + { + use SecondMultiTraitOriginalTrait { + SecondMultiTraitOriginalTrait::second as private secondOriginalAlias; + } + public function second(): string + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'second', + [ + Interceptor::before(The::advice('advisor.Test\traits\SecondMultiTrait->second')), + ], + $this->secondOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } + } +} \ No newline at end of file diff --git a/tests/Instrument/Transformer/_files/multiple-traits.php b/tests/Instrument/Transformer/_files/multiple-traits.php new file mode 100644 index 000000000..e774cde2c --- /dev/null +++ b/tests/Instrument/Transformer/_files/multiple-traits.php @@ -0,0 +1,20 @@ +