As a Drupal module developer, if you're as big of a fan of PHPStan as I am, then you'll no doubt have run across the Unsafe usage of new static() error that PHPStan throws when analyzing code at level 0. In this article I will describe what causes this error and provide four possible solutions, weighing the pros and cons of each.
Generally, this will occur in a create() method when you are injecting a dependency into a class using either ContainerFactoryPluginInterface or ContainerInjectionInterface using the common (simplified) Drupal pattern of:
class baseClass {
public function __construct(array $configuration, $plugin_id, $plugin_definition, ModuleHandlerInterface $module_handler) {
parent::__construct($configuration, $plugin_id, $plugin_definition);
$this->moduleHandler = $module_handler;
}
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
return new static(
$configuration,
$plugin_id,
$plugin_definition,
$container->get('module_handler'),
);
}
}Basically, PHPStan is warning you that there could be an issue if the class is extended and the new class overrides the constructor with different arguments. For example, if the Token service class is added:
class newClass extends baseClass {
public function __construct(array $configuration, $plugin_id, $plugin_definition, Token $token, ModuleHandlerInterface $module_handler) {
parent::__construct($configuration, $plugin_id, $plugin_definition);
$this->token = $token;
$this->moduleHandler = $module_handler;
}
}At this point, when newClass is instantiated by the container, it will fail because its inherited create() method no longer matches its overridden constructor. This is bad.
Now, this coding pattern is all over Drupal core and contributed modules, and we are generally very careful about breaking things (thank you, automated tests.) Regardless, PHPStan isn't cool with it, so let's look at some alternatives.
Dig hole in sand, stick head in it
Easiest, but probably not a good long-term solution.
PHPStan provides the ability for certain errors to be ignored, and this is definitely a good candidate for ignoring. Additionally, it is simple enough to do with a quick addition to your phpstan.neon configuration file:
parameters:
level: 0
paths:
- web/modules
ignoreErrors:
- '#Unsafe usage of new static\(\)#'Generally, I'm not a big fan of ignoring any code quality issues, whether they are from phpcs, PHPStan or any other code quality tool.
Pros
- PHPStan error goes away
Cons
- That nagging feeling that you really didn't fix anything.
Prevent others from extending the class
Easiest for custom code, with limitations.
If using new static is only an issue when the class is extended, then why not just prevent the class from being extended with the addition of the final keyword?
final class baseClass {
…
}This definitely solves the issue, and in many cases, is a valid solution. I almost always use final when writing a custom module - especially when I am in control of the code base.
Using final in core or a contributed module is a bit trickier, as in most cases the assumption does have to be made that someone somewhere is going to extend that class for reasons you haven't thought of. Being the co-maintainer of several contributed modules, I'm experimenting with using it - to see if anyone opens an issue letting me know that it is blocking them in some way or another. Time will tell.
Pros
- PHPStan is happy.
- Code protection against someone extending the class and not properly overriding the constructor and create() methods (if necessary).
Cons
- The class cannot be extended, possibly limiting its usefulness.
Change new static to new self
Most correct, but with potential repercussions.
Recommended by a Drupal.org documentation page, but with little explanation, this change looks like this (from our example above:)
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
return new self(
$configuration,
$plugin_id,
$plugin_definition,
$container->get('module_handler'),
);
}In this case, if the class is extended and the constructor is overridden, then the code will always fail unless the create() method is also overridden. Curiously, the documentation page also suggests adding final to the class. I suspect that the documentation page should be updated, but I'm not confident enough in what is the best way to handle this to make a suggestion. Yet.
Pros
- PHPStan is happy.
- This is probably the "most correct" solution.
Cons
- While the class can be extended, less experienced developers might not always recognize that they must override the create() method when overriding the constructor, potentially leading to a bug that can be tricky to figure out. For example, if the create() method isn't overridden, then the result would be that the base class is created and not the (expected) extended class.
Make the constructor final
Similar to marking the entire class "final".
I haven't spotted this potential solution in a Drupal community discussion, but on the PHPStan blog, one of the suggestions is to just make the constructor final. Again updating our code sample from above:
final public function __construct(array $configuration, $plugin_id, $plugin_definition, ModuleHandlerInterface $module_handler) {
parent::__construct($configuration, $plugin_id, $plugin_definition);
$this->moduleHandler = $module_handler;
}This seems like a slightly more flexible solution than making the entire class final or changing from new static to new self, but still would limit anyone extending the class. Maybe this is a reasonable middle-of-the-road solution? I haven't seen it implemented in Drupal yet.
This method would allow the class to be extended, but the constructor wouldn't be allowed to be overridden - which means no dependency changes can be made. Obviously, this limits what extended classes would be able to do.
Similar to this method, the blog post also suggested adding the constructor signature to an interface, which would have the same effect of locking it down so that it cannot be changed.
Pros
- PHPStan is happy.
- Code protection against someone extending the class and not properly overriding the constructor and create() methods by not allowing those methods to be overridden.
Cons
- If the constructor can't be overridden, then while the class can be extended, its dependencies cannot be changed, potentially limiting its usefulness.
Summary
So where does that leave us? Hopefully, with the ability to make a more informed decision on how to handle this particular PHPStan error. As I mentioned above, I'm not a big fan of ignoring PHPStan errors, so for now, I'll be adding final most of the time 😉.
Additional reading
- Philip Norton argues that since Drupal core uses new static, it should be ignored.
- Discussion in the Drupal core issue queue regarding new static versus new self.
- Discussion in the PHPStan Drupal issue queue related to whether or not these types of errors should be ignored in Drupal core PHPStan runs
- Discussion in PHPStan issue queue related to the effect on a large code base of changing new static to new self
Thanks to Professional Module Development course alumni James Shields and Hanane Kacemi as well as Ryan Price for reviewing this blog post.