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

Consistency of __PROPERTY__ magic constant meaning in hook signature #17222

Open
ondrejmirtes opened this issue Dec 20, 2024 · 2 comments
Open

Comments

@ondrejmirtes
Copy link
Contributor

Description

If I look at what __METHOD__ does, it has meaning in:

  • method attributes
  • default parameter values
  • parameter attributes
  • in method body

Here's a quick proof: https://3v4l.org/EqGLn

When I compare that with __PROPERTY__, it only has meaning in:

  • parameter attributes
  • in hook body

Of course, hook parameter cannot have a default value. But it'd be nice for consistency if it also had meaning in:

  • hook attributes

Which it currently has not. Additionally, it might be up to discussion, if it should have meaning in:

  • default property value
  • property attributes

Here's a quick script to verify: https://3v4l.org/4BMJt#v8.4.2

/cc @iluuu1994 @Crell

PHP Version

PHP 8.4

Operating System

N/A

@Crell
Copy link
Contributor

Crell commented Dec 20, 2024

Huh. I didn't know that __METHOD__ worked in most of those places... I've never used it except in the body. That's probably why it is how it is: We never thought about it. 😄

I'm fine with making it meaningful in the hook attribute, as it's functionally the same as a method. That... could probably be treated as a non-breaking bug fix if we wanted, but that's not my call.

Adding it to property attributes I'd also be in favor of, though I'm not sure if that would also be legal as a bug fix or if it would have to wait for 8.5.

I don't think the property default value is distinct enough from the property itself for it to have its own attributes to begin with, so there's nothing to do there.

Ilija?

@iluuu1994
Copy link
Member

I don't think BC is a concern yet. This can be treated as a bug IMO. Given the behavior of the other pseudo constants, I think all of these cases should work.

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

No branches or pull requests

3 participants