VIP-Coding-Standards icon indicating copy to clipboard operation
VIP-Coding-Standards copied to clipboard

Review the WordPressVIPMinimum.Security.ExitAfterRedirect sniff

Open jrfnl opened this issue 5 years ago • 2 comments

Review the WordPressVIPMinimum.Security.ExitAfterRedirect sniff for the following in as far as relevant to that sniff:

  • [ ] Code style independent sniffing / Correct handling of quirky code Typical things to add tests for and verify correct handling of:
    • [ ] Nested function/closure declarations
    • [ ] Nested class declarations
    • [ ] Comments in unexpected places
    • [ ] Variables being assigned to via list statements
    • [ ] Multiline text strings
    • [ ] Text strings provided via heredoc/nowdoc
    • [ ] Use of short open tags
    • [ ] Using PHP close tag as end of statement
    • [ ] Inline control structures (without braces)
  • [ ] Code simplifications which can be made using PHPCSUtils
  • [ ] Sniff stability improvements which can be made using PHPCSUtils
  • [ ] Correct handling of modern PHP code Typical things to add tests for and verify correct handling of (where applicable):
    • [ ] PHP 5.0 Try/catch/finally (PHP 5.5) and exceptions
    • [ ] PHP 5.3 Namespaced code vs code in the global namespace
    • [ ] PHP 5.3 Use import statements, incl aliasing
    • [ ] PHP 5.3 Short ternaries
    • [ ] PHP 5.3 Closures, incl closure use
    • [ ] PHP 5.4 Short arrays
    • [ ] PHP 5.5 Class name resolution using ::class
    • [ ] PHP 5.5 List in foreach
    • [ ] PHP 5.5/7.0 Generators using yield and yield from
    • [ ] PHP 5.6 Constant scalar expressions
    • [ ] PHP 5.6 Importing via use function/const
    • [ ] PHP 7.0 Null coalesce
    • [ ] PHP 7.0 Anonymous classes
    • [ ] PHP 7.0 Scalar and return type declarations
    • [ ] PHP 7.0 Group use statements
    • [ ] PHP 7.1 Short lists
    • [ ] PHP 7.1 Keyed lists
    • [ ] PHP 7.1 Multi-catch
    • [ ] PHP 7.1 Nullable types
    • [ ] PHP 7.3 List reference assignments
    • [ ] PHP 7.4 arrow functions
    • [ ] PHP 7.4 numeric literals with underscores
    • [ ] PHP 7.4 null coalesce equals
    • [ ] PHP 7.4 Typed properties
    • [ ] Various versions: trailing comma's in function calls, group use, function declarations, closure use etc

Other:

  • [ ] Review violation error vs warning
  • [ ] Review violation severity
  • [ ] Review violation message, consider adding a link
  • [ ] Check open issues related to the sniff
  • [ ] Review PHPDoc comments

Sniff basics, but changes need to be lined up for next major release:

  • [ ] Inappropriate use of public properties (#234)
  • [ ] Modular error codes (unique error code for each distinct message)

Once PHPCS/PHPCSUtils supports this:

  • [ ] PHP 8.0 Constructor property promotion
  • [ ] PHP 8.0 Union types
  • [ ] PHP 8.0 match expressions
  • [ ] PHP 8.0 Nullsafe operator
  • [ ] PHP 8.0 Named arguments
  • [ ] PHP 8.0 Single token namespaced names

jrfnl avatar Jul 27 '20 01:07 jrfnl

In my opinion, this sniff should be rewritten and based on the WPCS AbstractFunctionRestrictionsSniff (and eventually the PHPCSUtils version of that).

However, I also believe this sniff does not belong in the VIPCS repo, but should be part of WordPressCS and there's a long-standing issue open with a request for such a sniff.

I'm loath to make any significant changes to the VIP version of the sniff as the code here is licensed under GPLv2, which is incompatible with the WPCS MIT license and could end up blocking a sniff for the same going into WordPressCS .

jrfnl avatar Jul 23 '25 12:07 jrfnl

Hope I'm reporting at the right place and didn't miss it being in the list above already, but it reports false positives in simple conditions too:

if ( ! is_string( $foo ) && wp_redirect( get_home_url(), 302 ) === true ) {
	exit;
}

kkmuffme avatar Aug 29 '25 00:08 kkmuffme