"Unsafe usage of new static" breaks PHP "Late Static Binding" #4353
Replies: 5 comments 3 replies
-
This is needed for type safety though: You're free to ignore errors you don't want to check, but this rule prevent fatal errors in code, it seems pretty useful |
Beta Was this translation helpful? Give feedback.
-
Yes, the example by @orklah is the reason why this rule exists. And it doesn't block late static binding, because there are multiple ways you can solve this:
|
Beta Was this translation helpful? Give feedback.
-
The example giving can happen in all forms of extension. I dont think this rule is preventing any errors just preventing extension by encouraging users to finalise their classes or ignore the rule In the example when any use of the
Regarding solving the error without ignoring it, the error message only offers one solution. Should it not at least offer all the options. A normal user will not know they can solve it any other way. For a class that is designed for extension in mind, the advice to finalise the class would actually just lead to the rule being ignored making it pointless. |
Beta Was this translation helpful? Give feedback.
-
I spent some time trying to understand the landscape of this a little better and the downstream impact on the community. Searching for "Unsafe usage of new static()" you bump into many discussions about this rule. Adding an ignore is common. I also found discussions around changing code to use dynamic factories to workaround the rule. For me this is problematic, these factories are more likely to be buggy than just using late static binding to resolve the class instance. Some examples of what I found:
For me, the single biggest problem is the fact that the only tip being offered is to make the constructor final. Even though there are 3 ways of resolving this error. With the other two being very constructive and promoting good practices through the use of abstract classes and interfaces. Because PHPStan is viewed as an authority on good programming practices, and this advice prevents late static binding. People are then forced to either ignore advice negating the rule or coming up with creative and possibly dangerous workarounds. Proposal & Suggests If I put a PR together updating
I think PHPStan is a phenomenal project that is doing a lot of good in the PHP community, but I think this rule needs to be reviewed a little, as there are negative consequences of only offering the |
Beta Was this translation helpful? Give feedback.
-
I just published an article with possible solutions https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static and linked it as a tip 💡 in the rule: phpstan/phpstan-src@96f1e10 |
Beta Was this translation helpful? Give feedback.
-
Discussion
I would like to discuss the
NewStaticRule
.We are in the process of adding PHPStan to an existing code base and are surprised to see errors whenever the static keyword is used to new up a class.
After looking into the errors and the rationale behind the error, I have to say I strongly disagree with this error.
Applying this error makes any use of Late Static Bindings imposable and some frameworks and libraries usable. An example of this is Eloquent ORM. The base
Model
class usesnew static
, and this resolves to the actual class that is extended fromModel
.If an author did not want to use late static binding they should use the
self
keyword instead of thestatic
keyword.This
NewStaticRule
is implying that almost all forms of class extension are dangerous and forcing authors to finalize all their classes.In the case of late static binding, the author extending from the parent is the one responsible for the extension, and when I call
MyModel::newQuery()
I am expecting an instance ofMyModel
without needing to overridenewQuery()
.I would say if a class is open to extension by design, the use of the
static
keyword is safer because it will always resolve the instance to the calling / consumer codes expected class instance.By blocking the user of late static binding several OO paterns are impossible and a core language feature is unusable.
I would strongly advocate for the removal of the
NewStaticRule
rule as it breaks a core PHP feature.Beta Was this translation helpful? Give feedback.
All reactions