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 @@ +