Merge pull request #862 from Automattic/feature/functions-dynamic-calls-minor-improvements

Author: GaryJones

WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php

@@ -46,20 +46,29 @@ class DynamicCallsSniff extends Sniff {
 	];
 
 	/**
-	 * Array of variable assignments encountered, along with their values.
+	 * Potential end tokens for which the end pointer has to be set back by one.
 	 *
-	 * Populated at run-time.
+	 * {@internal The PHPCS `findEndOfStatement()` method is not completely consistent
+	 * in how it returns the statement end. This is just a simple way to bypass
+	 * the inconsistency for our purposes.}
 	 *
-	 * @var array<string, string> The key is the name of the variable, the value, its assigned value.
+	 * @var array<int|string, true>
 	 */
-	private $variables_arr = [];
+	private $inclusiveStopPoints = [
+		T_COLON        => true,
+		T_COMMA        => true,
+		T_DOUBLE_ARROW => true,
+		T_SEMICOLON    => true,
+	];
 
 	/**
-	 * The position in the stack where the token was found.
+	 * Array of variable assignments encountered, along with their values.
+	 *
+	 * Populated at run-time.
 	 *
-	 * @var int
+	 * @var array<string, string> The key is the name of the variable, the value, its assigned value.
 	 */
-	private $stackPtr;
+	private $variables_arr = [];
 
 	/**
 	 * Returns the token types that this sniff is interested in.
@@ -78,49 +87,44 @@ public function register() {
 	 * @return void
 	 */
 	public function process_token( $stackPtr ) {
-		$this->stackPtr = $stackPtr;
-
 		// First collect all variables encountered and their values.
-		$this->collect_variables();
+		$this->collect_variables( $stackPtr );
 
 		// Then find all dynamic calls, and report them.
-		$this->find_dynamic_calls();
+		$this->find_dynamic_calls( $stackPtr );
 	}
 
 	/**
 	 * Finds any variable-definitions in the file being processed and stores them
 	 * internally in a private array.
 	 *
+	 * @param int $stackPtr The position in the stack where the token was found.
+	 *
 	 * @return void
 	 */
-	private function collect_variables() {
+	private function collect_variables( $stackPtr ) {
 
-		$current_var_name = $this->tokens[ $this->stackPtr ]['content'];
+		$current_var_name = $this->tokens[ $stackPtr ]['content'];
 
 		/*
 		 * Find assignments ( $foo = "bar"; ) by finding all non-whitespaces,
 		 * and checking if the first one is T_EQUAL.
 		 */
-		$t_item_key = $this->phpcsFile->findNext(
-			Tokens::$emptyTokens,
-			$this->stackPtr + 1,
-			null,
-			true,
-			null,
-			true
-		);
-
+		$t_item_key = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true );
 		if ( $t_item_key === false || $this->tokens[ $t_item_key ]['code'] !== 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;
+		$end_of_statement = $this->phpcsFile->findEndOfStatement( ( $t_item_key + 1 ) );
+		if ( isset( $this->inclusiveStopPoints[ $this->tokens[ $end_of_statement ]['code'] ] ) === true ) {
+			--$end_of_statement;
+		}
 
-		for ( $i = $t_item_key + 1; $i < $end_of_statement; $i++ ) {
+		$value_ptr = null;
+		for ( $i = $t_item_key + 1; $i <= $end_of_statement; $i++ ) {
 			if ( isset( Tokens::$emptyTokens[ $this->tokens[ $i ]['code'] ] ) === true ) {
 				continue;
 			}
@@ -160,9 +164,11 @@ private function collect_variables() {
 	 *
 	 * Report on this when found, using the name of the function in the message.
 	 *
+	 * @param int $stackPtr The position in the stack where the token was found.
+	 *
 	 * @return void
 	 */
-	private function find_dynamic_calls() {
+	private function find_dynamic_calls( $stackPtr ) {
 		// No variables detected; no basis for doing anything.
 		if ( empty( $this->variables_arr ) ) {
 			return;
@@ -172,20 +178,20 @@ private function find_dynamic_calls() {
 		 * 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[ $stackPtr ]['content'] ] ) ) {
 			return;
 		}
 
 		/*
 		 * Check if we have an '(' next.
 		 */
-		$next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $this->stackPtr + 1 ), null, true );
+		$next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
 		if ( $next === false || $this->tokens[ $next ]['code'] !== T_OPEN_PARENTHESIS ) {
 			return;
 		}
 
 		$message = 'Dynamic calling is not recommended in the case of %s().';
-		$data    = [ $this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ] ];
-		$this->phpcsFile->addError( $message, $this->stackPtr, 'DynamicCalls', $data );
+		$data    = [ $this->variables_arr[ $this->tokens[ $stackPtr ]['content'] ] ];
+		$this->phpcsFile->addError( $message, $stackPtr, 'DynamicCalls', $data );
 	}
 }

…Tests/Functions/DynamicCallsUnitTest.inc …sts/Functions/DynamicCallsUnitTest.1.incWordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.inc renamed to WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.1.inc WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.inc renamed to WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.1.inc

@@ -3,7 +3,7 @@
 function my_test() {
 	echo esc_html( "foo" );
 }
-
+$irrelevant = 10;
 
 $my_notokay_func = 'extract';
 $my_notokay_func(); // Bad.
@@ -34,5 +34,15 @@ $ensure_no_notices_are_thrown_on_parse_error = /*comment*/ ;
 $test_double_quoted_string = "assert";
 $test_double_quoted_string(); // Bad.
 
-// Intentional parse error. This has to be the last test in the file.
-$my_notokay_func
+function hasStaticVars() {
+	static $staticvar_foo = 'irrelevant', $staticvar_bar = 'func_num_args', $staticvar_baz = 'nothing';
+	$staticvar_foo(); // OK.
+	$staticvar_bar(); // Bad.
+	$staticvar_baz(); // OK.
+}
+
+function functionParams( $param_foo = 'func_get_arg', $param_bar = 'nothing', $param_baz = 'func_num_args') {
+	$param_foo(); // Bad.
+	$param_bar(); // OK.
+	$param_baz(); // Bad.
+}

WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.2.inc

@@ -0,0 +1,4 @@
+<?php
+
+// Intentional parse error. This has to be the last (only) test in the file.
+$my_notokay_func

WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.php

@@ -19,14 +19,26 @@ class DynamicCallsUnitTest extends AbstractSniffUnitTest {
 	/**
 	 * Returns the lines where errors 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 errors.
 	 */
-	public function getErrorList() {
-		return [
-			9  => 1,
-			15 => 1,
-			35 => 1,
-		];
+	public function getErrorList( $testFile = '' ) {
+
+		switch ( $testFile ) {
+			case 'DynamicCallsUnitTest.1.inc':
+				return [
+					9  => 1,
+					15 => 1,
+					35 => 1,
+					40 => 1,
+					45 => 1,
+					47 => 1,
+				];
+
+			default:
+				return [];
+		}
 	}
 
 	/**