WordPress-Coding-Standards
WordPress-Coding-Standards copied to clipboard
Fixer for missing parentheses adds them to the wrong place in one case
Bug Description
The fixer for the missing parentheses sniff adds the parentheses to the wrong place for a particular statement.
Minimal Code Snippet
$this->current_provider = new $this->providers->{$this->current_provider_slug}['provider'];
The issue happens when running this command:
phpcs phpcs-test.php --standard=WordPress-Extra -sp --basepath=.
... over a file containing this code (minimal example, extracted from the real code):
<?php
$this->current_provider = new $this->providers->{$this->current_provider_slug}['provider'];
The file was auto-fixed via phpcbf to:
<?php
$this->current_provider = new $this->providers->(){$this->current_provider_slug}['provider'];
// ---------------------------------------------^ ❌
... while I expected the code to be fixed to:
<?php
$this->current_provider = new $this->providers->{$this->current_provider_slug}['provider']();
// ---------------------------------------------------------------------------------------^ ✅
Error Code
WordPress.Classes.ClassInstantiation.MissingParenthesis
Environment
| Question | Answer |
|---|---|
| PHP version | 8.0.5 |
| PHP_CodeSniffer version | Latest git pull of master today (51335eb46) |
| WPCS version | Latest git pull of develop today (a93d2970) |
| WPCS install type | Composer |
| IDE (if relevant) | N/A - CLI. |
Additional Context (optional)
Tested Against develop branch?
- [x] I have verified the issue still exists in the
developbranch of WPCS.
When running the fixer again on an already fixed line:
$this->current_provider = new $this->providers->{$this->current_provider_slug}['provider']();
...then the mid-line "fix" gets added again, leading to:
$this->current_provider = new $this->providers->(){$this->current_provider_slug}['provider']();
(two sets of ()).
As a workaround, changing it to:
$class = $this->providers->{$this->current_provider_slug}['provider'];
$this->current_provider = new $class;
results in it being correctly fixed to:
$class = $this->providers->{$this->current_provider_slug}['provider'];
$this->current_provider = new $class();
This sniff is on my list of things which need looking into anyway for WPCS 3.0.0 as the rule it applies a candidate for moving to the Core ruleset.
The WPCS sniff does not handle variable variables with new.
There is an upstream PSR12 sniff which does, however, that sniff doesn't handle anonymous classes and there are some more differences as also documented in the Extra ruleset:
https://github.com/WordPress/WordPress-Coding-Standards/blob/a93d2970a3694e683588fedaa0f8a901fe148d3b/WordPress-Extra/ruleset.xml#L130-L139
There is already an issue open to discuss this: https://github.com/WordPress/WordPress-Coding-Standards/issues/1884 Might be good to have a look through that and leave an opinion ?
For the fun of it, I just added
$this->classname_tokens[ \T_OPEN_CURLY_BRACKET ] = \T_OPEN_CURLY_BRACKET;
$this->classname_tokens[ \T_CLOSE_CURLY_BRACKET ] = \T_CLOSE_CURLY_BRACKET;
To the list of classname tokens and ran the phpcbf and got the correct result 😅
Note that I didn't run the tests or anything so this could be a one off fix that breaks other things, so...
EDIT: Added a test and it breaks anonymous classes:
-$util->setLogger( new class() {} );
+$util->setLogger( new class {}() );
So, not really a fix.
@dingo-d Yup, sniffing is tricky... there was a reason variable variables weren't supported (yet) 😉
This is an open source project, which means that this is the personal fiefdom, not really an open community. It's weird that it's included in the WordPress organization. At any rate, there hasn't been a published update in forever, and probably will never be one. But, these assholes will be rude as fuck and call you rude if you ask when it'll be updated and published. Fuck WordPress.
@WraithKenny You have been told before and I'm telling you again: this kind of behaviour and language is not tolerated here. Your behaviour is disruptive, rude, abusive and highly demotivating and discouraging for existing contributors (whether currently active or not).
You are put on warning and will be blocked from participating in this repo if you continue.