php-coding-standards icon indicating copy to clipboard operation
php-coding-standards copied to clipboard

[Feature Request]: Add a sniff to enforce property type

Open gmazzap opened this issue 2 years ago • 3 comments
trafficstars

Is your feature request related to a problem?

We already enforce functions' arguments and return types, considering that version v2 of this repo is PHP 7.4+ we should also enforce types on properties.

Describe the desired solution

Add a sniff that adds a warning whenever a class or trait declares properties without a type.

Describe the alternatives that you have considered

We either add it or not.

Additional context

To be noted: property type is invariant, so if a class overrides a property from a parent class in vendor (so the parent will not be scanned via the sniff) and the parent type has no type declaration, forcing the type declaration on the child (that would be scanned) will cause a parse error. PHPCS can't know that having a single-file context. Example

Using this rule with inheritance and without a static analyzer like Psalm could be dangerous. (Another good reason to use Psalm and use inheritance sparingly).

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

gmazzap avatar Nov 15 '23 11:11 gmazzap

Just noting that the Slevomat Coding Standard includes a dedicated sniff for property type declarations:

  • https://github.com/slevomat/coding-standard/blob/master/doc/type-hints.md#slevomatcodingstandardtypehintspropertytypehint-

tfrommen avatar Nov 15 '23 19:11 tfrommen

Thanks @tfrommen yes, I was pretty sure there was something out there that could work, but did not search without first getting a consensus.

Reading the description of the sniff it looks very good, but it seems it has an edge case. If you have:

/** @var list<int> */
private $list;

It does not complain, because @var list<int> is not a type you can represent in native PHP. However, it would be nice to enforce:

/** @var list<int> */
private array $list;

Maybe we can extend that rule and "fix" this issue if we believe it needs fixing.

gmazzap avatar Nov 15 '23 20:11 gmazzap