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

Review the WordPressVIPMinimum.Performance.LowExpiryCacheTime sniff #532

Open
1 of 51 tasks
jrfnl opened this issue Jul 27, 2020 · 2 comments
Open
1 of 51 tasks

Review the WordPressVIPMinimum.Performance.LowExpiryCacheTime sniff #532

jrfnl opened this issue Jul 27, 2020 · 2 comments
Assignees
Labels
PHPCSUtils The addition and utilisation of PHPCSUtils package
Milestone

Comments

@jrfnl
Copy link
Collaborator

jrfnl commented Jul 27, 2020

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
    • 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:

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 jrfnl added this to the 3.x milestone Sep 14, 2020
@jrfnl jrfnl added the PHPCSUtils The addition and utilisation of PHPCSUtils package label Sep 14, 2020
@jrfnl
Copy link
Collaborator Author

jrfnl commented Oct 7, 2020

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
Copy link
Collaborator Author

jrfnl commented Oct 8, 2020

Related open issues: #408 (PR #445)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHPCSUtils The addition and utilisation of PHPCSUtils package
Projects
None yet
Development

No branches or pull requests

1 participant