Skip to content

Commit dac90e5

Browse files
committed
LowExpiryCacheTime: improve handling of potential parse errors in evaled code
Issue reported by GaryJones in Automattic/VIP-Coding-Standards 628#issuecomment-790653774 In rare cases, it is possible that the call to `eval()` with "safe" tokens only, would still result in a parse error. An example of such a case is when the PHP 5.6 `**` (`T_POW`) operator is used. PHPCS backfills the tokenization, which means that the operator is correctly recognized by the sniff as a "safe" token to use in the `eval()` statement. However, when the sniff is being run on PHP 5.4/5.5, the `eval()` will also be run on PHP 5.4/5.5 and while PHPCS backfills the token, PHP does not, resulting in a parse error in the `eval`-ed code, upon which the `eval()` will return `false`. This commit: * Silences the parse error notice. * Checks the output of the `eval()` for it being boolean `false` and if so, flags the statement for manual inspection, same as when any "non-safe" token is encountered.
1 parent 288ad35 commit dac90e5

File tree

2 files changed

+17
-4
lines changed

2 files changed

+17
-4
lines changed

WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
7777
$reportPtr = null;
7878
$openParens = 0;
7979

80+
$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"';
81+
$error_code = 'CacheTimeUndetermined';
82+
$data = [ $matched_content, $parameters[4]['raw'] ];
83+
8084
for ( $i = $param['start']; $i <= $param['end']; $i++ ) {
8185
if ( isset( Tokens::$emptyTokens[ $this->tokens[ $i ]['code'] ] ) === true ) {
8286
$tokensAsString .= ' ';
@@ -151,9 +155,7 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
151155
}
152156

153157
// Encountered an unexpected token. Manual inspection needed.
154-
$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"';
155-
$data = [ $matched_content, $parameters[4]['raw'] ];
156-
$this->phpcsFile->addWarning( $message, $reportPtr, 'CacheTimeUndetermined', $data );
158+
$this->phpcsFile->addWarning( $message, $reportPtr, $error_code, $data );
157159

158160
return;
159161
}
@@ -177,7 +179,17 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
177179
}
178180
}
179181

180-
$time = eval( "return $tokensAsString;" ); // phpcs:ignore Squiz.PHP.Eval -- No harm here.
182+
$time = @eval( "return $tokensAsString;" ); // phpcs:ignore Squiz.PHP.Eval,WordPress.PHP.NoSilencedErrors -- No harm here.
183+
184+
if ( $time === false ) {
185+
/*
186+
* The eval resulted in a parse error. This will only happen for backfilled
187+
* arithmetic operator tokens, like T_POW, on PHP versions in which the token
188+
* did not exist. In that case, flag for manual inspection.
189+
*/
190+
$this->phpcsFile->addWarning( $message, $reportPtr, $error_code, $data );
191+
return;
192+
}
181193

182194
if ( $time < 300 && (int) $time !== 0 ) {
183195
$message = 'Low cache expiry time of %s seconds detected. It is recommended to have 300 seconds or more.';

WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public function getWarningList() {
5555
77 => 1,
5656
78 => 1,
5757
79 => 1,
58+
82 => ( PHP_VERSION_ID > 50600 ) ? 0 : 1,
5859
88 => 1,
5960
94 => 1,
6061
95 => 1,

0 commit comments

Comments
 (0)