+ * ```php
* $func = 'func_num_args';
* $func();
- *
+ * ```
*
- * See here: http://php.net/manual/en/migration71.incompatible.php
+ * Note that this sniff does not catch all possible forms of dynamic calling, only some.
*
- * Note that this sniff does not catch all possible forms of dynamic
- * calling, only some.
+ * @link http://php.net/manual/en/migration71.incompatible.php
*/
class DynamicCallsSniff extends Sniff {
+
/**
* Functions that should not be called dynamically.
*
* @var array
*/
- private $blacklisted_functions = [
- 'assert',
- 'compact',
- 'extract',
- 'func_get_args',
- 'func_get_arg',
- 'func_num_args',
- 'get_defined_vars',
- 'mb_parse_str',
- 'parse_str',
+ private $disallowed_functions = [
+ 'assert' => true,
+ 'compact' => true,
+ 'extract' => true,
+ 'func_get_args' => true,
+ 'func_get_arg' => true,
+ 'func_num_args' => true,
+ 'get_defined_vars' => true,
+ 'mb_parse_str' => true,
+ 'parse_str' => true,
];
/**
- * Array of functions encountered, along with their values.
- * Populated on run-time.
+ * Array of variable assignments encountered, along with their values.
*
- * @var array
+ * Populated at run-time.
+ *
+ * @var array The key is the name of the variable, the value, its assigned value.
*/
private $variables_arr = [];
@@ -61,8 +61,6 @@ class DynamicCallsSniff extends Sniff {
/**
* Returns the token types that this sniff is interested in.
*
- * We want everything variable- and function-related.
- *
* @return array(int)
*/
public function register() {
@@ -87,35 +85,21 @@ public function process_token( $stackPtr ) {
}
/**
- * Finds any variable-definitions in the file being processed,
- * and stores them internally in a private array. The data stored
- * is the name of the variable and its assigned value.
+ * Finds any variable-definitions in the file being processed and stores them
+ * internally in a private array.
*
* @return void
*/
private function collect_variables() {
- /*
- * Make sure we are working with a variable,
- * get its value if so.
- */
-
- if (
- $this->tokens[ $this->stackPtr ]['type'] !==
- 'T_VARIABLE'
- ) {
- return;
- }
$current_var_name = $this->tokens[ $this->stackPtr ]['content'];
/*
- * Find assignments ( $foo = "bar"; )
- * -- do this by finding all non-whitespaces, and
- * check if the first one is T_EQUAL.
+ * Find assignments ( $foo = "bar"; ) by finding all non-whitespaces,
+ * and checking if the first one is T_EQUAL.
*/
-
$t_item_key = $this->phpcsFile->findNext(
- [ T_WHITESPACE ],
+ Tokens::$emptyTokens,
$this->stackPtr + 1,
null,
true,
@@ -123,127 +107,82 @@ private function collect_variables() {
true
);
- if ( $t_item_key === false ) {
+ if ( $t_item_key === false || $this->tokens[ $t_item_key ]['code'] !== T_EQUAL ) {
return;
}
- if ( $this->tokens[ $t_item_key ]['type'] !== 'T_EQUAL' ) {
- return;
+ /*
+ * Find assignments which only assign a plain text string.
+ */
+ $end_of_statement = $this->phpcsFile->findNext( [ T_SEMICOLON, T_CLOSE_TAG ], ( $t_item_key + 1 ) );
+ $value_ptr = null;
+
+ for ( $i = $t_item_key + 1; $i < $end_of_statement; $i++ ) {
+ if ( isset( Tokens::$emptyTokens[ $this->tokens[ $i ]['code'] ] ) === true ) {
+ continue;
+ }
+
+ if ( $this->tokens[ $i ]['code'] !== T_CONSTANT_ENCAPSED_STRING ) {
+ // Not a plain text string value. Value cannot be determined reliably.
+ return;
+ }
+
+ $value_ptr = $i;
}
- if ( $this->tokens[ $t_item_key ]['length'] !== 1 ) {
+ if ( isset( $value_ptr ) === false ) {
+ // Parse error. Bow out.
return;
}
/*
- * Find encapsulated string ( "" )
+ * If we reached the end of the loop and the $value_ptr was set, we know for sure
+ * this was a plain text string variable assignment.
*/
- $t_item_key = $this->phpcsFile->findNext(
- [ T_CONSTANT_ENCAPSED_STRING ],
- $t_item_key + 1,
- null,
- false,
- null,
- true
- );
+ $current_var_value = $this->strip_quotes( $this->tokens[ $value_ptr ]['content'] );
- if ( $t_item_key === false ) {
+ if ( isset( $this->disallowed_functions[ $current_var_value ] ) === false ) {
+ // Text string is not one of the ones we're looking for.
return;
}
/*
- * We have found variable-assignment,
- * register its name and value in the
- * internal array for later usage.
+ * Register the variable name and value in the internal array for later usage.
*/
-
- $current_var_value =
- $this->tokens[ $t_item_key ]['content'];
-
- $this->variables_arr[ $current_var_name ] =
- str_replace( "'", '', $current_var_value );
+ $this->variables_arr[ $current_var_name ] = $current_var_value;
}
/**
* Find any dynamic calls being made using variables.
- * Report on this when found, using name of the function
- * in the message.
+ *
+ * Report on this when found, using the name of the function in the message.
*
* @return void
*/
private function find_dynamic_calls() {
- /*
- * No variables detected; no basis for doing
- * anything
- */
-
+ // No variables detected; no basis for doing anything.
if ( empty( $this->variables_arr ) ) {
return;
}
/*
- * Make sure we do have a variable to work with.
- */
-
- if (
- $this->tokens[ $this->stackPtr ]['type'] !==
- 'T_VARIABLE'
- ) {
- return;
- }
-
- /*
- * If variable is not found in our registry of
- * variables, do nothing, as we cannot be
- * sure that the function being called is one of the
- * blacklisted ones.
+ * If variable is not found in our registry of variables, do nothing, as we cannot be
+ * sure that the function being called is one of the disallowed ones.
*/
-
- if ( ! isset(
- $this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ]
- ) ) {
+ if ( ! isset( $this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ] ) ) {
return;
}
/*
- * Check if we have an '(' next, or separated by whitespaces
- * from our current position.
+ * Check if we have an '(' next.
*/
-
- $i = 0;
-
- do {
- $i++;
- } while (
- $this->tokens[ $this->stackPtr + $i ]['type'] ===
- 'T_WHITESPACE'
- );
-
- if (
- $this->tokens[ $this->stackPtr + $i ]['type'] !==
- 'T_OPEN_PARENTHESIS'
- ) {
- return;
- }
-
- $t_item_key = $this->stackPtr + $i;
-
- /*
- * We have a variable match, but make sure it contains name
- * of a function which is on our blacklist.
- */
-
- if ( ! in_array(
- $this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ],
- $this->blacklisted_functions,
- true
- ) ) {
+ $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $this->stackPtr + 1 ), null, true );
+ if ( $next === false || $this->tokens[ $next ]['code'] !== T_OPEN_PARENTHESIS ) {
return;
}
- // We do, so report.
- $message = 'Dynamic calling is not recommended in the case of %s.';
+ $message = 'Dynamic calling is not recommended in the case of %s().';
$data = [ $this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ] ];
- $this->phpcsFile->addError( $message, $t_item_key, 'DynamicCalls', $data );
+ $this->phpcsFile->addError( $message, $this->stackPtr, 'DynamicCalls', $data );
}
}
diff --git a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php
index 74995407..9893c1b3 100644
--- a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php
@@ -25,13 +25,6 @@ class RestrictedFunctionsSniff extends AbstractFunctionRestrictionsSniff {
public function getGroups() {
$groups = [
- 'wp_cache_get_multi' => [
- 'type' => 'error',
- 'message' => '`%s` is not supported on the WordPress.com VIP platform.',
- 'functions' => [
- 'wp_cache_get_multi',
- ],
- ],
'opcache' => [
'type' => 'error',
'message' => '`%s` is prohibited on the WordPress VIP platform due to memory corruption.',
@@ -50,13 +43,6 @@ public function getGroups() {
'opcache_get_configuration',
],
],
- 'get_super_admins' => [
- 'type' => 'error',
- 'message' => '`%s` is prohibited on the WordPress.com VIP platform.',
- 'functions' => [
- 'get_super_admins',
- ],
- ],
'internal' => [
'type' => 'error',
'message' => '`%1$s()` is for internal use only.',
@@ -64,7 +50,6 @@ public function getGroups() {
'wpcom_vip_irc',
],
],
- // @link WordPress.com: https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#flush_rewrite_rules
'flush_rewrite_rules' => [
'type' => 'error',
'message' => '`%s` should not be used in any normal circumstances in the theme code.',
@@ -96,11 +81,10 @@ public function getGroups() {
'dbDelta',
],
],
- // @link WordPress.com: https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#switch_to_blog
- // @link VIP Go: https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#switch_to_blog
+ // @link https://docs.wpvip.com/technical-references/code-review/vip-notices/#h-switch_to_blog
'switch_to_blog' => [
- 'type' => 'error',
- 'message' => '%s() is not something you should ever need to do in a VIP theme context. Instead use an API (XML-RPC, REST) to interact with other sites if needed.',
+ 'type' => 'warning',
+ 'message' => '%s() may not work as expected since it only changes the database context for the blog and does not load the plugins or theme of that site. Filters or hooks on the blog you are switching to will not run.',
'functions' => [
'switch_to_blog',
],
@@ -119,8 +103,7 @@ public function getGroups() {
'url_to_postid',
],
],
- // @link WordPress.com: https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#custom-roles
- // @link VIP Go: https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#custom-roles
+ // @link https://docs.wpvip.com/how-tos/customize-user-roles/
'custom_role' => [
'type' => 'error',
'message' => 'Use wpcom_vip_add_role() instead of %s().',
@@ -128,17 +111,6 @@ public function getGroups() {
'add_role',
],
],
- // @link WordPress.com: https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#wp_users-and-user_meta
- 'user_meta' => [
- 'type' => 'error',
- 'message' => '%s() usage is highly discouraged on WordPress.com VIP due to it being a multisite, please see https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#wp_users-and-user_meta.',
- 'functions' => [
- 'get_user_meta',
- 'update_user_meta',
- 'delete_user_meta',
- 'add_user_meta',
- ],
- ],
'term_exists' => [
'type' => 'error',
'message' => '%s() is highly discouraged due to not being cached; please use wpcom_vip_term_exists() instead.',
@@ -178,8 +150,7 @@ public function getGroups() {
'get_intermediate_image_sizes',
],
],
- // @link WordPress.com: https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#mobile-detection
- // @link VIP Go: https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#mobile-detection
+ // @link https://docs.wpvip.com/technical-references/code-review/vip-warnings/#h-mobile-detection
'wp_is_mobile' => [
'type' => 'error',
'message' => '%s() found. When targeting mobile visitors, jetpack_is_mobile() should be used instead of wp_is_mobile. It is more robust and works better with full page caching.',
@@ -259,15 +230,6 @@ public function getGroups() {
'lchown',
],
],
- 'site_option' => [
- 'type' => 'error',
- 'message' => '%s() will overwrite network option values, please use the `*_option()` equivalent instead (e.g. `update_option()`).',
- 'functions' => [
- 'add_site_option',
- 'update_site_option',
- 'delete_site_option',
- ],
- ],
'stats_get_csv' => [
'type' => 'error',
'message' => 'Using `%s` outside of Jetpack context pollutes the stats_cache entry in the wp_options table. We recommend building a custom function instead.',
@@ -298,8 +260,7 @@ public function getGroups() {
'the_field',
],
],
- // @link WordPress.com: https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#remote-calls
- // @link VIP Go: https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#remote-calls
+ // @link https://docs.wpvip.com/technical-references/code-review/vip-warnings/#h-remote-calls
'wp_remote_get' => [
'type' => 'warning',
'message' => '%s() is highly discouraged. Please use vip_safe_wp_remote_get() instead which is designed to more gracefully handle failure than wp_remote_get() does.',
@@ -307,11 +268,10 @@ public function getGroups() {
'wp_remote_get',
],
],
- // @link WordPress.com: https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#custom-roles
- // @link VIP Go: https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#cache-constraints
+ // @link https://docs.wpvip.com/technical-references/code-review/vip-errors/#h-cache-constraints
'cookies' => [
- 'type' => 'warning',
- 'message' => 'Due to using Batcache, server side based client related logic will not work, use JS instead.',
+ 'type' => 'error',
+ 'message' => 'Due to server-side caching, server-side based client related logic might not work. We recommend implementing client side logic in JavaScript instead.',
'functions' => [
'setcookie',
],
@@ -319,7 +279,7 @@ public function getGroups() {
// @todo Introduce a sniff specific to get_posts() that checks for suppress_filters=>false being supplied.
'get_posts' => [
'type' => 'warning',
- 'message' => '%s() is uncached unless the "suppress_filters" parameter is set to false. If the suppress_filter parameter is set to false this can be safely ignored. More Info: https://wpvip.com/documentation/vip-go/uncached-functions/.',
+ 'message' => '%s() is uncached unless the "suppress_filters" parameter is set to false. If the suppress_filter parameter is set to false this can be safely ignored. More Info: https://docs.wpvip.com/technical-references/caching/uncached-functions/.',
'functions' => [
'get_posts',
'wp_get_recent_posts',
@@ -328,7 +288,7 @@ public function getGroups() {
],
'create_function' => [
'type' => 'warning',
- 'message' => '%s() is highly discouraged, as it can execute arbritary code (additionally, it\'s deprecated as of PHP 7.2): https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#eval-and-create_function. )',
+ 'message' => '%s() is highly discouraged, as it can execute arbritary code (additionally, it\'s deprecated as of PHP 7.2): https://docs.wpvip.com/technical-references/code-review/vip-warnings/#h-eval-and-create_function. )',
'functions' => [
'create_function',
],
@@ -403,7 +363,7 @@ public function is_targetted_token( $stackPtr ) {
if ( isset( $skipped[ $this->tokens[ $prev ]['code'] ] ) ) {
return false;
}
- // Skip namespaced functions, ie: \foo\bar() not \bar().
+ // Skip namespaced functions, ie: `\foo\bar()` not `\bar()`.
if ( $this->tokens[ $prev ]['code'] === \T_NS_SEPARATOR ) {
$pprev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prev - 1, null, true );
if ( $pprev !== false && $this->tokens[ $pprev ]['code'] === \T_STRING ) {
diff --git a/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php
index f5ea5734..8029e732 100644
--- a/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php
@@ -30,7 +30,7 @@ class AlwaysReturnInFilterSniff extends Sniff {
* @return array(int)
*/
public function register() {
- return Tokens::$functionNameTokens;
+ return [ T_STRING ];
}
/**
@@ -122,15 +122,15 @@ private function processArray( $stackPtr ) {
* Process string.
*
* @param int $stackPtr The position in the stack where the token was found.
- * @param int $start The start of the token.
- * @param int $end The end of the token.
+ * @param int $start The start of the token.
+ * @param int $end The end of the token.
*/
private function processString( $stackPtr, $start = 0, $end = null ) {
$callbackFunctionName = substr( $this->tokens[ $stackPtr ]['content'], 1, -1 );
$callbackFunctionPtr = $this->phpcsFile->findNext(
- Tokens::$functionNameTokens,
+ T_STRING,
$start,
$end,
false,
@@ -149,8 +149,8 @@ private function processString( $stackPtr, $start = 0, $end = null ) {
* Process function.
*
* @param int $stackPtr The position in the stack where the token was found.
- * @param int $start The start of the token.
- * @param int $end The end of the token.
+ * @param int $start The start of the token.
+ * @param int $end The end of the token.
*/
private function processFunction( $stackPtr, $start = 0, $end = null ) {
@@ -175,6 +175,21 @@ private function processFunction( $stackPtr, $start = 0, $end = null ) {
*/
private function processFunctionBody( $stackPtr ) {
+ $filterName = $this->tokens[ $this->filterNamePtr ]['content'];
+
+ $methodProps = $this->phpcsFile->getMethodProperties( $stackPtr );
+ if ( $methodProps['is_abstract'] === true ) {
+ $message = 'The callback for the `%s` filter hook-in points to an abstract method. Please ensure that child class implementations of this method always return a value.';
+ $data = [ $filterName ];
+ $this->phpcsFile->addWarning( $message, $stackPtr, 'AbstractMethod', $data );
+ return;
+ }
+
+ if ( isset( $this->tokens[ $stackPtr ]['scope_opener'], $this->tokens[ $stackPtr ]['scope_closer'] ) === false ) {
+ // Live coding, parse or tokenizer error.
+ return;
+ }
+
$argPtr = $this->phpcsFile->findNext(
array_merge( Tokens::$emptyTokens, [ T_STRING, T_OPEN_PARENTHESIS ] ),
$stackPtr + 1,
@@ -189,8 +204,6 @@ private function processFunctionBody( $stackPtr ) {
return;
}
- $filterName = $this->tokens[ $this->filterNamePtr ]['content'];
-
$functionBodyScopeStart = $this->tokens[ $stackPtr ]['scope_opener'];
$functionBodyScopeEnd = $this->tokens[ $stackPtr ]['scope_closer'];
diff --git a/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php
index eb1dce4e..75b54729 100644
--- a/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php
@@ -25,7 +25,7 @@ class PreGetPostsSniff extends Sniff {
* @return array(int)
*/
public function register() {
- return Tokens::$functionNameTokens;
+ return [ T_STRING ];
}
/**
@@ -120,7 +120,7 @@ private function processString( $stackPtr ) {
$callbackFunctionName = substr( $this->tokens[ $stackPtr ]['content'], 1, -1 );
$callbackFunctionPtr = $this->phpcsFile->findNext(
- Tokens::$functionNameTokens,
+ T_STRING,
0,
null,
false,
@@ -195,7 +195,7 @@ private function processClosure( $stackPtr ) {
/**
* Process function's body
*
- * @param int $stackPtr The position in the stack where the token was found.
+ * @param int $stackPtr The position in the stack where the token was found.
* @param string $variableName Variable name.
*/
private function processFunctionBody( $stackPtr, $variableName ) {
@@ -363,7 +363,7 @@ private function isEarlyMainQueryCheck( $stackPtr ) {
* Is the current code a WP_Query call?
*
* @param int $stackPtr The position in the stack where the token was found.
- * @param null $method Method.
+ * @param null $method Method.
*
* @return bool
*/
@@ -394,7 +394,7 @@ private function isWPQueryMethodCall( $stackPtr, $method = null ) {
true
);
- return $next && in_array( $this->tokens[ $next ]['code'], Tokens::$functionNameTokens, true ) === true && $method === $this->tokens[ $next ]['content'];
+ return $next && $this->tokens[ $next ]['code'] === T_STRING && $method === $this->tokens[ $next ]['content'];
}
/**
diff --git a/WordPressVIPMinimum/Sniffs/Hooks/RestrictedHooksSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/RestrictedHooksSniff.php
index 5b44c2df..43054c2b 100644
--- a/WordPressVIPMinimum/Sniffs/Hooks/RestrictedHooksSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Hooks/RestrictedHooksSniff.php
@@ -53,8 +53,7 @@ class RestrictedHooksSniff extends AbstractFunctionParameterSniff {
],
],
'http_request' => [
- // WordPress.com: https://lobby.vip.wordpress.com/wordpress-com-documentation/fetching-remote-data/.
- // VIP Go: https://vip.wordpress.com/documentation/vip-go/fetching-remote-data/.
+ // https://docs.wpvip.com/technical-references/code-quality-and-best-practices/retrieving-remote-data/.
'type' => 'Warning',
'msg' => 'Please ensure that the timeout being filtered is not greater than 3s since remote requests require the user to wait for completion before the rest of the page will load. Manual inspection required.',
'hooks' => [
@@ -63,7 +62,7 @@ class RestrictedHooksSniff extends AbstractFunctionParameterSniff {
],
],
'robotstxt' => [
- // WordPress.com + VIP Go: https://wpvip.com/documentation/robots-txt/.
+ // https://docs.wpvip.com/how-tos/modify-the-robots-txt-file/.
'type' => 'Warning',
'msg' => 'Don\'t forget to flush the robots.txt cache by going to Settings > Reading and toggling the privacy settings.',
'hooks' => [
diff --git a/WordPressVIPMinimum/Sniffs/JS/StringConcatSniff.php b/WordPressVIPMinimum/Sniffs/JS/StringConcatSniff.php
index d0ace2ee..74fab5fc 100644
--- a/WordPressVIPMinimum/Sniffs/JS/StringConcatSniff.php
+++ b/WordPressVIPMinimum/Sniffs/JS/StringConcatSniff.php
@@ -65,8 +65,8 @@ public function process_token( $stackPtr ) {
/**
* Consolidated violation.
*
- * @param int $stackPtr The position of the current token in the stack passed in $tokens.
- * @param array $data Replacements for the error message.
+ * @param int $stackPtr The position of the current token in the stack passed in $tokens.
+ * @param array $data Replacements for the error message.
*/
private function addFoundError( $stackPtr, array $data ) {
$message = 'HTML string concatenation detected, this is a security risk, use DOM node construction or a templating language instead: %s.';
diff --git a/WordPressVIPMinimum/Sniffs/JS/StrippingTagsSniff.php b/WordPressVIPMinimum/Sniffs/JS/StrippingTagsSniff.php
index b140d8f6..8017fe58 100644
--- a/WordPressVIPMinimum/Sniffs/JS/StrippingTagsSniff.php
+++ b/WordPressVIPMinimum/Sniffs/JS/StrippingTagsSniff.php
@@ -7,7 +7,6 @@
namespace WordPressVIPMinimum\Sniffs\JS;
-use PHP_CodeSniffer\Files\File;
use WordPressVIPMinimum\Sniffs\Sniff;
use PHP_CodeSniffer\Util\Tokens;
diff --git a/WordPressVIPMinimum/Sniffs/JS/WindowSniff.php b/WordPressVIPMinimum/Sniffs/JS/WindowSniff.php
index 602d342c..e233e479 100644
--- a/WordPressVIPMinimum/Sniffs/JS/WindowSniff.php
+++ b/WordPressVIPMinimum/Sniffs/JS/WindowSniff.php
@@ -7,7 +7,6 @@
namespace WordPressVIPMinimum\Sniffs\JS;
-use PHP_CodeSniffer\Files\File;
use WordPressVIPMinimum\Sniffs\Sniff;
use PHP_CodeSniffer\Util\Tokens;
diff --git a/WordPressVIPMinimum/Sniffs/Performance/CacheValueOverrideSniff.php b/WordPressVIPMinimum/Sniffs/Performance/CacheValueOverrideSniff.php
index 96696437..806bebef 100644
--- a/WordPressVIPMinimum/Sniffs/Performance/CacheValueOverrideSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Performance/CacheValueOverrideSniff.php
@@ -29,7 +29,7 @@ class CacheValueOverrideSniff extends Sniff {
* @return array(int)
*/
public function register() {
- return Tokens::$functionNameTokens;
+ return [ T_STRING ];
}
@@ -97,10 +97,6 @@ public function process_token( $stackPtr ) {
*/
private function isFunctionCall( $stackPtr ) {
- if ( in_array( $this->tokens[ $stackPtr ]['code'], Tokens::$functionNameTokens, true ) === false ) {
- return false;
- }
-
// Find the next non-empty token.
$openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true );
diff --git a/WordPressVIPMinimum/Sniffs/Performance/FetchingRemoteDataSniff.php b/WordPressVIPMinimum/Sniffs/Performance/FetchingRemoteDataSniff.php
index 62d1d090..e3fc4330 100644
--- a/WordPressVIPMinimum/Sniffs/Performance/FetchingRemoteDataSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Performance/FetchingRemoteDataSniff.php
@@ -24,7 +24,7 @@ class FetchingRemoteDataSniff extends Sniff {
* @return array
*/
public function register() {
- return Tokens::$functionNameTokens;
+ return [ T_STRING ];
}
/**
diff --git a/WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php b/WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php
index 34ea78e6..23639560 100644
--- a/WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php
@@ -7,11 +7,14 @@
namespace WordPressVIPMinimum\Sniffs\Performance;
+use PHP_CodeSniffer\Util\Tokens;
use WordPressCS\WordPress\AbstractFunctionParameterSniff;
/**
* This sniff throws a warning when low cache times are set.
*
+ * {@internal VIP uses the Memcached object cache implementation. {@link https://github.com/Automattic/wp-memcached}}
+ *
* @package VIPCS\WordPressVIPMinimum
*
* @since 0.4.0
@@ -69,21 +72,135 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
return;
}
- $time = $parameters[4]['raw'];
+ $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'] ];
+
+ for ( $i = $param['start']; $i <= $param['end']; $i++ ) {
+ if ( isset( Tokens::$emptyTokens[ $this->tokens[ $i ]['code'] ] ) === true ) {
+ $tokensAsString .= ' ';
+ continue;
+ }
+
+ if ( $this->tokens[ $i ]['code'] === T_NS_SEPARATOR ) {
+ /*
+ * Ignore namespace separators. If it's part of a global WP time constant, it will be
+ * handled correctly. If it's used in any other context, another token *will* trigger the
+ * "undetermined" warning anyway.
+ */
+ continue;
+ }
+
+ if ( isset( $reportPtr ) === false ) {
+ // Set the report pointer to the first non-empty token we encounter.
+ $reportPtr = $i;
+ }
+
+ if ( $this->tokens[ $i ]['code'] === T_LNUMBER
+ || $this->tokens[ $i ]['code'] === T_DNUMBER
+ ) {
+ // Integer or float.
+ $tokensAsString .= $this->tokens[ $i ]['content'];
+ continue;
+ }
+
+ if ( $this->tokens[ $i ]['code'] === T_FALSE
+ || $this->tokens[ $i ]['code'] === T_NULL
+ ) {
+ $tokensAsString .= 0;
+ continue;
+ }
+
+ if ( $this->tokens[ $i ]['code'] === T_TRUE ) {
+ $tokensAsString .= 1;
+ continue;
+ }
+
+ if ( isset( Tokens::$arithmeticTokens[ $this->tokens[ $i ]['code'] ] ) === true ) {
+ $tokensAsString .= $this->tokens[ $i ]['content'];
+ continue;
+ }
- if ( is_numeric( $time ) === false ) {
// If using time constants, we need to convert to a number.
- $time = str_replace( array_keys( $this->wp_time_constants ), $this->wp_time_constants, $time );
+ if ( $this->tokens[ $i ]['code'] === T_STRING
+ && isset( $this->wp_time_constants[ $this->tokens[ $i ]['content'] ] ) === true
+ ) {
+ $tokensAsString .= $this->wp_time_constants[ $this->tokens[ $i ]['content'] ];
+ continue;
+ }
+
+ if ( $this->tokens[ $i ]['code'] === T_OPEN_PARENTHESIS ) {
+ $tokensAsString .= $this->tokens[ $i ]['content'];
+ ++$openParens;
+ continue;
+ }
- if ( preg_match( '#^[\s\d+*/-]+$#', $time ) > 0 ) {
- $time = eval( "return $time;" ); // phpcs:ignore Squiz.PHP.Eval -- No harm here.
+ if ( $this->tokens[ $i ]['code'] === T_CLOSE_PARENTHESIS ) {
+ $tokensAsString .= $this->tokens[ $i ]['content'];
+ --$openParens;
+ continue;
}
+
+ if ( $this->tokens[ $i ]['code'] === T_CONSTANT_ENCAPSED_STRING ) {
+ $content = $this->strip_quotes( $this->tokens[ $i ]['content'] );
+ if ( is_numeric( $content ) === true ) {
+ $tokensAsString .= $content;
+ continue;
+ }
+ }
+
+ // Encountered an unexpected token. Manual inspection needed.
+ $this->phpcsFile->addWarning( $message, $reportPtr, $error_code, $data );
+
+ return;
}
- if ( $time < 300 ) {
- $message = 'Low cache expiry time of "%s", it is recommended to have 300 seconds or more.';
- $data = [ $parameters[4]['raw'] ];
- $this->phpcsFile->addWarning( $message, $stackPtr, 'LowCacheTime', $data );
+ if ( $tokensAsString === '' ) {
+ // Nothing found to evaluate.
+ return;
+ }
+
+ $tokensAsString = trim( $tokensAsString );
+
+ if ( $openParens !== 0 ) {
+ /*
+ * Shouldn't be possible as that would indicate a parse error in the original code,
+ * but let's prevent getting parse errors in the `eval`-ed code.
+ */
+ if ( $openParens > 0 ) {
+ $tokensAsString .= str_repeat( ')', $openParens );
+ } else {
+ $tokensAsString = str_repeat( '(', abs( $openParens ) ) . $tokensAsString;
+ }
+ }
+
+ $time = @eval( "return $tokensAsString;" ); // phpcs:ignore Squiz.PHP.Eval,WordPress.PHP.NoSilencedErrors -- No harm here.
+
+ if ( $time === false ) {
+ /*
+ * The eval resulted in a parse error. This will only happen for backfilled
+ * arithmetic operator tokens, like T_POW, on PHP versions in which the token
+ * did not exist. In that case, flag for manual inspection.
+ */
+ $this->phpcsFile->addWarning( $message, $reportPtr, $error_code, $data );
+ return;
+ }
+
+ if ( $time < 300 && (int) $time !== 0 ) {
+ $message = 'Low cache expiry time of %s seconds detected. It is recommended to have 300 seconds or more.';
+ $data = [ $time ];
+
+ if ( (string) $time !== $tokensAsString ) {
+ $message .= ' Found: "%s"';
+ $data[] = $tokensAsString;
+ }
+
+ $this->phpcsFile->addWarning( $message, $reportPtr, 'LowCacheTime', $data );
}
}
}
diff --git a/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php b/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php
index 124b3aeb..9e23fc4f 100644
--- a/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php
@@ -14,7 +14,7 @@
/**
* Flag returning high or infinite posts_per_page.
*
- * @link https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#no-limit-queries
+ * @link https://docs.wpvip.com/technical-references/code-review/#no-limit-queries
*
* @package VIPCS\WordPressVIPMinimum
*
diff --git a/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php b/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php
index 47d3c604..e6e64c6f 100644
--- a/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php
@@ -14,7 +14,7 @@
/**
* Flag using orderby => rand.
*
- * @link https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#order-by-rand
+ * @link https://docs.wpvip.com/technical-references/code-review/#order-by-rand
*
* @package VIPCS\WordPressVIPMinimum
*
diff --git a/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php b/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php
index 9e828677..dea5fd5f 100644
--- a/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php
@@ -18,16 +18,14 @@ class RegexpCompareSniff extends AbstractArrayAssignmentRestrictionsSniff {
/**
* Groups of variables to restrict.
- * This should be overridden in extending classes.
*
* Example: groups => array(
- * 'wpdb' => array(
- * 'type' => 'error' | 'warning',
- * 'message' => 'Dont use this one please!',
- * 'variables' => array( '$val', '$var' ),
- * 'object_vars' => array( '$foo->bar', .. ),
- * 'array_members' => array( '$foo['bar']', .. ),
- * )
+ * 'groupname' => array(
+ * 'type' => 'error' | 'warning',
+ * 'message' => 'Dont use this one please!',
+ * 'keys' => array( 'key1', 'another_key' ),
+ * 'callback' => array( 'class', 'method' ), // Optional.
+ * )
* )
*
* @return array
diff --git a/WordPressVIPMinimum/Sniffs/Performance/RemoteRequestTimeoutSniff.php b/WordPressVIPMinimum/Sniffs/Performance/RemoteRequestTimeoutSniff.php
index e0c17f95..974532f4 100644
--- a/WordPressVIPMinimum/Sniffs/Performance/RemoteRequestTimeoutSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Performance/RemoteRequestTimeoutSniff.php
@@ -18,16 +18,14 @@ class RemoteRequestTimeoutSniff extends AbstractArrayAssignmentRestrictionsSniff
/**
* Groups of variables to restrict.
- * This should be overridden in extending classes.
*
* Example: groups => array(
- * 'wpdb' => array(
- * 'type' => 'error' | 'warning',
- * 'message' => 'Dont use this one please!',
- * 'variables' => array( '$val', '$var' ),
- * 'object_vars' => array( '$foo->bar', .. ),
- * 'array_members' => array( '$foo['bar']', .. ),
- * )
+ * 'groupname' => array(
+ * 'type' => 'error' | 'warning',
+ * 'message' => 'Dont use this one please!',
+ * 'keys' => array( 'key1', 'another_key' ),
+ * 'callback' => array( 'class', 'method' ), // Optional.
+ * )
* )
*
* @return array
diff --git a/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php b/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php
index 071dc641..c9c815be 100644
--- a/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php
@@ -51,7 +51,7 @@ class TaxonomyMetaInOptionsSniff extends Sniff {
* @return array
*/
public function register() {
- return Tokens::$functionNameTokens;
+ return [ T_STRING ];
}
/**
diff --git a/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php b/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php
index 949e3f5b..9b15ef63 100644
--- a/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php
@@ -8,7 +8,7 @@
namespace WordPressVIPMinimum\Sniffs\Performance;
-use WordPressVIPMinimum\Sniffs\Sniff;
+use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff;
use PHP_CodeSniffer\Util\Tokens;
/**
@@ -16,7 +16,7 @@
*
* @package VIPCS\WordPressVIPMinimum
*/
-class WPQueryParamsSniff extends Sniff {
+class WPQueryParamsSniff extends AbstractArrayAssignmentRestrictionsSniff {
/**
* Returns an array of tokens this test wants to listen for.
@@ -24,8 +24,38 @@ class WPQueryParamsSniff extends Sniff {
* @return array
*/
public function register() {
+ $targets = parent::register();
+
+ // Add the target for the "old" implementation.
+ $targets[] = T_CONSTANT_ENCAPSED_STRING;
+
+ return $targets;
+ }
+
+ /**
+ * Groups of variables to restrict.
+ * This should be overridden in extending classes.
+ *
+ * Example: groups => array(
+ * 'groupname' => array(
+ * 'type' => 'error' | 'warning',
+ * 'message' => 'Dont use this one please!',
+ * 'keys' => array( 'key1', 'another_key' ),
+ * 'callback' => array( 'class', 'method' ), // Optional.
+ * )
+ * )
+ *
+ * @return array
+ */
+ public function getGroups() {
return [
- T_CONSTANT_ENCAPSED_STRING,
+ 'PostNotIn' => [
+ 'type' => 'warning',
+ 'message' => 'Using `exclude`, which is subsequently used by `post__not_in`, should be done with caution, see https://docs.wpvip.com/how-tos/improve-performance-by-removing-usage-of-post__not_in/ for more information.',
+ 'keys' => [
+ 'exclude',
+ ],
+ ],
];
}
@@ -43,17 +73,32 @@ public function process_token( $stackPtr ) {
$next_token = $this->phpcsFile->findNext( array_merge( Tokens::$emptyTokens, [ T_EQUAL, T_CLOSE_SQUARE_BRACKET, T_DOUBLE_ARROW ] ), $stackPtr + 1, null, true );
if ( $this->tokens[ $next_token ]['code'] === T_TRUE ) {
- // WordPress.com: https://lobby.vip.wordpress.com/wordpress-com-documentation/uncached-functions/.
- // VIP Go: https://wpvip.com/documentation/vip-go/uncached-functions/.
+ // https://docs.wpvip.com/technical-references/caching/uncached-functions/.
$message = 'Setting `suppress_filters` to `true` is prohibited.';
$this->phpcsFile->addError( $message, $stackPtr, 'SuppressFiltersTrue' );
}
}
if ( trim( $this->tokens[ $stackPtr ]['content'], '\'' ) === 'post__not_in' ) {
- $message = 'Using `post__not_in` should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information.';
+ $message = 'Using `post__not_in` should be done with caution, see https://docs.wpvip.com/how-tos/improve-performance-by-removing-usage-of-post__not_in/ for more information.';
$this->phpcsFile->addWarning( $message, $stackPtr, 'PostNotIn' );
}
+
+ parent::process_token( $stackPtr );
+ }
+
+ /**
+ * Callback to process a confirmed key which doesn't need custom logic, but should always error.
+ *
+ * @param string $key Array index / key.
+ * @param mixed $val Assigned value.
+ * @param int $line Token line.
+ * @param array $group Group definition.
+ * @return mixed FALSE if no match, TRUE if matches, STRING if matches
+ * with custom error message passed to ->process().
+ */
+ public function callback( $key, $val, $line, $group ) {
+ return true;
}
}
diff --git a/WordPressVIPMinimum/Sniffs/Security/ExitAfterRedirectSniff.php b/WordPressVIPMinimum/Sniffs/Security/ExitAfterRedirectSniff.php
index 5d76976a..dfacc425 100644
--- a/WordPressVIPMinimum/Sniffs/Security/ExitAfterRedirectSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Security/ExitAfterRedirectSniff.php
@@ -24,7 +24,7 @@ class ExitAfterRedirectSniff extends Sniff {
* @return array
*/
public function register() {
- return Tokens::$functionNameTokens;
+ return [ T_STRING ];
}
/**
diff --git a/WordPressVIPMinimum/Sniffs/Security/ProperEscapingFunctionSniff.php b/WordPressVIPMinimum/Sniffs/Security/ProperEscapingFunctionSniff.php
index f94abdf2..9db2983e 100644
--- a/WordPressVIPMinimum/Sniffs/Security/ProperEscapingFunctionSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Security/ProperEscapingFunctionSniff.php
@@ -23,10 +23,56 @@ class ProperEscapingFunctionSniff extends Sniff {
*
* @var array
*/
- public $escaping_functions = [
- 'esc_url',
- 'esc_attr',
- 'esc_html',
+ protected $escaping_functions = [
+ 'esc_url' => 'url',
+ 'esc_attr' => 'attr',
+ 'esc_attr__' => 'attr',
+ 'esc_attr_x' => 'attr',
+ 'esc_attr_e' => 'attr',
+ 'esc_html' => 'html',
+ 'esc_html__' => 'html',
+ 'esc_html_x' => 'html',
+ 'esc_html_e' => 'html',
+ ];
+
+ /**
+ * List of tokens we can skip.
+ *
+ * @var array
+ */
+ private $echo_or_concat_tokens =
+ [
+ T_ECHO => T_ECHO,
+ T_OPEN_TAG => T_OPEN_TAG,
+ T_OPEN_TAG_WITH_ECHO => T_OPEN_TAG_WITH_ECHO,
+ T_STRING_CONCAT => T_STRING_CONCAT,
+ T_COMMA => T_COMMA,
+ T_NS_SEPARATOR => T_NS_SEPARATOR,
+ ];
+
+ /**
+ * List of attributes associated with url outputs.
+ *
+ * @var array
+ */
+ private $url_attrs = [
+ 'href',
+ 'src',
+ 'url',
+ 'action',
+ ];
+
+ /**
+ * List of syntaxes for inside attribute detection.
+ *
+ * @var array
+ */
+ private $attr_endings = [
+ '=',
+ '="',
+ "='",
+ "=\\'",
+ '=\\"',
];
/**
@@ -35,7 +81,9 @@ class ProperEscapingFunctionSniff extends Sniff {
* @return array
*/
public function register() {
- return Tokens::$functionNameTokens;
+ $this->echo_or_concat_tokens += Tokens::$emptyTokens;
+
+ return [ T_STRING ];
}
/**
@@ -47,47 +95,47 @@ public function register() {
*/
public function process_token( $stackPtr ) {
- if ( in_array( $this->tokens[ $stackPtr ]['content'], $this->escaping_functions, true ) === false ) {
+ $function_name = strtolower( $this->tokens[ $stackPtr ]['content'] );
+
+ if ( isset( $this->escaping_functions[ $function_name ] ) === false ) {
return;
}
- $function_name = $this->tokens[ $stackPtr ]['content'];
+ $next_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+ if ( $next_non_empty === false || $this->tokens[ $next_non_empty ]['code'] !== T_OPEN_PARENTHESIS ) {
+ // Not a function call.
+ return;
+ }
- $echo_or_string_concat = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $stackPtr - 1, null, true );
+ $html = $this->phpcsFile->findPrevious( $this->echo_or_concat_tokens, $stackPtr - 1, null, true );
- if ( $this->tokens[ $echo_or_string_concat ]['code'] === T_ECHO ) {
- // Very likely inline HTML with phpcsFile->findPrevious( Tokens::$emptyTokens, $echo_or_string_concat - 1, null, true );
+ // Use $textStringTokens b/c heredoc and nowdoc tokens will never be encountered in this context anyways..
+ if ( $html === false || isset( Tokens::$textStringTokens[ $this->tokens[ $html ]['code'] ] ) === false ) {
+ return;
+ }
- if ( $this->tokens[ $php_open ]['code'] !== T_OPEN_TAG ) {
- return;
- }
+ $data = [ $function_name ];
- $html = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $php_open - 1, null, true );
+ $content = $this->tokens[ $html ]['content'];
+ if ( isset( Tokens::$stringTokens[ $this->tokens[ $html ]['code'] ] ) === true ) {
+ $content = Sniff::strip_quotes( $content );
+ }
- if ( $this->tokens[ $html ]['code'] !== T_INLINE_HTML ) {
- return;
- }
- } elseif ( $this->tokens[ $echo_or_string_concat ]['code'] === T_STRING_CONCAT ) {
- // Very likely string concatenation mixing strings and functions/variables.
- $html = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $echo_or_string_concat - 1, null, true );
+ $escaping_type = $this->escaping_functions[ $function_name ];
- if ( $this->tokens[ $html ]['code'] !== T_CONSTANT_ENCAPSED_STRING ) {
- return;
- }
- } else {
- // Neither - bailing.
+ if ( $escaping_type === 'attr' && $this->is_outside_html_attr_context( $content ) ) {
+ $message = 'Wrong escaping function, using `%s()` in a context outside of HTML attributes may not escape properly.';
+ $this->phpcsFile->addError( $message, $html, 'notAttrEscAttr', $data );
return;
}
- $data = [ $function_name ];
-
- if ( $function_name !== 'esc_url' && $this->attr_expects_url( $this->tokens[ $html ]['content'] ) ) {
+ if ( $escaping_type !== 'url' && $this->attr_expects_url( $content ) ) {
$message = 'Wrong escaping function. href, src, and action attributes should be escaped by `esc_url()`, not by `%s()`.';
$this->phpcsFile->addError( $message, $stackPtr, 'hrefSrcEscUrl', $data );
return;
}
- if ( $function_name === 'esc_html' && $this->is_html_attr( $this->tokens[ $html ]['content'] ) ) {
+
+ if ( $escaping_type === 'html' && $this->is_html_attr( $content ) ) {
$message = 'Wrong escaping function. HTML attributes should be escaped by `esc_attr()`, not by `%s()`.';
$this->phpcsFile->addError( $message, $stackPtr, 'htmlAttrNotByEscHTML', $data );
return;
@@ -99,17 +147,12 @@ public function process_token( $stackPtr ) {
*
* @param string $content Haystack in which we look for an open attribute which exects a URL value.
*
- * @return bool True if string ends with open attribute which exects a URL value.
+ * @return bool True if string ends with open attribute which expects a URL value.
*/
public function attr_expects_url( $content ) {
$attr_expects_url = false;
- foreach ( [ 'href', 'src', 'url', 'action' ] as $attr ) {
- foreach ( [
- '="',
- "='",
- '=\'"', // The tokenizer does some fun stuff when it comes to mixing double and single quotes.
- '="\'', // The tokenizer does some fun stuff when it comes to mixing double and single quotes.
- ] as $ending ) {
+ foreach ( $this->url_attrs as $attr ) {
+ foreach ( $this->attr_endings as $ending ) {
if ( $this->endswith( $content, $attr . $ending ) === true ) {
$attr_expects_url = true;
break;
@@ -128,12 +171,7 @@ public function attr_expects_url( $content ) {
*/
public function is_html_attr( $content ) {
$is_html_attr = false;
- foreach ( [
- '="',
- "='",
- '=\'"', // The tokenizer does some fun stuff when it comes to mixing double and single quotes.
- '="\'', // The tokenizer does some fun stuff when it comes to mixing double and single quotes.
- ] as $ending ) {
+ foreach ( $this->attr_endings as $ending ) {
if ( $this->endswith( $content, $ending ) === true ) {
$is_html_attr = true;
break;
@@ -142,11 +180,22 @@ public function is_html_attr( $content ) {
return $is_html_attr;
}
+ /**
+ * Tests whether an attribute escaping function is being used outside of an HTML tag.
+ *
+ * @param string $content Haystack where we look for the end of a HTML tag.
+ *
+ * @return bool True if the passed string ends a HTML tag.
+ */
+ public function is_outside_html_attr_context( $content ) {
+ return $this->endswith( trim( $content ), '>' );
+ }
+
/**
* A helper function which tests whether string ends with some other.
*
* @param string $haystack String which is being tested.
- * @param string $needle The substring, which we try to locate on the end of the $haystack.
+ * @param string $needle The substring, which we try to locate on the end of the $haystack.
*
* @return bool True if haystack ends with needle.
*/
diff --git a/WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php b/WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php
index 09159a07..3d57edcc 100644
--- a/WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php
@@ -24,7 +24,7 @@ class StaticStrreplaceSniff extends Sniff {
* @return array
*/
public function register() {
- return Tokens::$functionNameTokens;
+ return [ T_STRING ];
}
/**
diff --git a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php
index fb1c0af0..6ea36135 100644
--- a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php
@@ -8,6 +8,7 @@
namespace WordPressVIPMinimum\Sniffs\Security;
+use PHP_CodeSniffer\Util\Tokens;
use WordPressVIPMinimum\Sniffs\Sniff;
/**
@@ -17,6 +18,29 @@
*/
class UnderscorejsSniff extends Sniff {
+ /**
+ * Regex to match unescaped output notations containing variable interpolation
+ * and retrieve a code snippet.
+ *
+ * @var string
+ */
+ const UNESCAPED_INTERPOLATE_REGEX = '`<%=\s*(?:.+?%>|$)`';
+
+ /**
+ * Regex to match execute notations containing a print command
+ * and retrieve a code snippet.
+ *
+ * @var string
+ */
+ const UNESCAPED_PRINT_REGEX = '`<%\s*(?:print\s*\(.+?\)\s*;|__p\s*\+=.+?)\s*%>`';
+
+ /**
+ * Regex to match the "interpolate" keyword when used to overrule the ERB-style delimiters.
+ *
+ * @var string
+ */
+ const INTERPOLATE_KEYWORD_REGEX = '`(?:templateSettings\.interpolate|\.interpolate\s*=\s*/|interpolate\s*:\s*/)`';
+
/**
* A list of tokenizers this sniff supports.
*
@@ -30,12 +54,11 @@ class UnderscorejsSniff extends Sniff {
* @return array
*/
public function register() {
- return [
- T_CONSTANT_ENCAPSED_STRING,
- T_PROPERTY,
- T_INLINE_HTML,
- T_HEREDOC,
- ];
+ $targets = Tokens::$textStringTokens;
+ $targets[] = T_PROPERTY;
+ $targets[] = T_STRING;
+
+ return $targets;
}
/**
@@ -46,15 +69,91 @@ public function register() {
* @return void
*/
public function process_token( $stackPtr ) {
+ /*
+ * Ignore Gruntfile.js files as they are configuration, not code.
+ */
+ $file_name = $this->strip_quotes( $this->phpcsFile->getFileName() );
+ $file_name = strtolower( basename( $file_name ) );
+
+ if ( $file_name === 'gruntfile.js' ) {
+ return;
+ }
+
+ /*
+ * Check for delimiter change in JS files.
+ */
+ if ( $this->tokens[ $stackPtr ]['code'] === T_STRING
+ || $this->tokens[ $stackPtr ]['code'] === T_PROPERTY
+ ) {
+ if ( $this->phpcsFile->tokenizerType !== 'JS' ) {
+ // These tokens are only relevant for JS files.
+ return;
+ }
+
+ if ( $this->tokens[ $stackPtr ]['content'] !== 'interpolate' ) {
+ return;
+ }
+
+ // Check the context to prevent false positives.
+ if ( $this->tokens[ $stackPtr ]['code'] === T_STRING ) {
+ $prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true );
+ if ( $prev === false || $this->tokens[ $prev ]['code'] !== T_OBJECT_OPERATOR ) {
+ return;
+ }
+
+ $prevPrev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true );
+ $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+ if ( ( $prevPrev === false
+ || $this->tokens[ $prevPrev ]['code'] !== T_STRING
+ || $this->tokens[ $prevPrev ]['content'] !== 'templateSettings' )
+ && ( $next === false
+ || $this->tokens[ $next ]['code'] !== T_EQUAL )
+ ) {
+ return;
+ }
+ }
+
+ // Underscore.js delimiter change.
+ $message = 'Found Underscore.js delimiter change notation.';
+ $this->phpcsFile->addWarning( $message, $stackPtr, 'InterpolateFound' );
+
+ return;
+ }
+
+ $content = $this->strip_quotes( $this->tokens[ $stackPtr ]['content'] );
+
+ $match_count = preg_match_all( self::UNESCAPED_INTERPOLATE_REGEX, $content, $matches );
+ if ( $match_count > 0 ) {
+ foreach ( $matches[0] as $match ) {
+ if ( strpos( $match, '_.escape(' ) !== false ) {
+ continue;
+ }
+
+ // Underscore.js unescaped output.
+ $message = 'Found Underscore.js unescaped output notation: "%s".';
+ $data = [ $match ];
+ $this->phpcsFile->addWarning( $message, $stackPtr, 'OutputNotation', $data );
+ }
+ }
+
+ $match_count = preg_match_all( self::UNESCAPED_PRINT_REGEX, $content, $matches );
+ if ( $match_count > 0 ) {
+ foreach ( $matches[0] as $match ) {
+ if ( strpos( $match, '_.escape(' ) !== false ) {
+ continue;
+ }
- if ( strpos( $this->tokens[ $stackPtr ]['content'], '<%=' ) !== false ) {
- // Underscore.js unescaped output.
- $message = 'Found Underscore.js unescaped output notation: "<%=".';
- $this->phpcsFile->addWarning( $message, $stackPtr, 'OutputNotation' );
+ // Underscore.js unescaped output.
+ $message = 'Found Underscore.js unescaped print execution: "%s".';
+ $data = [ $match ];
+ $this->phpcsFile->addWarning( $message, $stackPtr, 'PrintExecution', $data );
+ }
}
- if ( strpos( $this->tokens[ $stackPtr ]['content'], 'interpolate' ) !== false ) {
- // Underscore.js unescaped output.
+ if ( $this->phpcsFile->tokenizerType !== 'JS'
+ && preg_match( self::INTERPOLATE_KEYWORD_REGEX, $content ) > 0
+ ) {
+ // Underscore.js delimiter change.
$message = 'Found Underscore.js delimiter change notation.';
$this->phpcsFile->addWarning( $message, $stackPtr, 'InterpolateFound' );
}
diff --git a/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php b/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php
index 6945c2f4..6b985f07 100644
--- a/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php
+++ b/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php
@@ -15,7 +15,7 @@
/**
* Discourages removal of the admin bar.
*
- * @link https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#removing-the-admin-bar
+ * @link https://docs.wpvip.com/technical-references/code-review/vip-warnings/#h-removing-the-admin-bar
*
* @package VIPCS\WordPressVIPMinimum
*
@@ -319,7 +319,7 @@ public function process_text_for_style( $stackPtr, $file_name ) {
/**
* Processes this test for T_STYLE tokens in CSS files.
*
- * @param int $stackPtr The position of the current token in the stack passed in $tokens.
+ * @param int $stackPtr The position of the current token in the stack passed in $tokens.
*
* @return void
*/
@@ -370,7 +370,7 @@ protected function process_css_style( $stackPtr ) {
/**
* Consolidated violation.
*
- * @param int $stackPtr The position of the current token in the stack passed in $tokens.
+ * @param int $stackPtr The position of the current token in the stack passed in $tokens.
*/
private function addHidingDetectedError( $stackPtr ) {
$message = 'Hiding of the admin bar is not allowed.';
diff --git a/WordPressVIPMinimum/Sniffs/Variables/RestrictedVariablesSniff.php b/WordPressVIPMinimum/Sniffs/Variables/RestrictedVariablesSniff.php
index 7b6d7917..fe19452d 100644
--- a/WordPressVIPMinimum/Sniffs/Variables/RestrictedVariablesSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Variables/RestrictedVariablesSniff.php
@@ -37,13 +37,11 @@ class RestrictedVariablesSniff extends AbstractVariableRestrictionsSniff {
*/
public function getGroups() {
return [
- // @link https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#wp_users-and-user_meta
'user_meta' => [
'type' => 'error',
- 'message' => 'Usage of users/usermeta tables is highly discouraged in VIP context, For storing user additional user metadata, you should look at User Attributes.',
+ 'message' => 'Usage of users tables is highly discouraged in VIP context',
'object_vars' => [
'$wpdb->users',
- '$wpdb->usermeta',
],
],
'session' => [
@@ -54,10 +52,10 @@ public function getGroups() {
],
],
- // @link https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#caching-constraints
+ // @link https://docs.wpvip.com/technical-references/code-review/vip-errors/#h-cache-constraints
'cache_constraints' => [
'type' => 'warning',
- 'message' => 'Due to using Batcache, server side based client related logic will not work, use JS instead.',
+ 'message' => 'Due to server-side caching, server-side based client related logic might not work. We recommend implementing client side logic in JavaScript instead.',
'variables' => [
'$_COOKIE',
],
diff --git a/WordPressVIPMinimum/Sniffs/Variables/VariableAnalysisSniff.php b/WordPressVIPMinimum/Sniffs/Variables/VariableAnalysisSniff.php
index 1296ea31..18e1ed27 100644
--- a/WordPressVIPMinimum/Sniffs/Variables/VariableAnalysisSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Variables/VariableAnalysisSniff.php
@@ -17,7 +17,7 @@
use PHP_CodeSniffer\Files\File;
/**
- * Checks the for undefined function variables.
+ * Checks for undefined function variables.
*
* This sniff checks that all function variables
* are defined in the function body.
@@ -46,21 +46,6 @@ class VariableAnalysisSniff extends \VariableAnalysis\Sniffs\CodeAnalysis\Variab
'FoundPropertyForDeprecatedSniff' => false,
];
- /**
- * Returns an array of tokens this test wants to listen for.
- *
- * @return int[]
- */
- public function register() {
- return [
- T_VARIABLE,
- T_DOUBLE_QUOTED_STRING,
- T_HEREDOC,
- T_CLOSE_CURLY_BRACKET,
- T_STRING,
- ];
- }
-
/**
* Don't use.
*
diff --git a/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.inc b/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.inc
index a330da69..ec4ab2f4 100644
--- a/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.inc
+++ b/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.inc
@@ -4,6 +4,30 @@ if ( ! defined( 'WPCOM_VIP' ) ) { // Okay.
define( 'WPCOM_VIP', true ); // Okay.
}
-if ( ! defined( WPCOM_VIP ) ) { // NOK.
- define( WPCOM_VIP ); // NOK.
-}
\ No newline at end of file
+if ( ! defined( WPCOM_VIP ) ) { // Error.
+ define( WPCOM_VIP, true ); // Error.
+}
+
+namespace Foo\Bar;
+const REST_ALLOWED_META_PREFIXES = [ 'foo-', 'bar-', 'baz-' ];
+if ( defined( __NAMESPACE__ . '\REST_ALLOWED_META_PREFIXES' ) && in_array( 'foo-', REST_ALLOWED_META_PREFIXES, true ) ) { // Ok.
+ define( __NAMESPACE__ . '\\' . REST_ALLOWED_META_PREFIXES[1], $value ); // OK.
+}
+
+define( __NAMESPACE__ . '\PLUGIN_URL', \plugins_url( '/', __FILE__ ) ); // OK.
+if ( defined( __NAMESPACE__ . '\\LOADED' ) ) {} // OK.
+
+if ( defined( $obj->constant_name_property ) === false ) { // OK.
+ define( $variable_containing_constant_name, $constant_value ); // OK.
+}
+
+if ( defined( MY_PREFIX . '_CONSTANT_NAME' ) === false ) { // OK.
+ define( 'PREFIX_' . $variable_part, $constant_value ); // OK.
+}
+
+if ( ! defined($generator->get()) { // OK.
+ define( $generator->getLast(), 'value'); // OK.
+}
+
+$defined = defined(); // OK, ignore.
+$defined = defined( /*comment*/ ); // OK, ignore.
diff --git a/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.inc b/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.inc
index b225a4a7..63dcf460 100644
--- a/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.inc
+++ b/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.inc
@@ -25,4 +25,13 @@ require_once "my_file.php"; // Not absolute path.
require '../my_file.php'; // Not absolute path.
require '../../my_file.php'; // Not absolute path.
include( 'http://www.google.com/bad_file.php' ); // External URL.
-include_once("http://www.google.com/bad_file.php"); // External URL.
\ No newline at end of file
+include_once("http://www.google.com/bad_file.php"); // External URL.
+
+// Allowed keywords
+include 'https://path.com/bad_file.php'; // Error - external URL with keyword from $allowedKeywords.
+require $path; // Warning - custom variable with keyword from $allowedKeywords.
+include_once dir_function(); // Error - custom functionm with keyword from $allowedKeywords.
+require CUSTOM_CONSTANT_DIR . 'file.php'; // OK.
+require_once ( VIPCS_PATH ) . 'file.php'; // OK.
+include_once
+ DIR_CUSTOM , 'file.php'; // OK.
\ No newline at end of file
diff --git a/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php b/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php
index 3253a83a..15aed707 100644
--- a/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php
+++ b/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php
@@ -29,6 +29,7 @@ public function getErrorList() {
26 => 1,
27 => 1,
28 => 1,
+ 31 => 1,
];
}
@@ -43,6 +44,8 @@ public function getWarningList() {
19 => 1,
20 => 1,
21 => 1,
+ 32 => 1,
+ 33 => 1,
];
}
diff --git a/WordPressVIPMinimum/Tests/Files/IncludingNonPHPFileUnitTest.inc b/WordPressVIPMinimum/Tests/Files/IncludingNonPHPFileUnitTest.inc
index ae1e4cf0..b4226a25 100644
--- a/WordPressVIPMinimum/Tests/Files/IncludingNonPHPFileUnitTest.inc
+++ b/WordPressVIPMinimum/Tests/Files/IncludingNonPHPFileUnitTest.inc
@@ -2,45 +2,45 @@
require_once __DIR__ . "/my_file.php"; // OK.
-require_once "my_file.php"; // OK.
+require "my_file.php"; // OK.
-include( __DIR__ . "/my_file.php" ); // OK.
+include_once( __DIR__ . "/my_file.php" ); // OK.
-include ( MY_CONSTANT . "my_file.php" ); // OK.
+include ( MY_CONSTANT . "my_file.INC" ); // OK.
require_once ( MY_CONSTANT . "my_file.php" ); // OK.
-include( locate_template('index-loop.php') ); // OK.
+Include( locate_template('index-loop.PHP') ); // OK.
-require_once __DIR__ . "/my_file.svg"; // NOK.
+require_once __DIR__ . "/my_file.SVG"; // NOK.
-require_once "my_file.svg"; // NOK.
+Require_Once "my_file.svg"; // NOK.
include( __DIR__ . "/my_file.svg" ); // NOK.
include ( MY_CONSTANT . "my_file.svg" ); // NOK.
-require_once ( MY_CONSTANT . "my_file.svg" ); // NOK.
+require ( MY_CONSTANT . "my_file.svg" ); // NOK.
include( locate_template('index-loop.svg') ); // NOK.
-require_once __DIR__ . "/my_file.css"; // NOK.
+require_once __DIR__ . "/my_file.CSS"; // NOK.
require_once "my_file.css"; // NOK.
-include( __DIR__ . "/my_file.css" ); // NOK.
+include_once( __DIR__ . "/my_file.css" ); // NOK.
include ( MY_CONSTANT . "my_file.css" ); // NOK.
require_once ( MY_CONSTANT . "my_file.css" ); // NOK.
-include( locate_template('index-loop.css') ); // NOK.
+include( locate_template('index-loop.Css') ); // NOK.
-require_once __DIR__ . "/my_file.csv"; // NOK.
+REQUIRE_ONCE __DIR__ . "/my_file.csv"; // NOK.
require_once "my_file.inc"; // OK.
-include( __DIR__ . "/my_file.csv" ); // NOK.
+include( __DIR__ . "/my_file.CSV" ); // NOK.
include ( MY_CONSTANT . "my_file.csv" ); // NOK.
@@ -50,4 +50,36 @@ include( locate_template('index-loop.csv') ); // NOK.
echo file_get_contents( 'index-loop.svg' ); // XSS OK.
-echo file_get_contents( 'index-loop.css' ); // XSS OK.
\ No newline at end of file
+echo file_get_contents( 'index-loop.css' ); // XSS OK.
+
+include_once 'path/to/geoip.phar'; // OK.
+
+require dirname(__DIR__) . '/externals/aws-sdk.phar'; // OK.
+
+require "$path/$file.inc"; // OK.
+
+require "$path/$file.css"; // NOK.
+
+include_once $path . '/' . "$file.js" ?>
+ 1,
47 => 1,
49 => 1,
+ 61 => 1,
+ 63 => 1,
+ 69 => 1,
+ 72 => 1,
+ 75 => 1,
+ 77 => 1,
];
}
diff --git a/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.inc
index 7cde79e4..fc307b2f 100644
--- a/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.inc
+++ b/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.inc
@@ -6,10 +6,33 @@ function my_test() {
$my_notokay_func = 'extract';
-$my_notokay_func();
+$my_notokay_func(); // Bad.
$my_okay_func = 'my_test';
-$my_okay_func();
+$my_okay_func(); // OK.
+$test_with_comment /*comment*/ = 'func_get_args';
+$test_with_comment /*comment*/ (); // Bad.
+$test_getting_the_actual_value_1 = function_call( 'extract' );
+$test_getting_the_actual_value_1(); // OK. Unclear what the actual variable value will be.
+$test_getting_the_actual_value_2 = $array['compact'];
+$test_getting_the_actual_value_2(); // OK. Unclear what the actual variable value will be.
+
+$test_getting_the_actual_value_3 = 10 ?>
+