Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RestrictedVariables: don't report on "use" in isset() #569

Merged

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Jul 29, 2020

Disregard when the existence of a restricted variable is being checked. This uses the upstream WPCS Sniff::is_in_isset_or_empty() method.

This means that variables will not be reported as "used" when they are wrapped in a call to:

  • isset()
  • empty()
  • array_key_exists()

This also means that the if ( isset( $_SERVER['REMOTE_ADDR'] ) ) { test on line 13 will no longer report a warning.

As $_SERVER['REMOTE_ADDR'] was then no longer tested for and $_SERVER['HTTP_USER_AGENT'] was being tested twice (line 14 and line 28), I've changed the occurrence on line 14 to use $_SERVER['REMOTE_ADDR'] to make sure both are still tested.

Includes additional unit test for the case as reported by the op.

Fixes #568

@jrfnl jrfnl added this to the 2.2.0 milestone Jul 29, 2020
@jrfnl jrfnl requested a review from a team as a code owner July 29, 2020 11:36
@jrfnl
Copy link
Collaborator Author

jrfnl commented Jul 29, 2020

Note: this reminds me that I should open a review issue for the abstract sniff ;-)

@jrfnl jrfnl force-pushed the fix/568-restrictedvariable-disregard-when-in-isset-or-empty branch 2 times, most recently from f69245a to b54105f Compare July 29, 2020 12:33
Disregard when the existence of a restricted variable is being checked. This uses the upstream WPCS `Sniff::is_in_isset_or_empty()` method.

This means that variables will not be reported as "used" when they are wrapped in a call to:
* `isset()`
* `empty()`
* `array_key_exists()`

This also means that the `if ( isset( $_SERVER['REMOTE_ADDR'] ) ) {` test on line 13 will no longer report a warning.

As `$_SERVER['REMOTE_ADDR']` was then no longer tested for and `$_SERVER['HTTP_USER_AGENT']` was being tested twice (line 14 and line 28), I've changed the occurrence on line 14 to use `$_SERVER['REMOTE_ADDR']` to make sure both are still tested.

Includes additional unit test for the case as reported by the op.

Fixes 568
@GaryJones GaryJones merged commit a3334db into develop Jul 30, 2020
@GaryJones GaryJones deleted the fix/568-restrictedvariable-disregard-when-in-isset-or-empty branch July 30, 2020 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check for RestrictedVariables wrong
2 participants