False positive: PHP8 property promotion attribute target

[dbrekelmans]

Bug report

When using a PHP8 attribute with TARGET_PROPERTY, you should be able to use it on constructor parameters if you're using constructor property promotion.

Instead, you get the error: Attribute class Foo does not have the parameter target.

Code snippet that reproduces the problem

https://phpstan.org/r/17f00d68-0912-4f7f-84fc-6c69fa3683e3

Expected output

No error

[mergeable]

This bug report is missing a link to reproduction on phpstan.org.

It will most likely be closed after manual review.

[ondrejmirtes]

Not true, PHPStan is right. Runtime proof: https://3v4l.org/tBjj2

[dbrekelmans]

Strange, https://wiki.php.net/rfc/constructor_promotion#attributes implies that it should work. Maybe this was overlooked when implementing attributes.

Thanks either way for looking into this!

[dktapps]

To my understanding, this is intentionally not allowed because php-src developers couldn't decide whether attributes on promoted properties should apply to the parameter, the property or both. They chose to disallow it to avoid the problem for the time being. https://externals.io/message/111942

[michaelzangerle]

@ondrejmirtes but it works like this https://3v4l.org/RqJLk. As it's not just a simple parameter but a promoted property it kind of makes sense I guess. WDYT?

[ondrejmirtes]

From attributes RFC:

validation of attributes is only performed during ReflectionAttribute::newInstance()

[michaelzangerle]

@ondrejmirtes I think that I am exactly calling that function (as you did in your example) in the loop? I copied your example and only changed the way you are getting the attributes -> from the properties instead of the parameters. What am I missing?

[ondrejmirtes]

Alright, didn't notice that, time to rethink this a bit maybe :)

[derrabus]

I ran into this issue today as well. I think, when using constructor property promotion, PHPStan should only check if the annotation is allowed on at least one of the targets "property" and "parameter". If both targets are disallowed, the error is valid.

When using CPP, I cannot specify that the attribute should only be applied to the property. If I wanted to please PHPStan, I would have to specify the property in the traditional way. That would be unfortunate, imho.

[iquito]

I have run into this error as well when I recently started using constructor property promotion for validation classes, and the missing parameter target is never a big deal for the code/application itself - even when using parameter attributes in constructors with constructor property promotion (or intermixing them wildly, if that would be useful in the future), any library would only instantiate their own attributes and can easily avoid this fatal error, so it seems unlikely that a real problem would occur in the specific case of constructor property promotion. Tolerating this in PHPStan would be great.

[dbrekelmans]

@ondrejmirtes Would you be able to provide some pointers? Maybe knowing where to look I might be able to submit a PR for this

[ondrejmirtes]

@dbrekelmans The logic is here: https://github.com/phpstan/phpstan-src/blob/master/src/Rules/AttributesCheck.php

Check the places where the check method is called from. I expect a test to be added to one of the rules that calls this class.

[ondrejmirtes]

Fixed by: phpstan/phpstan-src#528

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.