From 6478c5ac534d497e17e010eac1a0385474fe11b9 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 27 Dec 2024 16:55:20 +0100 Subject: [PATCH] Fix PropertyHook::getStmts() for set hook Produce the correct desugaring if the propertyName attribute is set, and set it in the parser. Otherwise throw an exception. Fixes #1053. --- grammar/php.y | 9 +++-- lib/PhpParser/Node/PropertyHook.php | 13 +++++-- lib/PhpParser/Parser/Php7.php | 2 ++ lib/PhpParser/Parser/Php8.php | 3 ++ lib/PhpParser/ParserAbstract.php | 15 ++++++++ test/PhpParser/Node/PropertyHookTest.php | 45 ++++++++++++++++++++---- 6 files changed, 76 insertions(+), 11 deletions(-) diff --git a/grammar/php.y b/grammar/php.y index 2545f7d8..7fe86b26 100644 --- a/grammar/php.y +++ b/grammar/php.y @@ -686,11 +686,13 @@ parameter: optional_attributes optional_property_modifiers optional_type_without_static optional_arg_ref optional_ellipsis plain_variable optional_property_hook_list { $$ = new Node\Param($6, null, $3, $4, $5, attributes(), $2, $1, $7); - $this->checkParam($$); } + $this->checkParam($$); + $this->addPropertyNameToHooks($$); } | optional_attributes optional_property_modifiers optional_type_without_static optional_arg_ref optional_ellipsis plain_variable '=' expr optional_property_hook_list { $$ = new Node\Param($6, $8, $3, $4, $5, attributes(), $2, $1, $9); - $this->checkParam($$); } + $this->checkParam($$); + $this->addPropertyNameToHooks($$); } | optional_attributes optional_property_modifiers optional_type_without_static optional_arg_ref optional_ellipsis error { $$ = new Node\Param(Expr\Error[], null, $3, $4, $5, attributes(), $2, $1); } @@ -841,7 +843,8 @@ class_statement: | optional_attributes variable_modifiers optional_type_without_static property_declaration_list '{' property_hook_list '}' { $$ = new Stmt\Property($2, $4, attributes(), $3, $1, $6); $this->checkPropertyHooksForMultiProperty($$, #5); - $this->checkEmptyPropertyHookList($6, #5); } + $this->checkEmptyPropertyHookList($6, #5); + $this->addPropertyNameToHooks($$); } #endif | optional_attributes method_modifiers T_CONST class_const_list semi { $$ = new Stmt\ClassConst($4, $2, attributes(), $1); diff --git a/lib/PhpParser/Node/PropertyHook.php b/lib/PhpParser/Node/PropertyHook.php index d8a77fe2..349b9cef 100644 --- a/lib/PhpParser/Node/PropertyHook.php +++ b/lib/PhpParser/Node/PropertyHook.php @@ -3,6 +3,9 @@ namespace PhpParser\Node; use PhpParser\Modifiers; +use PhpParser\Node\Expr\Assign; +use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\Return_; use PhpParser\NodeAbstract; @@ -74,8 +77,14 @@ class PropertyHook extends NodeAbstract implements FunctionLike { return [new Return_($this->body)]; } if ($name === 'set') { - // TODO: This should generate $this->prop = $expr, but we don't know the property name. - return [new Expression($this->body)]; + if (!$this->hasAttribute('propertyName')) { + throw new \LogicException( + 'Can only use getStmts() on a "set" hook if the "propertyName" attribute is set'); + } + + $propName = $this->getAttribute('propertyName'); + $prop = new PropertyFetch(new Variable('this'), (string) $propName); + return [new Expression(new Assign($prop, $this->body))]; } throw new \LogicException('Unknown property hook "' . $name . '"'); } diff --git a/lib/PhpParser/Parser/Php7.php b/lib/PhpParser/Parser/Php7.php index 2ab16954..b9097820 100644 --- a/lib/PhpParser/Parser/Php7.php +++ b/lib/PhpParser/Parser/Php7.php @@ -1844,10 +1844,12 @@ class Php7 extends \PhpParser\ParserAbstract 290 => static function ($self, $stackPos) { $self->semValue = new Node\Param($self->semStack[$stackPos-(7-6)], null, $self->semStack[$stackPos-(7-3)], $self->semStack[$stackPos-(7-4)], $self->semStack[$stackPos-(7-5)], $self->getAttributes($self->tokenStartStack[$stackPos-(7-1)], $self->tokenEndStack[$stackPos]), $self->semStack[$stackPos-(7-2)], $self->semStack[$stackPos-(7-1)], $self->semStack[$stackPos-(7-7)]); $self->checkParam($self->semValue); + $self->addPropertyNameToHooks($self->semValue); }, 291 => static function ($self, $stackPos) { $self->semValue = new Node\Param($self->semStack[$stackPos-(9-6)], $self->semStack[$stackPos-(9-8)], $self->semStack[$stackPos-(9-3)], $self->semStack[$stackPos-(9-4)], $self->semStack[$stackPos-(9-5)], $self->getAttributes($self->tokenStartStack[$stackPos-(9-1)], $self->tokenEndStack[$stackPos]), $self->semStack[$stackPos-(9-2)], $self->semStack[$stackPos-(9-1)], $self->semStack[$stackPos-(9-9)]); $self->checkParam($self->semValue); + $self->addPropertyNameToHooks($self->semValue); }, 292 => static function ($self, $stackPos) { $self->semValue = new Node\Param(new Expr\Error($self->getAttributes($self->tokenStartStack[$stackPos-(6-1)], $self->tokenEndStack[$stackPos])), null, $self->semStack[$stackPos-(6-3)], $self->semStack[$stackPos-(6-4)], $self->semStack[$stackPos-(6-5)], $self->getAttributes($self->tokenStartStack[$stackPos-(6-1)], $self->tokenEndStack[$stackPos]), $self->semStack[$stackPos-(6-2)], $self->semStack[$stackPos-(6-1)]); diff --git a/lib/PhpParser/Parser/Php8.php b/lib/PhpParser/Parser/Php8.php index e97914a5..3addf944 100644 --- a/lib/PhpParser/Parser/Php8.php +++ b/lib/PhpParser/Parser/Php8.php @@ -1839,10 +1839,12 @@ class Php8 extends \PhpParser\ParserAbstract 290 => static function ($self, $stackPos) { $self->semValue = new Node\Param($self->semStack[$stackPos-(7-6)], null, $self->semStack[$stackPos-(7-3)], $self->semStack[$stackPos-(7-4)], $self->semStack[$stackPos-(7-5)], $self->getAttributes($self->tokenStartStack[$stackPos-(7-1)], $self->tokenEndStack[$stackPos]), $self->semStack[$stackPos-(7-2)], $self->semStack[$stackPos-(7-1)], $self->semStack[$stackPos-(7-7)]); $self->checkParam($self->semValue); + $self->addPropertyNameToHooks($self->semValue); }, 291 => static function ($self, $stackPos) { $self->semValue = new Node\Param($self->semStack[$stackPos-(9-6)], $self->semStack[$stackPos-(9-8)], $self->semStack[$stackPos-(9-3)], $self->semStack[$stackPos-(9-4)], $self->semStack[$stackPos-(9-5)], $self->getAttributes($self->tokenStartStack[$stackPos-(9-1)], $self->tokenEndStack[$stackPos]), $self->semStack[$stackPos-(9-2)], $self->semStack[$stackPos-(9-1)], $self->semStack[$stackPos-(9-9)]); $self->checkParam($self->semValue); + $self->addPropertyNameToHooks($self->semValue); }, 292 => static function ($self, $stackPos) { $self->semValue = new Node\Param(new Expr\Error($self->getAttributes($self->tokenStartStack[$stackPos-(6-1)], $self->tokenEndStack[$stackPos])), null, $self->semStack[$stackPos-(6-3)], $self->semStack[$stackPos-(6-4)], $self->semStack[$stackPos-(6-5)], $self->getAttributes($self->tokenStartStack[$stackPos-(6-1)], $self->tokenEndStack[$stackPos]), $self->semStack[$stackPos-(6-2)], $self->semStack[$stackPos-(6-1)]); @@ -1995,6 +1997,7 @@ class Php8 extends \PhpParser\ParserAbstract $self->semValue = new Stmt\Property($self->semStack[$stackPos-(7-2)], $self->semStack[$stackPos-(7-4)], $self->getAttributes($self->tokenStartStack[$stackPos-(7-1)], $self->tokenEndStack[$stackPos]), $self->semStack[$stackPos-(7-3)], $self->semStack[$stackPos-(7-1)], $self->semStack[$stackPos-(7-6)]); $self->checkPropertyHooksForMultiProperty($self->semValue, $stackPos-(7-5)); $self->checkEmptyPropertyHookList($self->semStack[$stackPos-(7-6)], $stackPos-(7-5)); + $self->addPropertyNameToHooks($self->semValue); }, 349 => static function ($self, $stackPos) { $self->semValue = new Stmt\ClassConst($self->semStack[$stackPos-(5-4)], $self->semStack[$stackPos-(5-2)], $self->getAttributes($self->tokenStartStack[$stackPos-(5-1)], $self->tokenEndStack[$stackPos]), $self->semStack[$stackPos-(5-1)]); diff --git a/lib/PhpParser/ParserAbstract.php b/lib/PhpParser/ParserAbstract.php index 475b705e..667f21f5 100644 --- a/lib/PhpParser/ParserAbstract.php +++ b/lib/PhpParser/ParserAbstract.php @@ -32,6 +32,7 @@ use PhpParser\Node\Stmt\Nop; use PhpParser\Node\Stmt\Property; use PhpParser\Node\Stmt\TryCatch; use PhpParser\Node\UseItem; +use PhpParser\Node\VarLikeIdentifier; use PhpParser\NodeVisitor\CommentAnnotatingVisitor; abstract class ParserAbstract implements Parser { @@ -1201,6 +1202,20 @@ abstract class ParserAbstract implements Parser { } } + /** + * @param Property|Param $node + */ + protected function addPropertyNameToHooks(Node $node): void { + if ($node instanceof Property) { + $name = $node->props[0]->name->toString(); + } else { + $name = $node->var->name; + } + foreach ($node->hooks as $hook) { + $hook->setAttribute('propertyName', $name); + } + } + /** @param array $args */ private function isSimpleExit(array $args): bool { if (\count($args) === 0) { diff --git a/test/PhpParser/Node/PropertyHookTest.php b/test/PhpParser/Node/PropertyHookTest.php index 755f2574..8e5cf99f 100644 --- a/test/PhpParser/Node/PropertyHookTest.php +++ b/test/PhpParser/Node/PropertyHookTest.php @@ -3,9 +3,14 @@ namespace PhpParser\Node; use PhpParser\Modifiers; +use PhpParser\Node\Expr\Assign; +use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; +use PhpParser\Node\Scalar\Int_; use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\Return_; +use PhpParser\ParserFactory; +use PhpParser\PrettyPrinter\Standard; class PropertyHookTest extends \PHPUnit\Framework\TestCase { /** @@ -40,17 +45,45 @@ class PropertyHookTest extends \PHPUnit\Framework\TestCase { $get = new PropertyHook('get', $expr); $this->assertEquals([new Return_($expr)], $get->getStmts()); - // TODO: This is incorrect. - $set = new PropertyHook('set', $expr); - $this->assertEquals([new Expression($expr)], $set->getStmts()); + $set = new PropertyHook('set', $expr, [], ['propertyName' => 'abc']); + $this->assertEquals([ + new Expression(new Assign(new PropertyFetch(new Variable('this'), 'abc'), $expr)) + ], $set->getStmts()); } - public function testSetStmtsUnknownHook(): void { + public function testGetStmtsSetHookFromParser(): void { + $parser = (new ParserFactory())->createForNewestSupportedVersion(); + $prettyPrinter = new Standard(); + $stmts = $parser->parse(<<<'CODE' + 123; } + + public function __construct(public $prop2 { set => 456; }) {} + } + CODE); + + $hook1 = $stmts[0]->stmts[0]->hooks[0]; + $this->assertEquals('$this->prop1 = 123;', $prettyPrinter->prettyPrint($hook1->getStmts())); + + $hook2 = $stmts[0]->stmts[1]->params[0]->hooks[0]; + $this->assertEquals('$this->prop2 = 456;', $prettyPrinter->prettyPrint($hook2->getStmts())); + } + + public function testGetStmtsUnknownHook(): void { $expr = new Variable('test'); - $get = new PropertyHook('foobar', $expr); + $hook = new PropertyHook('foobar', $expr); $this->expectException(\LogicException::class); $this->expectExceptionMessage('Unknown property hook "foobar"'); - $get->getStmts(); + $hook->getStmts(); + } + + public function testGetStmtsSetHookWithoutPropertyName(): void { + $expr = new Variable('test'); + $set = new PropertyHook('set', $expr); + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Can only use getStmts() on a "set" hook if the "propertyName" attribute is set'); + $set->getStmts(); } }