Skip to content

Performance/LowExpiryCacheTime: various sniff improvements #853

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.

Sign up for GitHub

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

Merged
GaryJones merged 5 commits into from
Jul 21, 2025
Merged

Performance/LowExpiryCacheTime: various sniff improvements #853

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b52dad5
Performance/LowExpiryCacheTime: move a parse error test to own test c…
jrfnl Jul 20, 2025
3043a9d
Performance/LowExpiryCacheTime: improve the tests
jrfnl Jul 20, 2025
7c5899b
Performance/LowExpiryCacheTime: add support for PHP 7.4+ numeric lite…
jrfnl Jul 20, 2025
38c0bdb
Performance/LowExpiryCacheTime: cleaner error message
jrfnl Jul 19, 2025
8acb4ae
Performance/LowExpiryCacheTime: add support for handling PHP 8.0+ fun…
jrfnl Jul 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
namespace WordPressVIPMinimum\Sniffs\Performance;

use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\Numbers;
use PHPCSUtils\Utils\PassedParameters;
use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\AbstractFunctionParameterSniff;

Expand Down Expand Up @@ -67,21 +69,21 @@ class LowExpiryCacheTimeSniff extends AbstractFunctionParameterSniff {
* normal file processing.
*/
public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
if ( isset( $parameters[4] ) === false ) {
$expire_param = PassedParameters::getParameterFromStack( $parameters, 4, 'expire' );
if ( $expire_param === false ) {
// If no cache expiry time, bail (i.e. we don't want to flag for something like feeds where it is cached indefinitely until a hook runs).
return;
}

$param = $parameters[4];
$tokensAsString = '';
$reportPtr = null;
$openParens = 0;

$message = 'Cache expiry time could not be determined. Please inspect that the fourth parameter passed to %s() evaluates to 300 seconds or more. Found: "%s"';
$error_code = 'CacheTimeUndetermined';
$data = [ $matched_content, $parameters[4]['raw'] ];
$data = [ $matched_content, $expire_param['clean'] ];

for ( $i = $param['start']; $i <= $param['end']; $i++ ) {
for ( $i = $expire_param['start']; $i <= $expire_param['end']; $i++ ) {
if ( isset( Tokens::$emptyTokens[ $this->tokens[ $i ]['code'] ] ) === true ) {
$tokensAsString .= ' ';
continue;
Expand All @@ -104,8 +106,10 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
if ( $this->tokens[ $i ]['code'] === T_LNUMBER
|| $this->tokens[ $i ]['code'] === T_DNUMBER
) {
// Integer or float.
$tokensAsString .= $this->tokens[ $i ]['content'];
// Make sure that PHP 7.4 numeric literals and PHP 8.1 explicit octals don't cause problems.
$number_info = Numbers::getCompleteNumber( $this->phpcsFile, $i );
$tokensAsString .= $number_info['decimal'];
$i = $number_info['last_token'];
continue;
}

Expand Down
64 changes: 51 additions & 13 deletions ...erformance/LowExpiryCacheTimeUnitTest.inc → ...formance/LowExpiryCacheTimeUnitTest.1.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ wp_cache_set( $testing, $data, 'test_group', 5*MINUTE_IN_SECONDS );
wp_cache_set( 123, $data, 'test_group', 5 * MINUTE_IN_SECONDS );
wp_cache_set( 1234, $data, '', 425 );
wp_cache_set( $testing, $data, null, 350 );
wp_cache_set( $testing, $data );
\wp_cache_set( $testing, $data );
wp_cache_set( 'test', $data, $group );

wp_cache_add( 'test', $data, $group, 300 );
wp_cache_add( $testing, $data, 'test_group', 6*MINUTE_IN_SECONDS );
wp_cache_add( 1234, $data, '', 425 );
WP_CACHE_ADD( 1234, $data, '', 425 );
wp_cache_add( $testing, $data, null, 350 );

wp_cache_replace( 'test', $data, $group, 300 );
Expand All @@ -30,13 +30,13 @@ wp_cache_set( 123, $data, null, 1.5 * MINUTE_IN_SECONDS ); // Lower than 300.
wp_cache_set( $testing, $data, '', 1.5 * MINUTE_IN_SECONDS ); // Lower than 300.

wp_cache_add( 'test', $data, $group, 100 ); // Lower than 300.
wp_cache_add( 'test', $data, $group, 2*MINUTE_IN_SECONDS ); // Lower than 300.
\wp_cache_add( 'test', $data, $group, 2*MINUTE_IN_SECONDS ); // Lower than 300.
wp_cache_add( 123, $data, null, 1.5 * MINUTE_IN_SECONDS ); // Lower than 300.
wp_cache_add( $testing, $data, '', 1.5 * MINUTE_IN_SECONDS ); // Lower than 300.

wp_cache_replace( 'test', $data, $group, 100 ); // Lower than 300.
wp_cache_replace( 'test', $data, $group, 2*MINUTE_IN_SECONDS ); // Lower than 300.
wp_cache_replace( 123, $data, null, 1.5 * MINUTE_IN_SECONDS ); // Lower than 300.
WP_CACHE_REPLACE( 123, $data, null, 1.5 * MINUTE_IN_SECONDS ); // Lower than 300.
wp_cache_replace( $testing, $data, '', 1.5 * MINUTE_IN_SECONDS ); // Lower than 300.

// Test error being reported on the line containing the parameter.
Expand All @@ -49,7 +49,7 @@ wp_cache_replace(

// Test calculations with floats.
wp_cache_replace( $testing, $data, '', 7.5 * MINUTE_IN_SECONDS ); // OK.
wp_cache_replace( $testing, $data, '', 500 * 0.1 ); // Bad.
wp_cache_replace( $testing, $data, '', 500 * 0.1, ); // Bad.

// Test comment handling.
wp_cache_add( 'test', $data, $group, /* Deliberately left empty */ ); // OK.
Expand All @@ -71,11 +71,11 @@ wp_cache_add(
); // OK.

// Test variable/constant with or without calculation being passed.
wp_cache_set( $key, $data, '', $time ); // Manual inspection warning.
WP_Cache_Set( $key, $data, '', $time ); // Manual inspection warning.
wp_cache_set( $key, $data, '', PREFIX_FIVE_MINUTES ); // Manual inspection warning.
wp_cache_set( $key, $data, '', 20 * $time ); // Manual inspection warning.
wp_cache_set( $key, $data, '', $base + $extra ); // Manual inspection warning.
wp_cache_set( $key, $data, '', 300 + $extra ); // Manual inspection warning.
wp_cache_set( $key, $data, '', 20 * $time /*comment*/ ); // Manual inspection warning.
wp_cache_set( $key, $data, '', $base + $extra, ); // Manual inspection warning.
\wp_cache_set( $key, $data, '', 300 + $extra ); // Manual inspection warning.
wp_cache_set( $key, $data, '', PREFIX_CUSTOM_TIME * 5); // Manual inspection warning.

// Test calculations with additional aritmetic operators.
Expand All @@ -84,9 +84,9 @@ wp_cache_add( 'test', $data, $group, WEEK_IN_SECONDS / 3 + HOUR_IN_SECONDS ); /

// Test calculations grouped with parentheses.
wp_cache_set( $key, $data, '', (24 * 60 * 60) ); // OK.
wp_cache_set( $key, $data, '', (-(2 * 60) + 600) ); // OK.
wp_cache_set( $key, $data, '', (-(2 * 60) + 600), ); // OK.
wp_cache_set( $key, $data, '', (2 * 60) ); // Bad.
wp_cache_set( $key, $data, '', (-(2 * 60) + 600 ); // OK - includes parse error, close parenthesis missing.


// Test handling of numbers passed as strings.
wp_cache_set( 'test', $data, $group, '300' ); // OK - type cast to integer within the function.
Expand All @@ -110,14 +110,52 @@ wp_cache_set( 'test', $data, $group, \MONTH_IN_SECONDS ); // OK.

// Test passing something which may look like one of the time constants, but isn't.
wp_cache_set( 'test', $data, $group, month_in_seconds ); // Bad - constants are case-sensitive.
wp_cache_set( 'test', $data, $group, HOUR_IN_SECONDS::methodName() ); // Bad - not a constant.
wp_cache_set( 'test', $data, $group, /*comment*/ HOUR_IN_SECONDS::methodName() ); // Bad - not a constant.
wp_cache_set( 'test', $data, $group, $obj->MONTH_IN_SECONDS ); // Bad - not a constant.
wp_cache_set( 'test', $data, $group, $obj::MONTH_IN_SECONDS ); // Bad - not the WP constant.
wp_cache_set( 'test', $data, $group, PluginNamespace\SubLevel\DAY_IN_SECONDS ); // Bad - not the WP constant.
WP_Cache_Set( 'test', $data, $group, PluginNamespace\SubLevel\DAY_IN_SECONDS, ); // Bad - not the WP constant.

// Test passing negative number as cache time.
wp_cache_set( 'test', $data, $group, -300 ); // Bad.
wp_cache_add( $testing, $data, 'test_group', -6 * MINUTE_IN_SECONDS ); // Bad.

// Test more complex logic in the parameter.
wp_cache_add( $key, $data, '', ($toggle ? 200 : 400) ); // Manual inspection warning.

// Test handling of non-numeric data in text string.
wp_cache_set( 'test', $data, $group, '' ); // Manual inspection warning.
wp_cache_set( 'test', $data, $group, '300 Mulberry Street' ); // Manual inspection warning.

// Test handling of edge case/parse error.
wp_cache_set( 'test', $data, $group,\); // OK, ignore.

// Test handling of some modern PHP syntaxes.
wp_cache_add('test', $data, $group, ...$params); // PHP 5.6 argument unpacking. Manual inspection warning.
wp_cache_add( $key, $data, '', $toggle ?? 400) ); // PHP 7.0 null coalesce. Manual inspection warning.
add_action('my_action', wp_cache_set(...)); // PHP 8.1 first class callable. OK, ignore.

// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute.
#[WP_Cache_Replace('text', 'data', 'group', 50)]
function foo() {}

// Alternative numeric base.
wp_cache_set( $key, $data, '', 0620 ); // Octal number. OK (=400).
wp_cache_set( $key, $data, '', 0x190 ); // Hexidecimal number. OK (=400).
wp_cache_set( $key, $data, '', 0b110010000 ); // PHP 5.4 binary number. OK (=400).
wp_cache_set( $key, $data, '', 1_000 ); // PHP 7.4 numeric literal with underscore. OK.
wp_cache_set( $key, $data, '', 0o620 ); // PHP 8.1 octal literal. OK (=400).

wp_cache_set( $key, $data, '', 0226 ); // Octal number. Bad (=150).
wp_cache_set( $key, $data, '', 0x96 ); // Hexidecimal number. Bad (=150).
wp_cache_set( $key, $data, '', 0b10010110 ); // PHP 5.4 binary number. Bad (=150).
wp_cache_set( $key, $data, '', 1_50 ); // PHP 7.4 numeric literal with underscore. Bad.
wp_cache_set( $key, $data, '', 0o226 ); // PHP 8.1 octal literal. Bad (=150).

// Safeguard handling of function calls using PHP 8.0+ named parameters.
wp_cache_add(data: $data, group: $group); // OK, well, not really, missing required $key param, but that's not the concern of this sniff.
wp_cache_replace(data: $data, expire: 400, group: $group); // OK.
wp_cache_add($key, group: $group, data: $data, expires: 100,); // OK, well, not really, typo in param name, but that's not the concern of the sniff.
wp_cache_replace($key, expire: 400, group: $group, data: 100,); // OK.

wp_cache_replace($key, $data, expire: 100 ); // Bad.
wp_cache_replace(expire: 100, data: $data, key: $group); // Bad.
5 changes: 5 additions & 0 deletions WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.2.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php

// Parse error/live coding (missing close parentheses).
// This should be the only test in this file.
wp_cache_set( $key, $data, '', (-(2 * 60) + 600 ); // OK.
93 changes: 56 additions & 37 deletions WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,44 +28,63 @@ public function getErrorList() {
/**
* Returns the lines where warnings should occur.
*
* @param string $testFile The name of the file being tested.
*
* @return array<int, int> Key is the line number, value is the number of expected warnings.
*/
public function getWarningList() {
return [
27 => 1,
28 => 1,
29 => 1,
30 => 1,
32 => 1,
33 => 1,
34 => 1,
35 => 1,
37 => 1,
38 => 1,
39 => 1,
40 => 1,
47 => 1,
52 => 1,
56 => 1,
74 => 1,
75 => 1,
76 => 1,
77 => 1,
78 => 1,
79 => 1,
82 => ( PHP_VERSION_ID > 50600 ) ? 0 : 1,
88 => 1,
94 => 1,
95 => 1,
105 => 1,
112 => 1,
113 => 1,
114 => 1,
115 => 1,
116 => 1,
119 => 1,
120 => 1,
123 => 1,
];
public function getWarningList( $testFile = '' ) {
switch ( $testFile ) {
case 'LowExpiryCacheTimeUnitTest.1.inc':
return [
27 => 1,
28 => 1,
29 => 1,
30 => 1,
32 => 1,
33 => 1,
34 => 1,
35 => 1,
37 => 1,
38 => 1,
39 => 1,
40 => 1,
47 => 1,
52 => 1,
56 => 1,
74 => 1,
75 => 1,
76 => 1,
77 => 1,
78 => 1,
79 => 1,
82 => ( PHP_VERSION_ID > 50600 ) ? 0 : 1,
88 => 1,
94 => 1,
95 => 1,
105 => 1,
112 => 1,
113 => 1,
114 => 1,
115 => 1,
116 => 1,
119 => 1,
120 => 1,
123 => 1,
126 => 1,
127 => 1,
133 => 1,
134 => 1,
148 => 1,
149 => 1,
150 => 1,
151 => 1,
152 => 1,
160 => 1,
161 => 1,
];

default:
return [];
}
}
}