diff --git a/src/Instrument/Transformer/ConstructorExecutionTransformer.php b/src/Instrument/Transformer/ConstructorExecutionTransformer.php index 0c6e506c..4ffa4c72 100644 --- a/src/Instrument/Transformer/ConstructorExecutionTransformer.php +++ b/src/Instrument/Transformer/ConstructorExecutionTransformer.php @@ -14,11 +14,8 @@ use Go\Aop\Framework\ReflectionConstructorInvocation; use Go\Aop\InitializationAware; -use PhpParser\Node; -use PhpParser\Node\Expr\New_; use PhpParser\Node\Name; use PhpParser\NodeTraverser; -use PhpParser\NodeVisitor\FindingVisitor; /** * Transforms the source code to add an ability to intercept new instances creation @@ -57,15 +54,16 @@ public static function getInstance(): self */ public function transform(StreamMetaData $metadata): TransformerResultEnum { - $newExpressionFinder = new FindingVisitor(fn(Node $node) => $node instanceof New_); + // Skips `new` inside constant-expression contexts (parameter defaults, static var + // initializers, attribute arguments, constants, enum cases) — see issue #603. + $newExpressionFinder = new NewExpressionFinderVisitor(); // TODO: move this logic into walkSyntaxTree(Visitor $nodeVistor) method $traverser = new NodeTraverser(); $traverser->addVisitor($newExpressionFinder); $traverser->traverse($metadata->syntaxTree); - /** @var Node\Expr\New_[] $newExpressions */ - $newExpressions = $newExpressionFinder->getFoundNodes(); + $newExpressions = $newExpressionFinder->getFoundNewExpressions(); if (empty($newExpressions)) { return TransformerResultEnum::RESULT_ABSTAIN; diff --git a/src/Instrument/Transformer/NewExpressionFinderVisitor.php b/src/Instrument/Transformer/NewExpressionFinderVisitor.php new file mode 100644 index 00000000..d57649e5 --- /dev/null +++ b/src/Instrument/Transformer/NewExpressionFinderVisitor.php @@ -0,0 +1,104 @@ + + * + * 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\Attribute; +use PhpParser\Node\Const_; +use PhpParser\Node\Expr\New_; +use PhpParser\Node\Param; +use PhpParser\Node\PropertyItem; +use PhpParser\Node\StaticVar; +use PhpParser\Node\Stmt\EnumCase; +use PhpParser\NodeVisitorAbstract; + +/** + * Finds all `new` expressions that are legal to rewrite into runtime interceptor calls. + * + * Since PHP 8.1 `new` may appear inside constant-expression contexts: parameter default + * values, static variable initializers, attribute arguments and global constants (and + * php-parser also accepts it in property/class-constant defaults and enum case values). + * Such occurrences must stay untouched — the interceptor rewrite + * `...getInstance()->{Foo::class}(...)` is not a valid constant expression and would + * trigger a compile-time fatal error (https://github.com/goaop/framework/issues/603). + */ +final class NewExpressionFinderVisitor extends NodeVisitorAbstract +{ + /** + * @var list + */ + private array $newExpressions = []; + + /** + * Object ids of subtree roots that are constant-expression contexts + * + * @var array + */ + private array $constExprRoots = []; + + /** + * How deep the traversal currently is inside constant-expression subtrees + */ + private int $constExprDepth = 0; + + /** + * @return list All found `new` expressions outside of constant-expression contexts + */ + public function getFoundNewExpressions(): array + { + return $this->newExpressions; + } + + public function enterNode(Node $node): null + { + if ($node instanceof Attribute) { + // Attribute arguments are always constant expressions — skip the whole attribute. + // Property hooks on promoted parameters contain runtime code, so for the other + // containers only the initializer child expression is marked, not the container. + $this->constExprRoots[spl_object_id($node)] = true; + } else { + $constExpr = match (true) { + $node instanceof Param => $node->default, + $node instanceof StaticVar => $node->default, + $node instanceof PropertyItem => $node->default, + $node instanceof Const_ => $node->value, + $node instanceof EnumCase => $node->expr, + default => null, + }; + if ($constExpr !== null) { + $this->constExprRoots[spl_object_id($constExpr)] = true; + } + } + + if (isset($this->constExprRoots[spl_object_id($node)])) { + ++$this->constExprDepth; + } + + if ($this->constExprDepth === 0 && $node instanceof New_) { + $this->newExpressions[] = $node; + } + + return null; + } + + public function leaveNode(Node $node): null + { + $nodeId = spl_object_id($node); + if (isset($this->constExprRoots[$nodeId])) { + --$this->constExprDepth; + unset($this->constExprRoots[$nodeId]); + } + + return null; + } +} diff --git a/src/Instrument/Transformer/WeavingTransformer.php b/src/Instrument/Transformer/WeavingTransformer.php index f18a05f7..4e11eaea 100644 --- a/src/Instrument/Transformer/WeavingTransformer.php +++ b/src/Instrument/Transformer/WeavingTransformer.php @@ -30,6 +30,8 @@ use Go\Proxy\EnumProxyGenerator; use Go\Proxy\FunctionProxyGenerator; use Go\Proxy\TraitProxyGenerator; +use PhpParser\Node\Param; +use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\EnumCase; use PhpParser\Node\Stmt\Property; use ReflectionProperty; @@ -205,7 +207,7 @@ private function adjustOriginalTrait( string $newClassName ): void { $classNode = $class->getNode(); - $position = $classNode->getAttribute('startTokenPos'); + $position = $this->getPositionAfterAttributeGroups($classNode); if (!is_int($position)) { return; } @@ -223,6 +225,32 @@ private function adjustOriginalTrait( } while (true); } + /** + * Returns the token position where the class/enum declaration scan should start. + * + * A ClassLike node's startTokenPos includes its attribute groups (`#[...]`), so scanning + * from there would rename the first T_STRING inside the attribute to the trait name and + * then delete the real class header (see https://github.com/goaop/framework/issues/598). + * Class-level attributes are kept as-is on the generated trait — attributes are legal + * on traits — so the scan starts right after the last attribute group. + */ + private function getPositionAfterAttributeGroups(ClassLike $classNode): ?int + { + $position = $classNode->getAttribute('startTokenPos'); + if (!is_int($position)) { + return null; + } + $lastAttrGroup = end($classNode->attrGroups); + if ($lastAttrGroup !== false) { + $attrGroupsEnd = $lastAttrGroup->getAttribute('endTokenPos'); + if (is_int($attrGroupsEnd)) { + $position = $attrGroupsEnd + 1; + } + } + + return $position; + } + /** * Convert a regular class declaration into a trait for the trait-based AOP engine. * @@ -241,7 +269,7 @@ private function convertClassToTrait( string $newClassName ): void { $classNode = $class->getNode(); - $position = $classNode->getAttribute('startTokenPos'); + $position = $this->getPositionAfterAttributeGroups($classNode); if (!is_int($position)) { return; } @@ -327,7 +355,7 @@ private function convertEnumToTrait( string $newClassName ): void { $classNode = $class->getNode(); - $position = $classNode->getAttribute('startTokenPos'); + $position = $this->getPositionAfterAttributeGroups($classNode); if (!is_int($position)) { return; } @@ -586,6 +614,7 @@ private function commentOutInterceptedPropertiesInTraitBody( } $mask = ReflectionProperty::IS_PUBLIC | ReflectionProperty::IS_PROTECTED | ReflectionProperty::IS_PRIVATE; + $promotedAssignments = []; foreach ($class->getProperties($mask) as $property) { if (!isset($interceptedProperties[$property->getName()])) { continue; @@ -598,6 +627,15 @@ private function commentOutInterceptedPropertiesInTraitBody( if (!is_object($propertyNode) || !method_exists($propertyNode, 'getAttribute')) { continue; } + if ($propertyNode instanceof Param) { + // Promoted constructor property (issue #599): the declaration cannot be commented + // out — it doubles as the constructor parameter. Demote it to a plain parameter + // instead and assign it to the (proxy-declared) property in the constructor body. + $this->demotePromotedPropertyParameter($propertyNode, $streamMetaData); + $propertyName = $property->getName(); + $promotedAssignments[] = sprintf('$this->%1$s = $%1$s;', $propertyName); + continue; + } $start = $propertyNode->getAttribute('startTokenPos'); $end = $propertyNode->getAttribute('endTokenPos'); if (!is_int($start) || !is_int($end)) { @@ -605,6 +643,149 @@ private function commentOutInterceptedPropertiesInTraitBody( } $this->commentOutMovedPropertyTokenRange($class->name, $property->getName(), $start, $end, $streamMetaData); } + + if ($promotedAssignments !== []) { + $this->injectConstructorAssignments($class, $promotedAssignments, $streamMetaData); + } + } + + /** + * Demotes a promoted constructor property to a plain constructor parameter (issue #599). + * + * Removes only the promotion modifiers (visibility, asymmetric set-visibility, readonly, + * final) from the parameter tokens, keeping attributes, type, name, and default value. + * The property itself is re-declared with interception hooks in the proxy class, and the + * value is assigned in the constructor body (see injectConstructorAssignments), which + * routes the write through the proxy's set hook. + * + * Whitespace containing newlines is preserved so line numbers stay intact. + */ + private function demotePromotedPropertyParameter(Param $parameterNode, StreamMetaData $streamMetaData): void + { + $start = $parameterNode->getAttribute('startTokenPos'); + $end = $parameterNode->getAttribute('endTokenPos'); + if (!is_int($start) || !is_int($end)) { + return; + } + + $modifierTokenIds = [ + T_PUBLIC, T_PROTECTED, T_PRIVATE, + T_PUBLIC_SET, T_PROTECTED_SET, T_PRIVATE_SET, + T_READONLY, T_FINAL, + ]; + + $position = $start; + while ($position <= $end) { + if (!isset($streamMetaData->tokenStream[$position])) { + ++$position; + continue; + } + $token = $streamMetaData->tokenStream[$position]; + // Modifiers can only appear before the parameter variable — stop there so that + // tokens inside the default value expression are never touched + if ($token->id === T_VARIABLE) { + break; + } + // Skip parameter attribute groups entirely: '#[' opens a bracket context that can + // contain arbitrary nested brackets inside attribute arguments + if ($token->id === T_ATTRIBUTE) { + $bracketDepth = 1; + ++$position; + while ($position <= $end && $bracketDepth > 0) { + $innerText = isset($streamMetaData->tokenStream[$position]) ? $streamMetaData->tokenStream[$position]->text : ''; + if ($innerText === '[') { + ++$bracketDepth; + } elseif ($innerText === ']') { + --$bracketDepth; + } + ++$position; + } + continue; + } + if (in_array($token->id, $modifierTokenIds, true)) { + unset($streamMetaData->tokenStream[$position]); + // Also drop the following whitespace unless it holds a newline (line budget) + if (isset($streamMetaData->tokenStream[$position + 1])) { + $nextToken = $streamMetaData->tokenStream[$position + 1]; + if ($nextToken->id === T_WHITESPACE && strpbrk($nextToken->text, "\r\n") === false) { + unset($streamMetaData->tokenStream[$position + 1]); + } + } + } + ++$position; + } + } + + /** + * Injects property assignments at the very beginning of the constructor body. + * + * The assignments are appended to the opening '{' token of the constructor body, all on + * the same line, so the original line numbers of the constructor statements are preserved. + * + * @param non-empty-list $assignments Assignment statements like '$this->name = $name;' + */ + private function injectConstructorAssignments( + ReflectionClass $class, + array $assignments, + StreamMetaData $streamMetaData + ): void { + $constructor = $class->getConstructor(); + if ($constructor === null || !$constructor instanceof ReflectionMethod) { + return; + } + $constructorNode = $constructor->getNode(); + $start = $constructorNode->getAttribute('startTokenPos'); + $end = $constructorNode->getAttribute('endTokenPos'); + if (!is_int($start) || !is_int($end)) { + return; + } + + // The body '{' is the first '{' token after the parameter list closes (parenthesis + // depth back to zero). Hook bodies of promoted parameters contain '{' too, but they + // are always nested inside the parameter parentheses, so the depth guard skips them. + $position = $start; + $seenFunction = false; + $seenParameterList = false; + $parenthesisDepth = 0; + while ($position <= $end) { + if (!isset($streamMetaData->tokenStream[$position])) { + ++$position; + continue; + } + $token = $streamMetaData->tokenStream[$position]; + if (!$seenFunction) { + // Skip attribute groups before the 'function' keyword — their arguments + // may contain arbitrary parentheses + if ($token->id === T_ATTRIBUTE) { + $bracketDepth = 1; + ++$position; + while ($position <= $end && $bracketDepth > 0) { + $innerText = isset($streamMetaData->tokenStream[$position]) ? $streamMetaData->tokenStream[$position]->text : ''; + if ($innerText === '[') { + ++$bracketDepth; + } elseif ($innerText === ']') { + --$bracketDepth; + } + ++$position; + } + continue; + } + $seenFunction = ($token->id === T_FUNCTION); + ++$position; + continue; + } + if ($token->text === '(') { + ++$parenthesisDepth; + $seenParameterList = true; + } elseif ($token->text === ')') { + --$parenthesisDepth; + } elseif ($token->text === '{' && $seenParameterList && $parenthesisDepth === 0) { + $streamMetaData->tokenStream[$position]->text .= ' ' . implode(' ', $assignments); + + return; + } + ++$position; + } } private function commentOutMovedPropertyTokenRange( diff --git a/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php b/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php index 951ca2b8..93b2cfc4 100644 --- a/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php +++ b/src/Proxy/Part/AbstractInterceptedPropertyGenerator.php @@ -55,31 +55,29 @@ protected function createBasePropertyGenerator(): PropertyGenerator if ($this->property->hasType()) { $generator->setType(TypeGenerator::fromReflectionType($this->property->getType())); } - if ($this->property->hasDefaultValue()) { - // When parser-reflection is loaded, prefer the raw AST default node over - // getDefaultValue(). This avoids parser-reflection bugs where getDefaultValue() - // crashes (uninitialized typed property for FCC) or returns null (Closure defaults), - // and correctly handles scalars, arrays, and FCC expressions uniformly. - if (method_exists($this->property, 'getNode')) { - $astNode = $this->property->getNode(); - $astDefault = ($astNode instanceof PropertyItem || $astNode instanceof Param) - ? $astNode->default - : null; - if ($astDefault !== null) { - $generator->setDefaultExpressionNode($astDefault); - } - } else { - $rawDefault = $this->property->getDefaultValue(); - if ($rawDefault instanceof \Closure) { - throw new \LogicException(sprintf( - 'Cannot generate proxy for property %s::$%s: PHP 8.5 Closure default values ' - . 'require goaop/parser-reflection for AST access.', - $this->property->getDeclaringClass()->getName(), - $this->property->getName() - )); - } - $generator->setDefaultValue($rawDefault); + // When parser-reflection is loaded, prefer the raw AST default node over + // getDefaultValue(). This avoids parser-reflection bugs where getDefaultValue() + // crashes (uninitialized typed property for FCC) or returns null (Closure defaults), + // and correctly handles scalars, arrays, and FCC expressions uniformly. + // The AST node is also the only source for promoted constructor property defaults: + // reflection reports hasDefaultValue() === false for them because the default + // formally belongs to the constructor parameter (see issue #599), but the proxy + // property must carry it so that the demoted woven trait plus proxy behave like + // the original promoted declaration. + $astDefault = $this->getAstDefaultNode(); + if ($astDefault !== null) { + $generator->setDefaultExpressionNode($astDefault); + } elseif ($this->property->hasDefaultValue() && !method_exists($this->property, 'getNode')) { + $rawDefault = $this->property->getDefaultValue(); + if ($rawDefault instanceof \Closure) { + throw new \LogicException(sprintf( + 'Cannot generate proxy for property %s::$%s: PHP 8.5 Closure default values ' + . 'require goaop/parser-reflection for AST access.', + $this->property->getDeclaringClass()->getName(), + $this->property->getName() + )); } + $generator->setDefaultValue($rawDefault); } $attributeGroups = AttributeGroupsGenerator::fromReflector($this->property); @@ -109,7 +107,27 @@ protected function isArrayTypedProperty(): bool protected function hasPotentiallyUninitializedTypedProperty(): bool { - return $this->property->hasType() && !$this->property->hasDefaultValue(); + return $this->property->hasType() + && !$this->property->hasDefaultValue() + && $this->getAstDefaultNode() === null; + } + + /** + * Returns the raw AST default value expression when parser-reflection is loaded. + * + * For promoted constructor properties the default lives on the Param node while + * reflection's hasDefaultValue() reports false, so the AST node is authoritative. + */ + private function getAstDefaultNode(): ?\PhpParser\Node\Expr + { + if (!method_exists($this->property, 'getNode')) { + return null; + } + $astNode = $this->property->getNode(); + + return ($astNode instanceof PropertyItem || $astNode instanceof Param) + ? $astNode->default + : null; } protected function createFieldAccessDocComment(string $variableName = 'fieldAccess', bool $isNullable = false): Doc diff --git a/tests/Fixtures/project/src/Application/PromotedPropertyClass.php b/tests/Fixtures/project/src/Application/PromotedPropertyClass.php new file mode 100644 index 00000000..a5c88b32 --- /dev/null +++ b/tests/Fixtures/project/src/Application/PromotedPropertyClass.php @@ -0,0 +1,24 @@ +name = trim($this->name); + } + + public function getName(): string + { + return $this->name; + } +} diff --git a/tests/Fixtures/project/src/Application/SingleLinePromotedClass.php b/tests/Fixtures/project/src/Application/SingleLinePromotedClass.php new file mode 100644 index 00000000..357ac7e5 --- /dev/null +++ b/tests/Fixtures/project/src/Application/SingleLinePromotedClass.php @@ -0,0 +1,13 @@ +name)")] + public function beforePromotedNameAccess(FieldAccess $access): void + { + // No-op: registration is asserted by functional tests + } + + #[Pointcut\Before("access(public Go\Tests\TestProject\Application\SingleLinePromotedClass->tag)")] + public function beforePromotedTagAccess(FieldAccess $access): void + { + // No-op: registration is asserted by functional tests + } +} diff --git a/tests/Fixtures/project/src/Kernel/DefaultAspectKernel.php b/tests/Fixtures/project/src/Kernel/DefaultAspectKernel.php index a1f0f291..12a3dfca 100644 --- a/tests/Fixtures/project/src/Kernel/DefaultAspectKernel.php +++ b/tests/Fixtures/project/src/Kernel/DefaultAspectKernel.php @@ -11,6 +11,7 @@ use Go\Tests\TestProject\Aspect\InitializationAspect; use Go\Tests\TestProject\Aspect\Issue293Aspect; use Go\Tests\TestProject\Aspect\LoggingAspect; +use Go\Tests\TestProject\Aspect\PromotedPropertyInterceptAspect; use Go\Tests\TestProject\Aspect\PropertyInterceptAspect; use Go\Tests\TestProject\Aspect\TraitCompositionAspect; use Go\Tests\TestProject\Aspect\WeavingAspect; @@ -27,6 +28,7 @@ protected function configureAop(AspectContainer $container): void $container->registerAspect(DoSomethingAspect::class); $container->registerAspect(ArrayPropertyInterceptAspect::class); $container->registerAspect(PropertyInterceptAspect::class); + $container->registerAspect(PromotedPropertyInterceptAspect::class); $container->registerAspect(Issue293Aspect::class); $container->registerAspect(InitializationAspect::class); $container->registerAspect(WeavingAspect::class); diff --git a/tests/Functional/ClassWeavingTest.php b/tests/Functional/ClassWeavingTest.php index 08fbd2de..85efa9c6 100644 --- a/tests/Functional/ClassWeavingTest.php +++ b/tests/Functional/ClassWeavingTest.php @@ -18,6 +18,8 @@ use Go\Tests\TestProject\Application\FinalClass; use Go\Tests\TestProject\Application\FooInterface; use Go\Tests\TestProject\Application\Main; +use Go\Tests\TestProject\Application\PromotedPropertyClass; +use Go\Tests\TestProject\Application\SingleLinePromotedClass; class ClassWeavingTest extends BaseFunctionalTestCase { @@ -87,6 +89,25 @@ public function testItDoesWeaveMethodWithComplexTypes(): void $this->assertMethodWoven(ClassWithComplexTypes::class, 'publicMethodWithDNFTypeReturn'); } + /** + * Promoted constructor properties must be weavable (issue #599): the promoted parameter + * is demoted to a plain parameter in the woven trait and the property is re-declared + * with interception hooks in the proxy — for multi-line and single-line constructors. + */ + public function testPromotedPropertyWeaving(): void + { + $this->assertPropertyWoven( + PromotedPropertyClass::class, + 'name', + 'Go\\Tests\\TestProject\\Aspect\\PromotedPropertyInterceptAspect->beforePromotedNameAccess' + ); + $this->assertPropertyWoven( + SingleLinePromotedClass::class, + 'tag', + 'Go\\Tests\\TestProject\\Aspect\\PromotedPropertyInterceptAspect->beforePromotedTagAccess' + ); + } + public function testArrayPropertyInterceptionAllowsIndirectModification(): void { $this->assertPropertyWoven( diff --git a/tests/Instrument/Transformer/ConstructorExecutionTransformerTest.php b/tests/Instrument/Transformer/ConstructorExecutionTransformerTest.php index 13ccfd7b..c5a84628 100644 --- a/tests/Instrument/Transformer/ConstructorExecutionTransformerTest.php +++ b/tests/Instrument/Transformer/ConstructorExecutionTransformerTest.php @@ -92,7 +92,41 @@ public static function listOfExpressions(): array [ '$n = new stdClass(new static::$object[0]->name)', '$n = \Go\Instrument\Transformer\ConstructorExecutionTransformer::getInstance()->{stdClass::class}(\Go\Instrument\Transformer\ConstructorExecutionTransformer::getInstance()->{static::$object[0]->name})' - ] + ], + // PHP 8.1 new in initializers (issue #603): `new` inside constant-expression + // contexts must stay untouched — the rewrite is not a valid constant expression. + 'parameter default value' => [ + 'function a($helper = new stdClass("x")) { return $helper; }', + 'function a($helper = new stdClass("x")) { return $helper; }' + ], + 'static variable initializer' => [ + 'function b() { static $memo = new \ArrayObject(); return $memo; }', + 'function b() { static $memo = new \ArrayObject(); return $memo; }' + ], + 'global constant initializer' => [ + 'const GLOBAL_SERVICE = new stdClass', + 'const GLOBAL_SERVICE = new stdClass' + ], + 'attribute argument' => [ + '#[SomeAttr(new stdClass)] function c() {}', + '#[SomeAttr(new stdClass)] function c() {}' + ], + 'parameter default kept while body is still rewritten' => [ + 'function d($helper = new stdClass) { return new stdClass; }', + 'function d($helper = new stdClass) { return \Go\Instrument\Transformer\ConstructorExecutionTransformer::getInstance()->{stdClass::class}; }' + ], + 'static variable kept while body is still rewritten' => [ + 'function e() { static $memo = new stdClass; $memo->x = new stdClass(); return $memo; }', + 'function e() { static $memo = new stdClass; $memo->x = \Go\Instrument\Transformer\ConstructorExecutionTransformer::getInstance()->{stdClass::class}(); return $memo; }' + ], + 'nested new inside parameter default' => [ + 'function f($helper = new stdClass(new stdClass())) {}', + 'function f($helper = new stdClass(new stdClass())) {}' + ], + 'promoted property hook body is still rewritten' => [ + 'class G { public function __construct(public stdClass $h = new stdClass { get { return new stdClass; } }) {} }', + 'class G { public function __construct(public stdClass $h = new stdClass { get { return \Go\Instrument\Transformer\ConstructorExecutionTransformer::getInstance()->{stdClass::class}; } }) {} }' + ], ]; } } diff --git a/tests/Instrument/Transformer/Stubs/FinalPromotedClass85.php b/tests/Instrument/Transformer/Stubs/FinalPromotedClass85.php new file mode 100644 index 00000000..6d59a483 --- /dev/null +++ b/tests/Instrument/Transformer/Stubs/FinalPromotedClass85.php @@ -0,0 +1,24 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\Instrument\Transformer\Stubs; + +/** + * This file uses the PHP 8.5+ `final` promoted constructor property syntax and must + * only be loaded on PHP >= 8.5. Tests referencing it are gated with #[RequiresPhp]. + * + * Used for testing demotion of intercepted promoted properties (issue #599). + */ +class FinalPromotedClass85 +{ + public function __construct(final public string $token = 'secret') {} +} diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index faf812cf..8d36a39f 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -377,6 +377,22 @@ public function testWeaverCopiesNonScalarAttributeArgumentsFromAst(): void $this->assertStringNotContainsString((string) PHP_INT_MAX, $actualProxyContent); } + /** + * Class-level attributes (with and without arguments) must survive the class→trait + * conversion untouched (issue #598). Previously the first token inside `#[...]` was + * renamed to the trait name and the rest of the attribute plus the real class header + * was deleted, producing a parse error like `#[Foo__AopProxied {`. + */ + public function testWeaverKeepsClassLevelAttributesOnWovenTrait(): void + { + $metadata = $this->loadTestMetadata('php80-class-attribute'); + $this->transformer->transform($metadata); + + $actual = $this->normalizeWhitespaces($metadata->source); + $expected = $this->normalizeWhitespaces($this->loadTestMetadata('php80-class-attribute-woven')->source); + $this->assertEquals($expected, $actual); + } + public function testWeaverMovesInterceptedPropertiesToProxyHooks(): void { $adviceMatcher = $this->createMock(AdviceMatcherInterface::class); @@ -427,6 +443,131 @@ public function testWeaverMovesInterceptedPropertiesToProxyHooks(): void $this->assertStringContainsString("InterceptorInjector::forProperty(self::class, 'limited'", $proxyContent); } + /** + * Intercepted promoted constructor properties must be demoted to plain parameters in the + * woven trait — keeping type and default value — with explicit assignments injected at the + * start of the constructor body (issue #599). The proxy re-declares the property with + * interception hooks and must keep the original default value. + */ + public function testWeaverDemotesInterceptedPromotedProperties(): void + { + $transformer = $this->createTransformerWithAdvices([ + AspectContainer::PROPERTY_PREFIX => [ + 'name' => ['advisor.Go\Tests\TestProject\Application\PromotedPropertyClass->name' => true], + 'counter' => ['advisor.Go\Tests\TestProject\Application\PromotedPropertyClass->counter' => true], + ], + AspectContainer::METHOD_PREFIX => [ + '__construct' => ['advisor.Go\Tests\TestProject\Application\PromotedPropertyClass->__construct' => true], + 'getName' => ['advisor.Go\Tests\TestProject\Application\PromotedPropertyClass->getName' => true], + ], + ]); + + $metadata = $this->loadTestMetadata('php80-promoted-property'); + $transformer->transform($metadata); + + $actual = $this->normalizeWhitespaces($metadata->source); + $expected = $this->normalizeWhitespaces($this->loadTestMetadata('php80-promoted-property-woven')->source); + $this->assertEquals($expected, $actual); + + // Non-intercepted promoted parameter must stay promoted in the trait + $this->assertStringContainsString('protected ?\ArrayObject $bag = null', $actual); + + $matches = []; + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $actual, $matches)); + $actualProxyContent = $this->normalizeWhitespaces((string) file_get_contents('vfs://' . $matches[1])); + $expectedProxyContent = $this->normalizeWhitespaces($this->loadTestMetadata('php80-promoted-property-proxy')->source); + $this->assertEquals($expectedProxyContent, $actualProxyContent); + + // The proxy hook property must keep the original promoted default value + $this->assertStringContainsString("private string \$name = 'initial' {", $actualProxyContent); + } + + /** + * A promoted property inside a single-line constructor must weave without a parse error + * (issue #599). Commenting the parameter out used to swallow the closing ')' and '{'. + */ + public function testWeaverDemotesPromotedPropertyInSingleLineConstructor(): void + { + $transformer = $this->createTransformerWithAdvices([ + AspectContainer::PROPERTY_PREFIX => [ + 'tag' => ['advisor.Go\Tests\TestProject\Application\SingleLinePromotedClass->tag' => true], + ], + AspectContainer::METHOD_PREFIX => [ + '__construct' => ['advisor.Go\Tests\TestProject\Application\SingleLinePromotedClass->__construct' => true], + ], + ]); + + $metadata = $this->loadTestMetadata('php80-promoted-property-single-line'); + $transformer->transform($metadata); + + $actual = $this->normalizeWhitespaces($metadata->source); + $expected = $this->normalizeWhitespaces($this->loadTestMetadata('php80-promoted-property-single-line-woven')->source); + $this->assertEquals($expected, $actual); + + $matches = []; + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $actual, $matches)); + $actualProxyContent = $this->normalizeWhitespaces((string) file_get_contents('vfs://' . $matches[1])); + $expectedProxyContent = $this->normalizeWhitespaces($this->loadTestMetadata('php80-promoted-property-single-line-proxy')->source); + $this->assertEquals($expectedProxyContent, $actualProxyContent); + } + + /** + * PHP 8.5 `final` promoted constructor properties must demote cleanly: the woven trait + * drops both `final` and the visibility modifier, while the proxy re-declares the + * property as final with the original default value (issue #599). + */ + #[\PHPUnit\Framework\Attributes\RequiresPhp('>= 8.5.0')] + public function testWeaverDemotesFinalPromotedProperty(): void + { + $transformer = $this->createTransformerWithAdvices([ + AspectContainer::PROPERTY_PREFIX => [ + 'token' => ['advisor.Go\Instrument\Transformer\Stubs\FinalPromotedClass85->token' => true], + ], + AspectContainer::METHOD_PREFIX => [ + '__construct' => ['advisor.Go\Instrument\Transformer\Stubs\FinalPromotedClass85->__construct' => true], + ], + ]); + + $fileName = __DIR__ . '/Stubs/FinalPromotedClass85.php'; + $stream = fopen('php://filter/string.tolower/resource=' . $fileName, 'r'); + $metadata = new StreamMetaData($stream, (string) file_get_contents($fileName)); + fclose($stream); + $transformer->transform($metadata); + + $actual = $this->normalizeWhitespaces($metadata->source); + $this->assertStringContainsString( + "public function __construct(string \$token = 'secret') { \$this->token = \$token;}", + $actual + ); + + $matches = []; + $this->assertSame(1, preg_match("/AOP_CACHE_DIR . '(.+)';$/m", $actual, $matches)); + $proxyContent = $this->normalizeWhitespaces((string) file_get_contents('vfs://' . $matches[1])); + $this->assertStringContainsString("final public string \$token = 'secret' {", $proxyContent); + } + + /** + * Creates a WeavingTransformer whose advice matcher returns the given advices for any class. + */ + private function createTransformerWithAdvices(array $advices): WeavingTransformer + { + $adviceMatcher = $this->createMock(AdviceMatcherInterface::class); + $adviceMatcher->method('getAdvicesForClass')->willReturn($advices); + $adviceMatcher->method('getAdvicesForFunctions')->willReturn([]); + + $loader = $this + ->getMockBuilder(AspectLoader::class) + ->setConstructorArgs([$this->getContainerMock()]) + ->getMock(); + + return new WeavingTransformer( + $this->kernel, + $adviceMatcher, + $this->cachePathManager, + $loader + ); + } + /** * Testcase for multiple classes (@see https://github.com/lisachenko/go-aop-php/issues/71) */ diff --git a/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php b/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php new file mode 100644 index 00000000..097134b1 --- /dev/null +++ b/tests/Instrument/Transformer/_files/php80-class-attribute-woven.php @@ -0,0 +1,29 @@ + $__joinPoint */ + static $__joinPoint = InterceptorInjector::forProperty(self::class, 'name', ['advisor.Go\Tests\TestProject\Application\PromotedPropertyClass->name']); + return $__joinPoint->__invoke($this, FieldAccessType::READ, $this->name); + } + set { + /** @var FieldAccess $__joinPoint */ + static $__joinPoint = InterceptorInjector::forProperty(self::class, 'name', ['advisor.Go\Tests\TestProject\Application\PromotedPropertyClass->name']); + $this->name = $__joinPoint->__invoke($this, FieldAccessType::WRITE, $value, $this->name); + } + } + final public private(set) int $counter = 1 { + get { + /** @var FieldAccess $__joinPoint */ + static $__joinPoint = InterceptorInjector::forProperty(self::class, 'counter', ['advisor.Go\Tests\TestProject\Application\PromotedPropertyClass->counter']); + return $__joinPoint->__invoke($this, FieldAccessType::READ, $this->counter); + } + set { + /** @var FieldAccess $__joinPoint */ + static $__joinPoint = InterceptorInjector::forProperty(self::class, 'counter', ['advisor.Go\Tests\TestProject\Application\PromotedPropertyClass->counter']); + $this->counter = $__joinPoint->__invoke($this, FieldAccessType::WRITE, $value, $this->counter); + } + } + public function __construct(string $name = 'initial', int $counter = 1, ?\ArrayObject $bag = null) + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod(self::class, '__construct', ['advisor.Go\Tests\TestProject\Application\PromotedPropertyClass->__construct'], $this->__aop____construct(...)); + return $__joinPoint->__invoke($this, \array_slice([$name, $counter, $bag], 0, \func_num_args())); + } + public function getName(): string + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod(self::class, 'getName', ['advisor.Go\Tests\TestProject\Application\PromotedPropertyClass->getName'], $this->__aop__getName(...)); + return $__joinPoint->__invoke($this); + } +} \ No newline at end of file diff --git a/tests/Instrument/Transformer/_files/php80-promoted-property-single-line-proxy.php b/tests/Instrument/Transformer/_files/php80-promoted-property-single-line-proxy.php new file mode 100644 index 00000000..6da7cc51 --- /dev/null +++ b/tests/Instrument/Transformer/_files/php80-promoted-property-single-line-proxy.php @@ -0,0 +1,36 @@ + $__joinPoint */ + static $__joinPoint = InterceptorInjector::forProperty(self::class, 'tag', ['advisor.Go\Tests\TestProject\Application\SingleLinePromotedClass->tag']); + return $__joinPoint->__invoke($this, FieldAccessType::READ, $this->tag); + } + set { + /** @var FieldAccess $__joinPoint */ + static $__joinPoint = InterceptorInjector::forProperty(self::class, 'tag', ['advisor.Go\Tests\TestProject\Application\SingleLinePromotedClass->tag']); + $this->tag = $__joinPoint->__invoke($this, FieldAccessType::WRITE, $value, $this->tag); + } + } + public function __construct(string $tag = 'default') + { + /** @var DynamicMethodInvocation $__joinPoint */ + static $__joinPoint = InterceptorInjector::forMethod(self::class, '__construct', ['advisor.Go\Tests\TestProject\Application\SingleLinePromotedClass->__construct'], $this->__aop____construct(...)); + return $__joinPoint->__invoke($this, \array_slice([$tag], 0, \func_num_args())); + } +} \ No newline at end of file diff --git a/tests/Instrument/Transformer/_files/php80-promoted-property-single-line-woven.php b/tests/Instrument/Transformer/_files/php80-promoted-property-single-line-woven.php new file mode 100644 index 00000000..e2195c20 --- /dev/null +++ b/tests/Instrument/Transformer/_files/php80-promoted-property-single-line-woven.php @@ -0,0 +1,14 @@ +tag = $tag;} +} +include_once AOP_CACHE_DIR . '/Transformer/_files/php80-promoted-property-single-line.php'; diff --git a/tests/Instrument/Transformer/_files/php80-promoted-property-single-line.php b/tests/Instrument/Transformer/_files/php80-promoted-property-single-line.php new file mode 100644 index 00000000..357ac7e5 --- /dev/null +++ b/tests/Instrument/Transformer/_files/php80-promoted-property-single-line.php @@ -0,0 +1,13 @@ +name = $name; $this->counter = $counter; + $this->name = trim($this->name); + } + + public function getName(): string + { + return $this->name; + } +} +include_once AOP_CACHE_DIR . '/Transformer/_files/php80-promoted-property.php'; diff --git a/tests/Instrument/Transformer/_files/php80-promoted-property.php b/tests/Instrument/Transformer/_files/php80-promoted-property.php new file mode 100644 index 00000000..a5c88b32 --- /dev/null +++ b/tests/Instrument/Transformer/_files/php80-promoted-property.php @@ -0,0 +1,24 @@ +name = trim($this->name); + } + + public function getName(): string + { + return $this->name; + } +}