Skip to content

Commit

Permalink
Fix PropertyHook::getStmts() for set hook
Browse files Browse the repository at this point in the history 
Produce the correct desugaring if the propertyName attribute is
set, and set it in the parser. Otherwise throw an exception.

Fixes #1053.
  • Loading branch information
@nikic
nikic committed Dec 27, 2024
1 parent 8bb4159 commit 6478c5a
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 11 deletions.
9 changes: 6 additions & 3 deletions grammar/php.y
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 11 additions & 2 deletions lib/PhpParser/Node/PropertyHook.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -74,8 +77,14 @@ public function getStmts(): ?array {
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 . '"');
}
Expand Down
2 changes: 2 additions & 0 deletions lib/PhpParser/Parser/Php7.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1844,10 +1844,12 @@ protected function initReduceCallbacks(): void {
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)]);
Expand Down
3 changes: 3 additions & 0 deletions lib/PhpParser/Parser/Php8.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1839,10 +1839,12 @@ protected function initReduceCallbacks(): void {
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)]);
Expand Down Expand Up @@ -1995,6 +1997,7 @@ protected function initReduceCallbacks(): void {
$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)]);
Expand Down
15 changes: 15 additions & 0 deletions lib/PhpParser/ParserAbstract.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
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 {
Expand Down Expand Up @@ -1201,6 +1202,20 @@ protected function checkPropertyHookModifiers(int $a, int $b, int $modifierPos):
}
}

/**
* @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<Node\Arg|Node\VariadicPlaceholder> $args */
private function isSimpleExit(array $args): bool {
if (\count($args) === 0) {
Expand Down
45 changes: 39 additions & 6 deletions test/PhpParser/Node/PropertyHookTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -40,17 +45,45 @@ public function testGetStmts(): void {
$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 testGetStmtsSetHookFromParser(): void {
$parser = (new ParserFactory())->createForNewestSupportedVersion();
$prettyPrinter = new Standard();
$stmts = $parser->parse(<<<'CODE'
<?php
class Test {
public $prop1 { set => 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 testSetStmtsUnknownHook(): void {
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();
}
}

0 comments on commit 6478c5a

Please sign in to comment.