Null coalescing operator #840
Conversation
Releasing version 0.10.0
Null coalescing operators were added to PHP in PHP7. They are "syntactic sugar for the common case of needing to use a ternary in conjunction with isset(). It returns its first operand if it exists and is not NULL; otherwise it returns its second operand." WordPress_Sniff::is_in_isset_or_empty should assume that null coalescing operators satisfy being inside a empty or isset test. Fixes WordPress#837
There was a problem hiding this comment.
Nice. What about also adding tests for this in https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.inc ? The example from the original ticket, for example: $test = $_GET['test'] ?? '';. Not sure how that will affect running against different PHP versions though.
Looks like there are also a few codestyle issues pointed out by Travis.
As the code is tokenized, not parsed, it should be fine as long as the tokens are accounted for in the various PHP versions. |
There was a problem hiding this comment.
I second @JDGrimes request for tests. I've ran some tests and find that the current code will break on a number of test cases. Will post the test code and results in a separate comment.
I also found that when the token is correctly detected, the unslash error seems to go missing.
Also: please rebase the PR against the current develop branch.
|
Summary of the outcome of my tests (based on a rebased copy of the PR):
Here's the test code I've used: /*
* Test recognition of the T_COALESCE token.
*/
$var = sanitize_text_field( wp_unslash( $_POST['foobar'] ?? '' ) ); // OK.
$var = sanitize_text_field( wp_unslash( $_POST['foobar']['sub'] ?? '' ) ); // OK.
$var = sanitize_text_field( wp_unslash( ( $_POST['foobar']['sub'] ) ?? '' ) ); // OK (extra brackets).
$var = sanitize_text_field( $_POST['foobar'] ?? '' ); // Bad x1 - unslash.
// The below two sets should give the same errors.
$var = $_POST['foobar'] ?? ''; // Bad x2 - sanitize + unslash.
$var = $_POST['foobar']['sub'] ?? ''; // Bad x2 - sanitize + unslash.
$var = ( $_POST['foobar']['sub'] ) ?? ''; // Bad x2 - sanitize + unslash.
$var = ( $_POST['foobar']['sub'] ?? '' ); // Bad x2 - sanitize + unslash.
$var = isset( $_POST['foobar'] ) ? $_POST['foobar'] : ''; // Bad x2 - sanitize + unslash.
$var = isset( $_POST['foobar']['sub'] ) ? $_POST['foobar']['sub'] : ''; // Bad x2 - sanitize + unslash.
$var = ( isset( $_POST['foobar']['sub'] ) ) ? $_POST['foobar']['sub'] : ''; // Bad x2 - sanitize + unslash.
$var = ( isset( $_POST['foobar']['sub'] ) ? $_POST['foobar']['sub'] : '' ); // Bad x2 - sanitize + unslash.And these are the results: |
|
@aaronjorbin Just checking: Do you have any intention of updating this PR based on the above feedback or should we close the PR ? |
|
@aaronjorbin Hi Aaron, just checking in again to see if you are still interested in finishing this. If you haven't got time or lost interest, please let us know and we'll see if we can find someone to take over. |
|
I'm not sure when I'll have time for this again, so if someone else is able to work on it they should absolutely feel free to. |
|
@aaronjorbin Thanks for the quick reply. I'll see what I can do. WCEU contributors day coming up, so who knows ;-) |
Fixes #837