Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PropertyHook::getStmts() is wrong for short body set hook #1053

Open
ondrejmirtes opened this issue Dec 11, 2024 · 1 comment
Open

PropertyHook::getStmts() is wrong for short body set hook #1053

ondrejmirtes opened this issue Dec 11, 2024 · 1 comment

Comments

@ondrejmirtes
Copy link
Contributor

Given short body hooks:

class Foo
{
    public $test {
        get => strtolower($this->test);
        set => strtoupper($value);
    }
}

The implementation of PropertyHook::getStmts():

    public function getStmts(): ?array {
        if ($this->body instanceof Expr) {
            return [new Return_($this->body)];
        }
        return $this->body;
    }

Is correct for the get hook, but not for set hook. I consulted this with Ilija (co-author of property hooks) and he showed me this piece of code: https://github.com/php/php-src/blob/6e759e079f882fe54afa6da31f9cc86e9f680bf6/Zend/zend_compile.c#L8492-L8506

Which shows that short body set hook is expanded to $this->test = strtoupper($value);, not return strtoupper($value);.

But the PropertyHook node does not have access to its property name so we can't construct the correct expression in getStmts().

What would you recommend here? It's easy to work around this problem, but it's a bit misleading about how it works. For example, the set hook always has implicit void return type so it's always wrong to "return" something from it.

@nikic
Copy link
Owner

nikic commented Dec 14, 2024

The best I can think of is to add an attribute with the property name that's set by the parser and make the method throw if it's not present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants