diff --git a/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php index 660bd0e1..36c92021 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php @@ -7,47 +7,47 @@ namespace WordPressVIPMinimum\Sniffs\Functions; +use PHP_CodeSniffer\Util\Tokens; use WordPressVIPMinimum\Sniffs\Sniff; /** - * This sniff enforces that certain functions are not - * dynamically called. + * This sniff enforces that certain functions are not dynamically called. * * An example: - * - * + * ```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 $function_names = [ + '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->function_names[ $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 blacklisted 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/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 ?> +
html
+ 1, + 9 => 1, + 15 => 1, + 35 => 1, ]; }