From 8d26964ec10fdf4339dd8b96f0dbd2ea48df1952 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 16:49:54 +0000 Subject: [PATCH] fix(weaving): copy the imports of the namespace block declaring the woven class A file may declare the same namespace in several blocks, each with its own imports. WeavingTransformer::processSingleClass() looked the imports up again with `new ReflectionFileNamespace($file, $namespaceName)`, which returns the first block of that name, so the proxy of a class declared in a later block got the imports of the first block: a parameter default like `Level::LEVEL` then referred to the wrong class. The imports are now read from the ReflectionFileNamespace being woven, passed down from transform(). This also drops the second namespace lookup per woven class. Extracted from the declined #757 (#749). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KXjD75egSqDNujmNS5kr5c --- CHANGELOG.md | 1 + .../Transformer/WeavingTransformer.php | 23 ++++++++++--------- .../Transformer/WeavingTransformerTest.php | 21 +++++++++++++++++ .../Transformer/_files/duplicate-ns.php | 21 +++++++++++++++++ 4 files changed, 55 insertions(+), 11 deletions(-) create mode 100644 tests/Instrument/Transformer/_files/duplicate-ns.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 71bcc507..c8bdff5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,7 @@ See [UPGRADE-4.0.md](UPGRADE-4.0.md) for the migration guide. * [Fixed] Closure advices registered through `PointcutBuilder` get stable advisor ids derived from the expression, the advice kind and the closure's source location instead of a process-wide counter, so changing the order of registrations in `configureAop()` no longer makes cached proxies call the wrong advice or fail with an unknown id (#640). * [Fixed] The weaving metadata files (`_transformation.cache`, `_include.cache`) carry a format version: a cache of another version is rebuilt, and with `Features::PREBUILT_CACHE` it fails with a hint to run `cache:warmup:aop`. Records keep the size and mtime of the woven source, so a source whose mtime moved backwards (rsync `-t`, checkout of an older revision) is woven again instead of serving the stale proxy (#685). * [Fixed] `cache:warmup:aop` and `debug:advisor` no longer fail with "Cannot rewind a generator that was already run" when more than one include path is configured (#751). +* [Fixed] The proxy of a class declared in the second (or a later) block of a namespace that a file declares several times copies the imports of its own block instead of the first block's. **Internal** * [Internal] Tooling: PHPUnit 13 with strict flags, PHPStan 2 at level 10 over `src/` and `tests/`, php-cs-fixer with the PER-CS rule set, and `composer test`/`analyze`/`cs`/`check` scripts. diff --git a/src/Instrument/Transformer/WeavingTransformer.php b/src/Instrument/Transformer/WeavingTransformer.php index b58981ef..20f025fa 100644 --- a/src/Instrument/Transformer/WeavingTransformer.php +++ b/src/Instrument/Transformer/WeavingTransformer.php @@ -109,6 +109,7 @@ public function transform(StreamMetaData $metadata): TransformerResult $advisors, $metadata, $class, + $namespace, $parsedSource->isStrictMode(), ); $totalTransformations += (int) $wasClassProcessed; @@ -125,16 +126,18 @@ public function transform(StreamMetaData $metadata): TransformerResult /** * 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 bool $useStrictMode If the source file used strict mode, the proxy should too + * @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 */ private function processSingleClass( array $advisors, StreamMetaData $metadata, ReflectionClass $class, + ReflectionFileNamespace $namespace, bool $useStrictMode, ): bool { try { @@ -165,13 +168,11 @@ private function processSingleClass( $newClassName = $class->getShortName() . AspectContainer::ORIGINAL_TRAIT_SUFFIX; $newFqcn = ($class->getNamespaceName() !== '' ? $class->getNamespaceName() . '\\' : '') . $newClassName; - $classFileName = $class->getFileName(); - if ($classFileName === false) { - return false; - } - // Imports of the original file are copied into the proxy (parameter defaults and types rely on - // them); the generators reserve their names and alias their own imports around them - $originalImports = new ReflectionFileNamespace($classFileName, $class->getNamespaceName())->getNamespaceAliases(); + // Imports of the namespace block declaring the class are copied into the proxy (parameter defaults + // and types rely on them); the generators reserve their names and alias their own imports around them. + // They are read from the block being woven: a lookup by file and namespace name returns the first block + // when the file declares the same namespace several times + $originalImports = $namespace->getNamespaceAliases(); // Intercepted methods are looked up by name several times per method, here and in the proxy generators $methods = ClassProxyGenerator::indexMethods($class); diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index 2081d529..97abddac 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -103,6 +103,27 @@ public function testMultipleNamespacesInOneFile(): void $this->assertEquals($expected, $actual); } + /** + * A file may declare the same namespace in several blocks, each with its own imports: the proxy of a class + * must copy the imports of the block declaring it, not the ones of the first block of that namespace + */ + public function testProxyCopiesImportsOfTheNamespaceBlockDeclaringTheClass(): void + { + $metadata = $this->loadTestMetadata('duplicate-ns'); + $this->transformer->transform($metadata); + + $this->assertStringContainsString('trait SecondBlockClassOriginalTrait', $metadata->source); + $this->assertStringNotContainsString('FirstBlockClassOriginalTrait', $metadata->source); + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $metadata->source, $matches)); + + $proxyContent = (string) file_get_contents('vfs://' . $matches[1]); + $this->assertStringContainsString('class SecondBlockClass', $proxyContent); + $this->assertStringContainsString('use Go\\Stubs\\Collision\\The as Level;', $proxyContent); + $this->assertStringNotContainsString('Go\\Stubs\\Collision\\Interceptor', $proxyContent); + $this->assertStringContainsString('string $level = Level::LEVEL', $proxyContent); + $this->assertPhpCompiles($proxyContent); + } + /** * Do not make anything for code without classes */ diff --git a/tests/Instrument/Transformer/_files/duplicate-ns.php b/tests/Instrument/Transformer/_files/duplicate-ns.php new file mode 100644 index 00000000..36118b9a --- /dev/null +++ b/tests/Instrument/Transformer/_files/duplicate-ns.php @@ -0,0 +1,21 @@ +