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

Review the WordPressVIPMinimum.Performance.LowExpiryCacheTime sniff

Open jrfnl opened this issue 5 years ago • 2 comments

Review the WordPressVIPMinimum.Performance.LowExpiryCacheTime 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
    • [x] 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

While working on #572, a number of other issues with the sniff caught my eye. A few of these will be addressed now in combination with #572, but there are also issues which are better left for when PHPCSUtils is in place.

Will be addressed now:

  • Handling of comments in the parameter.
  • Handling of 0 and values which evaluate to 0 being passed. 0 is the default value and will translate in the wp_cache_set() function internally as "no expiration".
  • One of the WP predefined time constants being passed on its own.

Edge cases, can be addressed later using Utils:

  • Prevent false positives for namespaced functions with the same name imported via a use function statement.
  • Prevent incorrect identification of namespaced constants with the same name as one of the WP time constants imported via a use const statement.
  • Handling of non-decimal numbers.
  • Handling of PHP 7.4 numeric literals with underscores.
  • Handling of other integer constants with known values.
  • More complex calculation handling (isNumericCalculation).
  • Handling of PHP 8.0 namespace names (as single token) for both the function name recognition as well as the WP time constant recognition.

Note: this is not a complete review, just notes as reminder for later.

jrfnl avatar Oct 07 '20 21:10 jrfnl

Related open issues: #408 (PR #445)

For some more example code, see: https://wpdirectory.net/search/01EM5DTRB0VBNB64F3HTGH5D97

jrfnl avatar Oct 08 '20 23:10 jrfnl