From 567ddfd83d3aca06052ce4c7844f916bf5da23c9 Mon Sep 17 00:00:00 2001 From: Alexander Lisachenko Date: Tue, 12 May 2026 00:12:41 +0300 Subject: [PATCH] Remove unnecessary FQDN in generated proxy trait adoption When the generated proxy and the trait being adopted share the same namespace, use the short (unqualified) trait name instead of the fully-qualified domain name. This makes generated proxy code cleaner and more readable. Also makes TraitGenerator consistently wrap cross-namespace names in Name\FullyQualified, matching ClassGenerator and EnumGenerator behavior. Closes #572 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Proxy/ClassProxyGenerator.php | 11 +++- src/Proxy/EnumProxyGenerator.php | 11 +++- src/Proxy/Generator/TraitGenerator.php | 9 +++- src/Proxy/TraitProxyGenerator.php | 9 ++-- .../Transformer/_files/class-proxy.php | 16 +++--- .../_files/final-readonly-class-proxy.php | 8 +-- .../Transformer/_files/php7-class-proxy.php | 36 ++++++------- .../Transformer/_files/php81-enum-proxy.php | 4 +- .../_files/php83-override-proxy.php | 6 +-- tests/Proxy/ClassProxyGeneratorTest.php | 52 +++++++++++++++++++ tests/Proxy/EnumProxyGeneratorTest.php | 48 +++++++++++++++++ tests/Proxy/TraitProxyGeneratorTest.php | 48 +++++++++++++++++ 12 files changed, 214 insertions(+), 44 deletions(-) diff --git a/src/Proxy/ClassProxyGenerator.php b/src/Proxy/ClassProxyGenerator.php index c0c21068..3892bf4e 100644 --- a/src/Proxy/ClassProxyGenerator.php +++ b/src/Proxy/ClassProxyGenerator.php @@ -155,10 +155,17 @@ public function __construct( $classGenerator->addAttributeGroups($classAttrGroups); } + // Use the short (unqualified) trait name only when the trait and proxy + // share the same namespace; otherwise keep the FQCN + $lastBackslash = strrpos($traitName, '\\'); + $traitNamespace = $lastBackslash !== false ? substr($traitName, 0, $lastBackslash) : ''; + $sameNamespace = $traitNamespace === $originalClass->getNamespaceName(); + $effectiveTraitName = ($sameNamespace && $lastBackslash !== false) ? substr($traitName, $lastBackslash + 1) : $traitName; + // Always include the original class body trait — even when no methods are intercepted // (e.g. introduction-only aspects). addTraitAlias also registers the trait, so this // explicit addTraits call only matters when $interceptedMethods is empty. - $classGenerator->addTraits([$traitName]); + $classGenerator->addTraits([$effectiveTraitName]); // Alias each intercepted method as private __aop__ foreach ($interceptedMethods as $methodName) { @@ -167,7 +174,7 @@ public function __construct( continue; } - $classGenerator->addTraitAlias($traitName, $methodName, AbstractMethodInvocation::TRAIT_ALIAS_PREFIX . $methodName, ReflectionMethod::IS_PRIVATE); + $classGenerator->addTraitAlias($effectiveTraitName, $methodName, AbstractMethodInvocation::TRAIT_ALIAS_PREFIX . $methodName, ReflectionMethod::IS_PRIVATE); } // Add any AOP-introduced traits $classGenerator->addTraits(array_values($introducedTraits)); diff --git a/src/Proxy/EnumProxyGenerator.php b/src/Proxy/EnumProxyGenerator.php index 6acfc427..024b3ced 100644 --- a/src/Proxy/EnumProxyGenerator.php +++ b/src/Proxy/EnumProxyGenerator.php @@ -141,13 +141,20 @@ public function __construct( $enumGenerator->addEnumCase($caseName, $caseValue); } + // Use the short (unqualified) trait name only when the trait and proxy + // share the same namespace; otherwise keep the FQCN + $lastBackslash = strrpos($traitName, '\\'); + $traitNamespace = $lastBackslash !== false ? substr($traitName, 0, $lastBackslash) : ''; + $sameNamespace = $traitNamespace === $originalClass->getNamespaceName(); + $effectiveTraitName = ($sameNamespace && $lastBackslash !== false) ? substr($traitName, $lastBackslash + 1) : $traitName; + // Always include the original enum body trait - $enumGenerator->addTraits([$traitName]); + $enumGenerator->addTraits([$effectiveTraitName]); // Alias each intercepted method as private __aop__ foreach ($interceptedMethods as $methodName) { $enumGenerator->addTraitAlias( - $traitName, + $effectiveTraitName, $methodName, AbstractMethodInvocation::TRAIT_ALIAS_PREFIX . $methodName, ReflectionMethod::IS_PRIVATE diff --git a/src/Proxy/Generator/TraitGenerator.php b/src/Proxy/Generator/TraitGenerator.php index ba47b38c..b6fb2fde 100644 --- a/src/Proxy/Generator/TraitGenerator.php +++ b/src/Proxy/Generator/TraitGenerator.php @@ -16,6 +16,7 @@ use PhpParser\Comment\Doc; use PhpParser\Modifiers; use PhpParser\Node\Name; +use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Stmt\Property as PropertyNode; use PhpParser\Node\Stmt\Trait_ as TraitNode; use PhpParser\Node\Stmt\TraitUse; @@ -124,7 +125,9 @@ public function getNode(): TraitNode $traitUseAdaptations = []; foreach ($this->traitAliases as $aliasInfo) { $traitUseAdaptations[] = new TraitUseAdaptation\Alias( - new Name($aliasInfo['trait']), + str_contains($aliasInfo['trait'], '\\') + ? new FullyQualified($aliasInfo['trait']) + : new Name($aliasInfo['trait']), new \PhpParser\Node\Identifier($aliasInfo['method']), $this->mapVisibility($aliasInfo['visibility']), new \PhpParser\Node\Identifier($aliasInfo['alias']) @@ -133,7 +136,9 @@ public function getNode(): TraitNode if (!empty($this->usedTraits)) { $traitNames = array_map( - static fn(string $t) => new Name($t), + static fn(string $t) => str_contains($t, '\\') + ? new FullyQualified($t) + : new Name($t), $this->usedTraits ); $builder->addStmt(new TraitUse($traitNames, $traitUseAdaptations)); diff --git a/src/Proxy/TraitProxyGenerator.php b/src/Proxy/TraitProxyGenerator.php index f59ad4ee..697964f8 100644 --- a/src/Proxy/TraitProxyGenerator.php +++ b/src/Proxy/TraitProxyGenerator.php @@ -72,9 +72,12 @@ public function __construct( $generatedProperties ); - // Normalize FQDN for the parent trait reference - $namespaceParts = explode('\\', $parentTraitName); - $parentNormalizedName = end($namespaceParts); + // Use the short (unqualified) trait name only when the parent trait and proxy + // trait share the same namespace; otherwise keep the FQCN + $lastBackslash = strrpos($parentTraitName, '\\'); + $traitNamespace = $lastBackslash !== false ? substr($parentTraitName, 0, $lastBackslash) : ''; + $sameNamespace = $traitNamespace === $originalTrait->getNamespaceName(); + $parentNormalizedName = ($sameNamespace && $lastBackslash !== false) ? substr($parentTraitName, $lastBackslash + 1) : $parentTraitName; $traitGenerator->addTrait($parentNormalizedName); foreach ($interceptedMethods as $methodName) { diff --git a/tests/Instrument/Transformer/_files/class-proxy.php b/tests/Instrument/Transformer/_files/class-proxy.php index 4f3d06a9..375369da 100644 --- a/tests/Instrument/Transformer/_files/class-proxy.php +++ b/tests/Instrument/Transformer/_files/class-proxy.php @@ -6,14 +6,14 @@ use Go\Aop\Intercept\StaticMethodInvocation; class TestClass implements \Go\Aop\Proxy { - use \Test\ns1\TestClass__AopProxied { - \Test\ns1\TestClass__AopProxied::publicMethod as private __aop__publicMethod; - \Test\ns1\TestClass__AopProxied::protectedMethod as private __aop__protectedMethod; - \Test\ns1\TestClass__AopProxied::publicStaticMethod as private __aop__publicStaticMethod; - \Test\ns1\TestClass__AopProxied::protectedStaticMethod as private __aop__protectedStaticMethod; - \Test\ns1\TestClass__AopProxied::publicMethodDynamicArguments as private __aop__publicMethodDynamicArguments; - \Test\ns1\TestClass__AopProxied::publicMethodFixedArguments as private __aop__publicMethodFixedArguments; - \Test\ns1\TestClass__AopProxied::methodWithSpecialTypeArguments as private __aop__methodWithSpecialTypeArguments; + use TestClass__AopProxied { + TestClass__AopProxied::publicMethod as private __aop__publicMethod; + TestClass__AopProxied::protectedMethod as private __aop__protectedMethod; + TestClass__AopProxied::publicStaticMethod as private __aop__publicStaticMethod; + TestClass__AopProxied::protectedStaticMethod as private __aop__protectedStaticMethod; + TestClass__AopProxied::publicMethodDynamicArguments as private __aop__publicMethodDynamicArguments; + TestClass__AopProxied::publicMethodFixedArguments as private __aop__publicMethodFixedArguments; + TestClass__AopProxied::methodWithSpecialTypeArguments as private __aop__methodWithSpecialTypeArguments; } public function publicMethod() { diff --git a/tests/Instrument/Transformer/_files/final-readonly-class-proxy.php b/tests/Instrument/Transformer/_files/final-readonly-class-proxy.php index 1495118c..4156d57c 100644 --- a/tests/Instrument/Transformer/_files/final-readonly-class-proxy.php +++ b/tests/Instrument/Transformer/_files/final-readonly-class-proxy.php @@ -6,10 +6,10 @@ use Go\Aop\Intercept\StaticMethodInvocation; final readonly class TestReadonlyClass implements \Go\Aop\Proxy { - use \Test\ns1\TestReadonlyClass__AopProxied { - \Test\ns1\TestReadonlyClass__AopProxied::publicMethod as private __aop__publicMethod; - \Test\ns1\TestReadonlyClass__AopProxied::anotherMethod as private __aop__anotherMethod; - \Test\ns1\TestReadonlyClass__AopProxied::staticMethod as private __aop__staticMethod; + use TestReadonlyClass__AopProxied { + TestReadonlyClass__AopProxied::publicMethod as private __aop__publicMethod; + TestReadonlyClass__AopProxied::anotherMethod as private __aop__anotherMethod; + TestReadonlyClass__AopProxied::staticMethod as private __aop__staticMethod; } public function publicMethod(): string { diff --git a/tests/Instrument/Transformer/_files/php7-class-proxy.php b/tests/Instrument/Transformer/_files/php7-class-proxy.php index 15d9b5b7..ec77845a 100644 --- a/tests/Instrument/Transformer/_files/php7-class-proxy.php +++ b/tests/Instrument/Transformer/_files/php7-class-proxy.php @@ -5,24 +5,24 @@ use Go\Aop\Intercept\DynamicMethodInvocation; class TestPhp7Class implements \Go\Aop\Proxy { - use \Test\ns1\TestPhp7Class__AopProxied { - \Test\ns1\TestPhp7Class__AopProxied::stringSth as private __aop__stringSth; - \Test\ns1\TestPhp7Class__AopProxied::floatSth as private __aop__floatSth; - \Test\ns1\TestPhp7Class__AopProxied::boolSth as private __aop__boolSth; - \Test\ns1\TestPhp7Class__AopProxied::intSth as private __aop__intSth; - \Test\ns1\TestPhp7Class__AopProxied::callableSth as private __aop__callableSth; - \Test\ns1\TestPhp7Class__AopProxied::arraySth as private __aop__arraySth; - \Test\ns1\TestPhp7Class__AopProxied::variadicStringSthByRef as private __aop__variadicStringSthByRef; - \Test\ns1\TestPhp7Class__AopProxied::exceptionArg as private __aop__exceptionArg; - \Test\ns1\TestPhp7Class__AopProxied::stringRth as private __aop__stringRth; - \Test\ns1\TestPhp7Class__AopProxied::floatRth as private __aop__floatRth; - \Test\ns1\TestPhp7Class__AopProxied::boolRth as private __aop__boolRth; - \Test\ns1\TestPhp7Class__AopProxied::intRth as private __aop__intRth; - \Test\ns1\TestPhp7Class__AopProxied::callableRth as private __aop__callableRth; - \Test\ns1\TestPhp7Class__AopProxied::arrayRth as private __aop__arrayRth; - \Test\ns1\TestPhp7Class__AopProxied::exceptionRth as private __aop__exceptionRth; - \Test\ns1\TestPhp7Class__AopProxied::noRth as private __aop__noRth; - \Test\ns1\TestPhp7Class__AopProxied::returnSelf as private __aop__returnSelf; + use TestPhp7Class__AopProxied { + TestPhp7Class__AopProxied::stringSth as private __aop__stringSth; + TestPhp7Class__AopProxied::floatSth as private __aop__floatSth; + TestPhp7Class__AopProxied::boolSth as private __aop__boolSth; + TestPhp7Class__AopProxied::intSth as private __aop__intSth; + TestPhp7Class__AopProxied::callableSth as private __aop__callableSth; + TestPhp7Class__AopProxied::arraySth as private __aop__arraySth; + TestPhp7Class__AopProxied::variadicStringSthByRef as private __aop__variadicStringSthByRef; + TestPhp7Class__AopProxied::exceptionArg as private __aop__exceptionArg; + TestPhp7Class__AopProxied::stringRth as private __aop__stringRth; + TestPhp7Class__AopProxied::floatRth as private __aop__floatRth; + TestPhp7Class__AopProxied::boolRth as private __aop__boolRth; + TestPhp7Class__AopProxied::intRth as private __aop__intRth; + TestPhp7Class__AopProxied::callableRth as private __aop__callableRth; + TestPhp7Class__AopProxied::arrayRth as private __aop__arrayRth; + TestPhp7Class__AopProxied::exceptionRth as private __aop__exceptionRth; + TestPhp7Class__AopProxied::noRth as private __aop__noRth; + TestPhp7Class__AopProxied::returnSelf as private __aop__returnSelf; } public function stringSth(string $arg) { diff --git a/tests/Instrument/Transformer/_files/php81-enum-proxy.php b/tests/Instrument/Transformer/_files/php81-enum-proxy.php index a34b9361..55acd06e 100644 --- a/tests/Instrument/Transformer/_files/php81-enum-proxy.php +++ b/tests/Instrument/Transformer/_files/php81-enum-proxy.php @@ -5,8 +5,8 @@ use Go\Aop\Intercept\DynamicMethodInvocation; enum TestStatus : string implements \Go\Aop\Proxy { - use \Test\ns1\TestStatus__AopProxied { - \Test\ns1\TestStatus__AopProxied::label as private __aop__label; + use TestStatus__AopProxied { + TestStatus__AopProxied::label as private __aop__label; } case Active = 'active'; case Inactive = 'inactive'; diff --git a/tests/Instrument/Transformer/_files/php83-override-proxy.php b/tests/Instrument/Transformer/_files/php83-override-proxy.php index 2644de5c..dddec077 100644 --- a/tests/Instrument/Transformer/_files/php83-override-proxy.php +++ b/tests/Instrument/Transformer/_files/php83-override-proxy.php @@ -10,9 +10,9 @@ */ class TestClassWithOverride implements \Go\Aop\Proxy { - use \Test\ns1\TestClassWithOverride__AopProxied { - \Test\ns1\TestClassWithOverride__AopProxied::overriddenMethod as private __aop__overriddenMethod; - \Test\ns1\TestClassWithOverride__AopProxied::normalMethod as private __aop__normalMethod; + use TestClassWithOverride__AopProxied { + TestClassWithOverride__AopProxied::overriddenMethod as private __aop__overriddenMethod; + TestClassWithOverride__AopProxied::normalMethod as private __aop__normalMethod; } #[\Override] public function overriddenMethod(): string diff --git a/tests/Proxy/ClassProxyGeneratorTest.php b/tests/Proxy/ClassProxyGeneratorTest.php index 96f167c2..5db88cc4 100644 --- a/tests/Proxy/ClassProxyGeneratorTest.php +++ b/tests/Proxy/ClassProxyGeneratorTest.php @@ -520,6 +520,58 @@ public function normalMethod(): void {} ); } + /** + * When the trait and the proxy share the same namespace, the generated use-block + * must reference the trait by its short (unqualified) name, not the FQCN. + * + * @throws ReflectionException + */ + public function testTraitAdoptionUsesShortNameWhenSameNamespace(): void + { + $reflectionClass = new ReflectionClass(First::class); + $classAdvices = [ + 'method' => [ + 'publicMethod' => ['test'], + ], + ]; + + // Trait in the same namespace as the proxy (Go\Stubs) + $traitFqcn = 'Go\\Stubs\\First__AopProxied'; + $generator = new ClassProxyGenerator($reflectionClass, $traitFqcn, $classAdvices, false); + $output = "generate(); + + // Must use the short (unqualified) trait name + $this->assertStringContainsString('use First__AopProxied {', $output); + $this->assertStringContainsString('First__AopProxied::publicMethod as private __aop__publicMethod', $output); + $this->assertStringNotContainsString('\\Go\\Stubs\\First__AopProxied', $output); + } + + /** + * When the trait is in a different namespace from the proxy, the generated use-block + * must keep the FQCN so PHP can resolve the trait correctly. + * + * @throws ReflectionException + */ + public function testTraitAdoptionUsesFqcnWhenDifferentNamespace(): void + { + $reflectionClass = new ReflectionClass(First::class); + $classAdvices = [ + 'method' => [ + 'publicMethod' => ['test'], + ], + ]; + + // Trait in a different namespace from the proxy (proxy is in Go\Stubs) + $traitFqcn = 'Other\\Namespace\\First__AopProxied'; + $generator = new ClassProxyGenerator($reflectionClass, $traitFqcn, $classAdvices, false); + $output = "generate(); + + // Must use the FQCN for the trait name + $this->assertStringContainsString('use \\Other\\Namespace\\First__AopProxied {', $output); + $this->assertStringContainsString('\\Other\\Namespace\\First__AopProxied::publicMethod as private __aop__publicMethod', $output); + $this->assertStringNotContainsString('use First__AopProxied {', $output); + } + /** * Provides list of methods with expected attributes * diff --git a/tests/Proxy/EnumProxyGeneratorTest.php b/tests/Proxy/EnumProxyGeneratorTest.php index fce09025..6d6c8cc1 100644 --- a/tests/Proxy/EnumProxyGeneratorTest.php +++ b/tests/Proxy/EnumProxyGeneratorTest.php @@ -177,6 +177,54 @@ public function testGenerateDoesNotIncludeBuiltinEnumInterfaces(): void $this->assertStringContainsString('\Go\Aop\Proxy', $implementsClause); } + /** + * When the trait and the proxy enum share the same namespace, the generated use-block + * must reference the trait by its short (unqualified) name, not the FQCN. + */ + public function testTraitAdoptionUsesShortNameWhenSameNamespace(): void + { + $reflectionClass = new ReflectionClass(StubBackedEnum::class); + $classAdvices = [ + 'method' => [ + 'label' => ['advisor'], + ], + ]; + + // Trait in the same namespace as the proxy enum (Go\Stubs) + $traitFqcn = 'Go\\Stubs\\StubBackedEnum__AopProxied'; + $generator = new EnumProxyGenerator($reflectionClass, $traitFqcn, $classAdvices, false); + $output = "generate(); + + // Must use the short (unqualified) trait name + $this->assertStringContainsString('use StubBackedEnum__AopProxied {', $output); + $this->assertStringContainsString('StubBackedEnum__AopProxied::label as private __aop__label', $output); + $this->assertStringNotContainsString('\\Go\\Stubs\\StubBackedEnum__AopProxied', $output); + } + + /** + * When the trait is in a different namespace from the proxy enum, the generated use-block + * must keep the FQCN so PHP can resolve the trait correctly. + */ + public function testTraitAdoptionUsesFqcnWhenDifferentNamespace(): void + { + $reflectionClass = new ReflectionClass(StubBackedEnum::class); + $classAdvices = [ + 'method' => [ + 'label' => ['advisor'], + ], + ]; + + // Trait in a different namespace from the proxy enum (proxy is in Go\Stubs) + $traitFqcn = 'Other\\Namespace\\StubBackedEnum__AopProxied'; + $generator = new EnumProxyGenerator($reflectionClass, $traitFqcn, $classAdvices, false); + $output = "generate(); + + // Must use the FQCN for the trait name + $this->assertStringContainsString('use \\Other\\Namespace\\StubBackedEnum__AopProxied {', $output); + $this->assertStringContainsString('\\Other\\Namespace\\StubBackedEnum__AopProxied::label as private __aop__label', $output); + $this->assertStringNotContainsString('use StubBackedEnum__AopProxied {', $output); + } + /** * Built-in enum methods (cases, from, tryFrom) must be filtered out and never intercepted. * They are synthesised by PHP and cannot be aliased via trait use blocks. diff --git a/tests/Proxy/TraitProxyGeneratorTest.php b/tests/Proxy/TraitProxyGeneratorTest.php index d2cdfcce..bfb992e4 100644 --- a/tests/Proxy/TraitProxyGeneratorTest.php +++ b/tests/Proxy/TraitProxyGeneratorTest.php @@ -244,4 +244,52 @@ public function testGenerateTraitWithClassTypedPropertyUsesFullyQualifiedTypeInF $output ); } + + /** + * When the parent trait and the proxy trait share the same namespace, the generated use-block + * must reference the parent trait by its short (unqualified) name, not the FQCN. + */ + public function testTraitAdoptionUsesShortNameWhenSameNamespace(): void + { + $reflectionTrait = new ReflectionClass(TraitAliasProxied::class); + $traitAdvices = [ + 'method' => [ + 'publicMethod' => ['advisor'], + ], + ]; + + // Parent trait in the same namespace as the proxy trait (Go\Stubs) + $parentTraitFqcn = 'Go\\Stubs\\TraitAliasProxied__AopProxied'; + $generator = new TraitProxyGenerator($reflectionTrait, $parentTraitFqcn, $traitAdvices, false); + $output = "generate(); + + // Must use the short (unqualified) parent trait name + $this->assertStringContainsString('use TraitAliasProxied__AopProxied {', $output); + $this->assertStringContainsString('TraitAliasProxied__AopProxied::publicMethod as private __aop__publicMethod', $output); + $this->assertStringNotContainsString('\\Go\\Stubs\\TraitAliasProxied__AopProxied', $output); + } + + /** + * When the parent trait is in a different namespace from the proxy trait, the generated + * use-block must keep the FQCN so PHP can resolve the trait correctly. + */ + public function testTraitAdoptionUsesFqcnWhenDifferentNamespace(): void + { + $reflectionTrait = new ReflectionClass(TraitAliasProxied::class); + $traitAdvices = [ + 'method' => [ + 'publicMethod' => ['advisor'], + ], + ]; + + // Parent trait in a different namespace from the proxy trait (proxy is in Go\Stubs) + $parentTraitFqcn = 'Other\\Namespace\\TraitAliasProxied__AopProxied'; + $generator = new TraitProxyGenerator($reflectionTrait, $parentTraitFqcn, $traitAdvices, false); + $output = "generate(); + + // Must use the FQCN for the parent trait name + $this->assertStringContainsString('use \\Other\\Namespace\\TraitAliasProxied__AopProxied {', $output); + $this->assertStringContainsString('\\Other\\Namespace\\TraitAliasProxied__AopProxied::publicMethod as private __aop__publicMethod', $output); + $this->assertStringNotContainsString('use TraitAliasProxied__AopProxied {', $output); + } }