From 1110746e019a5874305c0745bcc604e1d4cb9a69 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 22:19:25 +0000 Subject: [PATCH] fix(proxy): emit fully-qualified parent for classes extending global classes A woven namespaced class extending a global class (\ArrayObject, \Exception, ...) got a proxy with `extends ArrayObject`: the reflection parent name has no leading backslash, so ClassGenerator emitted a relative name that resolved in the proxy namespace and loading the proxy failed with `Class "Ns\ArrayObject" not found`. - ClassProxyGenerator roots the reflection parent name, like it already does for the original interfaces; introduced traits and interfaces are rooted too, so direct callers that bypass AdviceMatcher are safe. - ClassGenerator::addTraitAlias() keeps the trait name as given, so a rooted global trait stays fully qualified in its alias adaptation. - The OriginalTrait reference keeps its deliberate short relative name. Tests: weaving snapshot plus a runtime load check for a namespaced class extending \ArrayObject, ClassProxyGenerator cases for the global parent and global introductions, and a ClassGenerator alias case. No existing snapshot changed. Closes #759 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KXjD75egSqDNujmNS5kr5c --- src/Proxy/AGENTS.md | 2 + src/Proxy/ClassProxyGenerator.php | 19 +++-- src/Proxy/Generator/ClassGenerator.php | 6 +- .../Transformer/WeavingTransformerTest.php | 72 +++++++++++++++++++ .../_files/global-parent-class-proxy.php | 27 +++++++ .../_files/global-parent-class-woven.php | 12 ++++ .../_files/global-parent-class.php | 11 +++ tests/Proxy/ClassProxyGeneratorTest.php | 54 ++++++++++++++ tests/Proxy/Generator/ClassGeneratorTest.php | 10 +++ tests/Stubs/GlobalParentCollection.php | 26 +++++++ 10 files changed, 232 insertions(+), 7 deletions(-) create mode 100644 tests/Instrument/Transformer/_files/global-parent-class-proxy.php create mode 100644 tests/Instrument/Transformer/_files/global-parent-class-woven.php create mode 100644 tests/Instrument/Transformer/_files/global-parent-class.php create mode 100644 tests/Stubs/GlobalParentCollection.php diff --git a/src/Proxy/AGENTS.md b/src/Proxy/AGENTS.md index 72ca8376..bb0a3f0a 100644 --- a/src/Proxy/AGENTS.md +++ b/src/Proxy/AGENTS.md @@ -4,6 +4,8 @@ - ClassProxyGenerator — trait-based proxy for regular classes - Constructor takes $traitName (FooOriginalTrait FQCN) as 2nd arg - Always emits `use $traitName` (even for introduction-only aspects) + - Parent and interfaces from reflection are always rooted (`\`); only the OriginalTrait name is a deliberate short + relative name (#759) - FunctionProxyGenerator — function wrappers - TraitProxyGenerator — trait proxies - EnumProxyGenerator — enum proxies (trait extraction + case re-declaration) diff --git a/src/Proxy/ClassProxyGenerator.php b/src/Proxy/ClassProxyGenerator.php index 09ba7b02..076f4b36 100644 --- a/src/Proxy/ClassProxyGenerator.php +++ b/src/Proxy/ClassProxyGenerator.php @@ -119,14 +119,15 @@ public function __construct( $propertyAdvices = $classAdviceNames[AspectContainer::PROPERTY_PREFIX] ?? []; $interceptedMethods = array_keys($dynamicMethodAdvices + $staticMethodAdvices); $interceptedProperties = array_keys($propertyAdvices); - $introducedInterfaces = array_values(array_filter( + // Introduced names are class names, root them so a global name never resolves in the proxy namespace + $introducedInterfaces = array_map(self::rootClassName(...), array_values(array_filter( $classAdviceNames[AspectContainer::INTRODUCTION_INTERFACE_PREFIX]['root'] ?? [], is_string(...), - )); - $introducedTraits = array_values(array_filter( + ))); + $introducedTraits = array_map(self::rootClassName(...), array_values(array_filter( $classAdviceNames[AspectContainer::INTRODUCTION_TRAIT_PREFIX]['root'] ?? [], is_string(...), - )); + ))); $staticInitializationAdvices = $classAdviceNames[AspectContainer::STATIC_INIT_PREFIX]['root'] ?? []; $initializationAdvices = $classAdviceNames[AspectContainer::INIT_PREFIX]['root'] ?? []; @@ -184,7 +185,7 @@ public function __construct( // Proxy parent = original class parent (not the trait — there is no inheritance layer) $parentClass = $originalClass->getParentClass(); - $parentClassName = $parentClass !== false ? $parentClass->getName() : null; + $parentClassName = $parentClass !== false ? '\\' . $parentClass->getName() : null; // Proxy modifiers: preserve final/abstract/readonly from original class. $modifiers = []; @@ -570,4 +571,12 @@ protected static function shortClassName(string $className): string return $lastSeparator === false ? $className : substr($className, $lastSeparator + 1); } + + /** + * Returns the class name rooted with a leading backslash, so that it is emitted fully qualified + */ + private static function rootClassName(string $className): string + { + return '\\' . ltrim($className, '\\'); + } } diff --git a/src/Proxy/Generator/ClassGenerator.php b/src/Proxy/Generator/ClassGenerator.php index 7a2e8aee..04b919eb 100644 --- a/src/Proxy/Generator/ClassGenerator.php +++ b/src/Proxy/Generator/ClassGenerator.php @@ -108,7 +108,8 @@ public function addTraitAlias(string $traitFqcn, string $methodName, string $ali { $this->traits[] = $traitFqcn; $this->traitAliases[] = [ - 'trait' => ltrim($traitFqcn, '\\'), + // Kept as given: a leading backslash roots the name in classNameNode() + 'trait' => $traitFqcn, 'method' => $methodName, 'alias' => $alias, 'visibility' => $visibility, @@ -122,7 +123,8 @@ public function addTraitAlias(string $traitFqcn, string $methodName, string $ali * short name resolves in the generated class's own namespace (e.g. the * FooOriginalTrait body trait). Global-namespace names coming from * ::class constants are rooted upstream (AdviceMatcher, proxy generators) - * before they reach this generator. + * before they reach this generator; ClassProxyGenerator also roots the + * parent class name it takes from reflection (#759). */ private static function classNameNode(string $name): Name { diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index 97abddac..15325e63 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -500,6 +500,78 @@ public function testWovenEnumWithConstantExpressionCaseValuesWorksAtRuntime(): v } } + /** + * A namespaced class extending a global class must get a proxy that extends the parent fully + * qualified, otherwise the parent resolves in the proxy namespace (issue #759). + */ + public function testWeaverRootsGlobalParentClassOfNamespacedClass(): void + { + $transformer = $this->createTransformerWithAdvices([ + AspectContainer::METHOD_PREFIX => [ + 'hello' => ['advisor.Test\ns1\GlobalParentCollection->hello' => new BeforeInterceptor(static function (): void {})], + ], + ]); + $metadata = $this->loadTestMetadata('global-parent-class'); + $transformer->transform($metadata); + + $actual = $this->normalizeWhitespaces($metadata->source); + $expected = $this->normalizeWhitespaces($this->loadTestMetadata('global-parent-class-woven')->source); + $this->assertEquals($expected, $actual); + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $actual, $matches)); + + $proxyContent = (string) file_get_contents('vfs://' . $matches[1]); + $this->assertEquals( + $this->normalizeWhitespaces($this->loadTestMetadata('global-parent-class-proxy')->source), + $this->normalizeWhitespaces($proxyContent), + ); + $this->assertPhpCompiles($proxyContent); + } + + /** + * Functional check for issue #759: the woven trait plus the generated proxy must load, and the proxy + * must keep extending the global parent class. + */ + public function testWovenClassWithGlobalParentClassWorksAtRuntime(): void + { + $transformer = $this->createTransformerWithAdvices([ + AspectContainer::METHOD_PREFIX => [ + 'hello' => ['advisor.Test\ns1\GlobalParentCollection->hello' => new BeforeInterceptor(static function (): void {})], + ], + ]); + $metadata = $this->loadTestMetadata('global-parent-class'); + $transformer->transform($metadata); + + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $metadata->source, $matches)); + $proxyContent = file_get_contents('vfs://' . $matches[1]); + + // The woven trait source, without the include_once tail (the proxy is included manually) + $traitSource = preg_replace('/^include_once AOP_CACHE_DIR.*$/m', '', $metadata->source); + + $runtimeDir = static::$fileSystem->path('/runtime'); + if (!is_dir($runtimeDir)) { + mkdir($runtimeDir, 0777, true); + } + $traitFile = $runtimeDir . '/aop_global_parent_trait.php'; + $proxyFile = $runtimeDir . '/aop_global_parent_proxy.php'; + try { + file_put_contents($traitFile, $traitSource); + file_put_contents($proxyFile, $proxyContent); + include $traitFile; + include $proxyFile; + + // The class only exists after the runtime includes above + $className = 'Test\\ns1\\GlobalParentCollection'; + $this->assertTrue(class_exists($className, false)); + $this->assertTrue(is_subclass_of($className, \ArrayObject::class)); + $instance = new $className(['a' => 1]); + $this->assertCount(1, $instance); + $this->assertSame(1, $instance['a']); + } finally { + unlink($traitFile); + unlink($proxyFile); + } + } + /** * PHP 8.3 #[\Override] attribute must be stripped from intercepted methods. * diff --git a/tests/Instrument/Transformer/_files/global-parent-class-proxy.php b/tests/Instrument/Transformer/_files/global-parent-class-proxy.php new file mode 100644 index 00000000..7a359f1f --- /dev/null +++ b/tests/Instrument/Transformer/_files/global-parent-class-proxy.php @@ -0,0 +1,27 @@ + $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod( + self::class, + 'hello', + [ + Interceptor::before(The::advice('advisor.Test\ns1\GlobalParentCollection->hello')), + ], + $this->helloOriginalAlias(...), + ); + return $__joinPoint->__invoke($this); + } +} diff --git a/tests/Instrument/Transformer/_files/global-parent-class-woven.php b/tests/Instrument/Transformer/_files/global-parent-class-woven.php new file mode 100644 index 00000000..ecd11d32 --- /dev/null +++ b/tests/Instrument/Transformer/_files/global-parent-class-woven.php @@ -0,0 +1,12 @@ +assertStringNotContainsString('use FirstOriginalTrait {', $output); } + /** + * A namespaced class extending a global class: the parent from reflection has no leading backslash + * and must be emitted fully qualified, otherwise it resolves in the proxy namespace (issue #759). + * The original body trait keeps its deliberate short name. + * + * @throws ReflectionException + */ + public function testParentFromGlobalNamespaceIsFullyQualified(): void + { + $reflectionClass = new ReflectionClass(GlobalParentCollection::class); + $classAdvices = [ + 'method' => [ + 'hello' => [self::testAdvice()], + ], + ]; + + $generator = new ClassProxyGenerator($reflectionClass, 'Go\\Stubs\\GlobalParentCollectionOriginalTrait', $classAdvices); + $output = "generate(); + + $this->assertStringContainsString('class GlobalParentCollection extends \\ArrayObject implements', $output); + $this->assertStringContainsString('use GlobalParentCollectionOriginalTrait {', $output); + $this->assertStringContainsString( + 'GlobalParentCollectionOriginalTrait::hello as private helloOriginalAlias', + $output, + ); + $this->assertPhpCompiles($output); + } + + /** + * A trait or interface introduced from the global namespace must stay fully qualified, whether the + * caller roots the name or not (direct callers may bypass AdviceMatcher, which roots it). + * + * @throws ReflectionException + */ + #[\PHPUnit\Framework\Attributes\TestWith(['GlobalHelperTrait', 'GlobalHelperInterface'])] + #[\PHPUnit\Framework\Attributes\TestWith(['\\GlobalHelperTrait', '\\GlobalHelperInterface'])] + public function testIntroducedNamesFromGlobalNamespaceAreFullyQualified(string $trait, string $interface): void + { + $reflectionClass = new ReflectionClass(GlobalParentCollection::class); + $classAdvices = [ + 'interface' => ['root' => [$interface]], + 'trait' => ['root' => [$trait]], + ]; + + $generator = new ClassProxyGenerator($reflectionClass, 'Go\\Stubs\\GlobalParentCollectionOriginalTrait', $classAdvices); + $output = "generate(); + + $this->assertStringContainsString('use GlobalParentCollectionOriginalTrait, \\GlobalHelperTrait;', $output); + $this->assertStringContainsString(', \\GlobalHelperInterface', $output); + $this->assertStringNotContainsString(' GlobalHelperInterface', $output); + $this->assertPhpCompiles($output); + } + private static function testAdvice(): GeneratedInterceptor { return GeneratedInterceptor::fromAdvice('test', new BeforeInterceptor(static function (): void {})); diff --git a/tests/Proxy/Generator/ClassGeneratorTest.php b/tests/Proxy/Generator/ClassGeneratorTest.php index 9bac400e..b7328316 100644 --- a/tests/Proxy/Generator/ClassGeneratorTest.php +++ b/tests/Proxy/Generator/ClassGeneratorTest.php @@ -102,6 +102,16 @@ public function testUsesExplicitlyRootedGlobalTraitStaysFullyQualified(): void $this->assertStringNotContainsString('\MyClassOriginalTrait', $output); } + public function testTraitAliasOfExplicitlyRootedGlobalTraitStaysFullyQualified(): void + { + $gen = new ClassGenerator('MyClass', 'My\Namespace', [], null); + $gen->addTraitAlias('\GlobalHelperTrait', 'help', 'helpOriginalAlias', Visibility::Private); + $output = $gen->generate(); + $this->assertStringContainsString('use \GlobalHelperTrait {', $output); + $this->assertStringContainsString('\GlobalHelperTrait::help as private helpOriginalAlias;', $output); + $this->assertStringNotContainsString(' GlobalHelperTrait::help', $output); + } + public function testWithMethod(): void { $method = MethodGenerator::fromReflection(new ReflectionMethod( diff --git a/tests/Stubs/GlobalParentCollection.php b/tests/Stubs/GlobalParentCollection.php new file mode 100644 index 00000000..eb35d002 --- /dev/null +++ b/tests/Stubs/GlobalParentCollection.php @@ -0,0 +1,26 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Stubs; + +/** + * Namespaced class extending a class from the global namespace (issue #759) + * + * @extends \ArrayObject + */ +class GlobalParentCollection extends \ArrayObject +{ + public function hello(): string + { + return 'hello'; + } +}