Skip to content

Commit

Permalink
Move "interpolated variable" related utilities to dedicated `TextStri…
Browse files Browse the repository at this point in the history
…ngHelper`

The "interpolated variable" related utilities are only used by a small set of sniffs, so are better placed in a dedicated class.

This commit moves the `REGEX_COMPLEX_VARS` constant, the `get_interpolated_variables()` method and the `strip_interpolated_variables()` method to a new `WordPressCS\WordPress\Helpers\TextStringHelper` and starts using that class in the relevant sniffs.

In contrast to some of the other "move methods out of the Sniff class" PRs, these methods have been moved to a class and made `static` - instead of moved to a `trait`.
The reason for this difference is that the methods in other "moves" are setting properties which the sniff classes would need access to, while these methods are 100% stand-alone.

**Note**:
It is expected for PHPCSUtils to have dedicated methods for the same at some point in the future. If/when those methods become available, it is recommended for the sniffs to start using the PHPCSUtils methods and for this class to be deprecated and removed in the next major release.

Also note that PHP 8.2 will be making changes to the kind of interpolation supported by PHP. This will need further investigation at a future point in time (probably in PHPCSUtils rather than here), if nothing else to ensure that what was supported in PHP prior to PHP 8.2 is fully supported by these methods (which is probably not the case at this moment).
Ref: https://wiki.php.net/rfc/deprecate_dollar_brace_string_interpolation

Related to 1465
  • Loading branch information
jrfnl committed May 16, 2022
1 parent 1702977 commit 76f3376
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 61 deletions.
79 changes: 79 additions & 0 deletions WordPress/Helpers/TextStringHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<?php
/**
* WordPress Coding Standard.
*
* @package WPCS\WordPressCodingStandards
* @link https://github.com/WordPress/WordPress-Coding-Standards
* @license https://opensource.org/licenses/MIT MIT
*/

namespace WordPressCS\WordPress\Helpers;

/**
* Helper utilities for handling text strings.
*
* {@internal The functionality in this class will likely be replaced at some point in
* the future by functions from PHPCSUtils.}
*
* @package WPCS\WordPressCodingStandards
* @since 3.0.0 The constant and methods in this class were previously contained in the
* `WordPressCS\WordPress\Sniff` class and have been moved here.
*/
final class TextStringHelper {

/**
* Regex to get complex variables from T_DOUBLE_QUOTED_STRING or T_HEREDOC.
*
* @since 0.14.0
* @since 3.0.0 Moved from the Sniff class to this class.
*
* @var string
*/
const REGEX_COMPLEX_VARS = '`(?:(\{)?(?<!\\\\)\$)?(\{)?(?<!\\\\)\$(\{)?(?P<varname>[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)(?:->\$?(?P>varname)|\[[^\]]+\]|::\$?(?P>varname)|\([^\)]*\))*(?(3)\}|)(?(2)\}|)(?(1)\}|)`';

/**
* Get the interpolated variable names from a string.
*
* Check if '$' is followed by a valid variable name, and that it is not preceded by an escape sequence.
*
* @since 0.9.0
* @since 3.0.0 - Moved from the Sniff class to this class.
* - Visibility is now `public` (was `protected`) and the method `static`.
*
* @param string $string The contents of a T_DOUBLE_QUOTED_STRING or T_HEREDOC token.
*
* @return array Variable names (without '$' sigil).
*/
public static function get_interpolated_variables( $string ) {
$variables = array();
if ( preg_match_all( '/(?P<backslashes>\\\\*)\$(?P<symbol>\w+)/', $string, $match_sets, \PREG_SET_ORDER ) ) {
foreach ( $match_sets as $matches ) {
if ( ! isset( $matches['backslashes'] ) || ( \strlen( $matches['backslashes'] ) % 2 ) === 0 ) {
$variables[] = $matches['symbol'];
}
}
}
return $variables;
}

/**
* Strip variables from an arbitrary double quoted/heredoc string.
*
* Intended for use with the contents of a T_DOUBLE_QUOTED_STRING or T_HEREDOC token.
*
* @since 0.14.0
* @since 3.0.0 - Moved from the Sniff class to this class.
* - The method was made `static`.
*
* @param string $string The raw string.
*
* @return string String without variables in it.
*/
public static function strip_interpolated_variables( $string ) {
if ( strpos( $string, '$' ) === false ) {
return $string;
}

return preg_replace( self::REGEX_COMPLEX_VARS, '', $string );
}
}
51 changes: 0 additions & 51 deletions WordPress/Sniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,6 @@
*/
abstract class Sniff implements PHPCS_Sniff {

/**
* Regex to get complex variables from T_DOUBLE_QUOTED_STRING or T_HEREDOC.
*
* @since 0.14.0
*
* @var string
*/
const REGEX_COMPLEX_VARS = '`(?:(\{)?(?<!\\\\)\$)?(\{)?(?<!\\\\)\$(\{)?(?P<varname>[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)(?:->\$?(?P>varname)|\[[^\]]+\]|::\$?(?P>varname)|\([^\)]*\))*(?(3)\}|)(?(2)\}|)(?(1)\}|)`';

/**
* List of the functions which verify nonces.
*
Expand Down Expand Up @@ -2063,48 +2054,6 @@ protected function is_in_array_comparison( $stackPtr ) {
return false;
}

/**
* Get the interpolated variable names from a string.
*
* Check if '$' is followed by a valid variable name, and that it is not preceded by an escape sequence.
*
* @since 0.9.0
*
* @param string $string The contents of a T_DOUBLE_QUOTED_STRING or T_HEREDOC token.
*
* @return array Variable names (without '$' sigil).
*/
protected function get_interpolated_variables( $string ) {
$variables = array();
if ( preg_match_all( '/(?P<backslashes>\\\\*)\$(?P<symbol>\w+)/', $string, $match_sets, \PREG_SET_ORDER ) ) {
foreach ( $match_sets as $matches ) {
if ( ! isset( $matches['backslashes'] ) || ( \strlen( $matches['backslashes'] ) % 2 ) === 0 ) {
$variables[] = $matches['symbol'];
}
}
}
return $variables;
}

/**
* Strip variables from an arbitrary double quoted/heredoc string.
*
* Intended for use with the contents of a T_DOUBLE_QUOTED_STRING or T_HEREDOC token.
*
* @since 0.14.0
*
* @param string $string The raw string.
*
* @return string String without variables in it.
*/
public function strip_interpolated_variables( $string ) {
if ( strpos( $string, '$' ) === false ) {
return $string;
}

return preg_replace( self::REGEX_COMPLEX_VARS, '', $string );
}

/**
* Checks whether this is a call to a $wpdb method that we want to sniff.
*
Expand Down
7 changes: 4 additions & 3 deletions WordPress/Sniffs/DB/PreparedSQLPlaceholdersSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@

namespace WordPressCS\WordPress\Sniffs\DB;

use WordPressCS\WordPress\Sniff;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\PassedParameters;
use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\Helpers\TextStringHelper;
use WordPressCS\WordPress\Sniff;

/**
* Check for incorrect use of the $wpdb->prepare method.
Expand Down Expand Up @@ -278,9 +279,9 @@ public function process_token( $stackPtr ) {
|| \T_HEREDOC === $this->tokens[ $i ]['code']
) {
// Only interested in actual query text, so strip out variables.
$stripped_content = $this->strip_interpolated_variables( $content );
$stripped_content = TextStringHelper::strip_interpolated_variables( $content );
if ( $stripped_content !== $content ) {
$interpolated_vars = $this->get_interpolated_variables( $content );
$interpolated_vars = TextStringHelper::get_interpolated_variables( $content );
$vars_without_wpdb = array_diff( $interpolated_vars, array( 'wpdb' ) );
$content = $stripped_content;

Expand Down
5 changes: 3 additions & 2 deletions WordPress/Sniffs/DB/PreparedSQLSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@

namespace WordPressCS\WordPress\Sniffs\DB;

use WordPressCS\WordPress\Sniff;
use PHP_CodeSniffer\Util\Tokens;
use WordPressCS\WordPress\Helpers\TextStringHelper;
use WordPressCS\WordPress\Sniff;

/**
* Sniff for prepared SQL.
Expand Down Expand Up @@ -138,7 +139,7 @@ public function process_token( $stackPtr ) {
) {

$bad_variables = array_filter(
$this->get_interpolated_variables( $this->tokens[ $this->i ]['content'] ),
TextStringHelper::get_interpolated_variables( $this->tokens[ $this->i ]['content'] ),
function ( $symbol ) {
return ( 'wpdb' !== $symbol );
}
Expand Down
5 changes: 3 additions & 2 deletions WordPress/Sniffs/NamingConventions/ValidPostTypeSlugSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@

namespace WordPressCS\WordPress\Sniffs\NamingConventions;

use WordPressCS\WordPress\AbstractFunctionParameterSniff;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\AbstractFunctionParameterSniff;
use WordPressCS\WordPress\Helpers\TextStringHelper;

/**
* Validates post type names.
Expand Down Expand Up @@ -161,7 +162,7 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
'PartiallyDynamic',
$data
);
$post_type = $this->strip_interpolated_variables( $post_type );
$post_type = TextStringHelper::strip_interpolated_variables( $post_type );
}

if ( preg_match( self::POST_TYPE_CHARACTER_WHITELIST, $post_type ) === 0 ) {
Expand Down
5 changes: 3 additions & 2 deletions WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@

namespace WordPressCS\WordPress\Sniffs\Security;

use WordPressCS\WordPress\Sniff;
use PHP_CodeSniffer\Util\Tokens;
use WordPressCS\WordPress\Helpers\TextStringHelper;
use WordPressCS\WordPress\Sniff;

/**
* Flag any non-validated/sanitized input ( _GET / _POST / etc. ).
Expand Down Expand Up @@ -100,7 +101,7 @@ public function process_token( $stackPtr ) {
function ( $symbol ) {
return '$' . $symbol;
},
$this->get_interpolated_variables( $this->tokens[ $stackPtr ]['content'] )
TextStringHelper::get_interpolated_variables( $this->tokens[ $stackPtr ]['content'] )
);
foreach ( array_intersect( $interpolated_variables, $superglobals ) as $bad_variable ) {
$this->phpcsFile->addError( 'Detected usage of a non-sanitized, non-validated input variable %s: %s', $stackPtr, 'InputNotValidatedNotSanitized', array( $bad_variable, $this->tokens[ $stackPtr ]['content'] ) );
Expand Down
3 changes: 2 additions & 1 deletion WordPress/Sniffs/WP/I18nSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use PHPCSUtils\Utils\TextStrings;
use XMLReader;
use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff;
use WordPressCS\WordPress\Helpers\TextStringHelper;

/**
* Makes sure WP internationalization functions are used properly.
Expand Down Expand Up @@ -470,7 +471,7 @@ protected function check_argument_tokens( $context ) {
}

if ( \T_DOUBLE_QUOTED_STRING === $tokens[0]['code'] || \T_HEREDOC === $tokens[0]['code'] ) {
$interpolated_variables = $this->get_interpolated_variables( $content );
$interpolated_variables = TextStringHelper::get_interpolated_variables( $content );
foreach ( $interpolated_variables as $interpolated_variable ) {
$code = $this->string_to_errorcode( 'InterpolatedVariable' . ucfirst( $arg_name ) );
$this->addMessage( 'The $%s arg must not contain interpolated variables. Found "$%s".', $stack_ptr, $is_error, $code, array( $arg_name, $interpolated_variable ) );
Expand Down

0 comments on commit 76f3376

Please sign in to comment.