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

PHP 8.1 Enums static methods support #1073

Open
Mapteg34 opened this issue Mar 1, 2024 · 5 comments
Open

PHP 8.1 Enums static methods support #1073

Mapteg34 opened this issue Mar 1, 2024 · 5 comments

Comments

@Mapteg34
Copy link

Mapteg34 commented Mar 1, 2024

  • PHPMD version: 2.15.0
  • PHP Version: 8.1.23
  • Installation type: composer
  • Operating System / Distribution & Version: Ubuntu 22.04

Current Behavior

Any of

Enum::from();
Enum::tryFrom();
Enum::cases();

trigger message Avoid using static access to class

Expected Behavior

No message, because this methods by design: https://www.php.net/manual/en/class.backedenum.php

Steps To Reproduce:

Try to use any method of https://www.php.net/manual/en/class.backedenum.php.


Please fix this behavior

@kylekatarnls
Copy link
Member

I agree it's not ideal, our doc says:

The only case when static access is acceptable is when used for factory methods.

A.K.A. Named constructors, from and tryFrom are such cases where it's generally accepted as a relevant usage of static method access. But with the limited possibilities (handling legacy code that is not strongly typed) we are currently unable to accurately detect if a method is actually such an acceptable case where we should not raise.

So at the moment it's on user to manually suppress those accesses that are valid.

It's far from being straightforward but when those methods are correctly type with : self or : ?self we could actually detect it's a named constructor and then not raise it.

For Enum::cases() it would be yet another difficulty level, as it's still acceptable use IMO by the fact that it's an array of self but even with the strong typing, we only see array so no easy static way to whitelist the method.

@AJenbo
Copy link
Member

AJenbo commented Mar 1, 2024

But with the limited possibilities (handling legacy code that is not strongly typed) we are currently unable to accurately detect if a method is actually such an acceptable case where we should not raise.

Would such methods not piratically always be strongly typed unless the class name is gotten dynamically as a string?

For Enum::cases() it would be yet another difficulty level, as it's still acceptable use IMO by the fact that it's an array of self but even with the strong typing, we only see array so no easy static way to whitelist the method.

I have somehow forgotten, but are we not able to read types from PHPDoc (array<self>)?

@kylekatarnls
Copy link
Member

Would such methods not piratically always be strongly typed unless the class name is gotten dynamically as a string?

Yes we can just consider public function from(): self and ignore /** @return self */ public function from() stuff.

I have somehow forgotten, but are we not able to read types from PHPDoc (array<self>)?

It's significantly less efficient to parse annotations (and the multiple forms: array<self>, list<self>, self[] etc.)

@AJenbo
Copy link
Member

AJenbo commented Mar 1, 2024

Arh i was thinking we could do it specifically for the enum class

@kylekatarnls
Copy link
Member

We can, it's a quick-win, and the same way we can also include other PHP native static constructors such as DateTime::createFromFormat(), then it will be a list that we have to maintain and then we need to provide a way for users to declare their own methods to be whitelisted (I think the @SuppressWarning needs to be done for each call at the moment and cannot be made once on declaration, but I should check this assertion).

@ravage84 ravage84 added this to the 2.x (unspecific) milestone May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants