Open dingo-d opened 4 years ago
??
usage would influence the minimum PHP version for the theme, wouldn't it?
Once core moves to PHP 7.0 (beginning of the next year) this shouldn't have to be an issue imo.
Once core moves to PHP 7.0
But this is for themes, not core. And themes can run on different versions of core, with different versions of PHP.
Both ?:
(PHP 5.3+) usage, as well as ??
(PHP 7.0+) usage influence the minimum PHP version for a theme.
If I remember correctly, the "default" presumption is that themes support the current WP version + up to three versions back. That would currently mean WP 5.0
- 5.3
, meaning the "normal" minimum PHP version for a theme would still need to be PHP 5.2
, though this can be changed of course with the Requires PHP:
header.
Yes, but considering that WP doesn't check the Requires PHP
header for themes yet, the theme still needs to make sure that it doesn't fatal. And a warning to the reviewer would be good to have.
Well, the PHPCompatibilityWP
ruleset which is included in TRTCS already does so as the minimum PHP version for that is set to 5.2
.
The decision from the triage: include this sniff but change it so that it throws a warning instead of an error.
This shouldn't be too hard, just extend the sniff and override the process_token
method
No need to extend the sniff, this can simply be changed from the ruleset <type>warning</type>
.
Even better 🙂
Rule:
https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#ternary-operator
Short ternaries look like
In this case the value of
$some_result
will be$some_value
unless the$some_value
evaluates tofalse
, in that case, the value of$some_result
will be the string'default'
.Inspecting the usage in the theme directory I saw that most authors are using it wrongly - they assume that the value of the array key will be used if it exists
However, the value of
$output
will never be$input['key']
.It will either bedefault
string in case the key doesn't exist ortrue
if it does. Which is (in the majority of cases) not intended behavior.The intended behavior is (probably)
But if the
$input['key']
doesn't exist, the above will throw an undefined index notice. Most likely the operator that the author intended to use is null coalesce (??
).Decision needed:
A decision is needed whether we'd include this in the first place or no?
If we include it, it could be downgraded to warning to be a caution for theme authors. On the other hand, reviewers who see this warning could report this to an author, even if they really used this with a correct intention. That could cause confusion during the review process. Although it would be a good way for both authors and reviewers to distinguish the difference between the short ternary (
?:
) and null coalesce operators (??
).To do:
sniffname
sniff to the ruleset.