diff --git a/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php b/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php index a00f0cf2ad..50ba9c7acb 100644 --- a/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php +++ b/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php @@ -21,6 +21,7 @@ * @since 0.12.0 * @since 0.13.0 Class name changed: this class is now namespaced. * @since 1.2.0 Now also checks whether namespaces are prefixed. + * @since 2.2.0 Now also checks variables assigned via the list() construct. * * @uses \WordPressCS\WordPress\Sniff::$custom_test_class_whitelist */ @@ -200,11 +201,13 @@ public function register() { // Set the sniff targets. $targets = array( - \T_NAMESPACE => \T_NAMESPACE, - \T_FUNCTION => \T_FUNCTION, - \T_CONST => \T_CONST, - \T_VARIABLE => \T_VARIABLE, - \T_DOLLAR => \T_DOLLAR, // Variable variables. + \T_NAMESPACE => \T_NAMESPACE, + \T_FUNCTION => \T_FUNCTION, + \T_CONST => \T_CONST, + \T_VARIABLE => \T_VARIABLE, + \T_DOLLAR => \T_DOLLAR, // Variable variables. + \T_LIST => \T_LIST, + \T_OPEN_SHORT_ARRAY => \T_OPEN_SHORT_ARRAY, ); $targets += Tokens::$ooScopeTokens; // T_ANON_CLASS is only used for skipping over test classes. @@ -303,6 +306,11 @@ public function process_token( $stackPtr ) { return $this->process_variable_assignment( $stackPtr ); + } elseif ( \T_LIST === $this->tokens[ $stackPtr ]['code'] + || \T_OPEN_SHORT_ARRAY === $this->tokens[ $stackPtr ]['code'] + ) { + return $this->process_list_assignment( $stackPtr ); + } elseif ( \T_NAMESPACE === $this->tokens[ $stackPtr ]['code'] ) { $namespace_name = $this->get_declared_namespace_name( $stackPtr ); @@ -573,20 +581,24 @@ protected function process_variable_variable( $stackPtr ) { * Check that defined global variables are prefixed. * * @since 0.12.0 + * @since 2.2.0 Added $in_list parameter. * - * @param int $stackPtr The position of the current token in the stack. + * @param int $stackPtr The position of the current token in the stack. + * @param bool $in_list Whether or not this is a variable in a list assignment. + * Defaults to false. * * @return int|void Integer stack pointer to skip forward or void to continue * normal file processing. */ - protected function process_variable_assignment( $stackPtr ) { + protected function process_variable_assignment( $stackPtr, $in_list = false ) { /* * We're only concerned with variables which are being defined. * `is_assigment()` will not recognize property assignments, which is good in this case. * However it will also not recognize $b in `foreach( $a as $b )` as an assignment, so * we need a separate check for that. */ - if ( false === $this->is_assignment( $stackPtr ) + if ( false === $in_list + && false === $this->is_assignment( $stackPtr ) && false === $this->is_foreach_as( $stackPtr ) ) { return; @@ -642,7 +654,7 @@ protected function process_variable_assignment( $stackPtr ) { } } else { // Function parameters do not need to be prefixed. - if ( isset( $this->tokens[ $stackPtr ]['nested_parenthesis'] ) ) { + if ( false === $in_list && isset( $this->tokens[ $stackPtr ]['nested_parenthesis'] ) ) { foreach ( $this->tokens[ $stackPtr ]['nested_parenthesis'] as $opener => $closer ) { if ( isset( $this->tokens[ $opener ]['parenthesis_owner'] ) && ( \T_FUNCTION === $this->tokens[ $this->tokens[ $opener ]['parenthesis_owner'] ]['code'] @@ -655,7 +667,7 @@ protected function process_variable_assignment( $stackPtr ) { } // Properties in a class do not need to be prefixed. - if ( true === $this->is_class_property( $stackPtr ) ) { + if ( false === $in_list && true === $this->is_class_property( $stackPtr ) ) { return; } @@ -716,6 +728,35 @@ protected function process_variable_assignment( $stackPtr ) { } } + /** + * Check that global variables declared via a list construct are prefixed. + * + * @internal No need to take special measures for nested lists. Nested or not, + * each list part can only contain one variable being written to. + * + * @since 2.2.0 + * + * @param int $stackPtr The position of the current token in the stack. + * + * @return int|void Integer stack pointer to skip forward or void to continue + * normal file processing. + */ + protected function process_list_assignment( $stackPtr ) { + $list_open_close = $this->find_list_open_close( $stackPtr ); + if ( false === $list_open_close ) { + // Short array, not short list. + return; + } + + $var_pointers = $this->get_list_variables( $stackPtr, $list_open_close ); + foreach ( $var_pointers as $ptr ) { + $this->process_variable_assignment( $ptr, true ); + } + + // No need to re-examine these variables. + return $list_open_close['closer']; + } + /** * Process the parameters of a matched function. * diff --git a/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.1.inc b/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.1.inc index 3bf03276fe..9ff1b6100a 100644 --- a/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.1.inc +++ b/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.1.inc @@ -429,4 +429,46 @@ function acronym_content_width() { $GLOBALS['content_width'] = apply_filters( 'acronym_content_width', 640 ); } +/* + * Issue #1774: detect variables being set via the list() construct. + */ +// Empty list, not allowed since PHP 7.0, but not our concern. +list() = $array; // OK. +list(, ,) = $array; // OK. + +// Ordinary list. +list( $var1, , $var2 ) = $array; // Bad x 2. +list( $acronym_var1, $acronym_var2 ) = $array; // OK. + +// Short list. +[ $var1, $var2 ] = $array; // Bad x 2. +[ $acronym_var1, $acronym_var2 ] = $array; // OK. + +// Keyed list. Keys are not assignments. +list((string)$a => $store["B"], (string)$c => $store["D"]) = $e->getIndexable(); // Bad x 2. +[$foo => $GLOBALS['bar']] = $bar; // Bad x 1. + +// Nested list. +list( $var1, , list( $var2, $var3, ), $var4 ) = $array; // Bad x 4. + +// List with array assignments. +list( $foo['key'], $foo[ $bar ] ) = $array; // Bad x 2. Variable array key should be ignored. + +function acronym_lists_in_function_scope() { + global $store, $c; + + list( $var1, , $var2 ) = $array; // OK. + [ $var1, $var2 ] = $array; // OK. + + // Keyed list. Keys are not assignments. + list((string)$a => $store["B"], (string)$c => $store["D"]) = $e->getIndexable(); // Bad x 2. + [$foo => $GLOBALS['bar']] = $bar; // Bad x 1. + + // Nested list. + list( $var1, , list( $c, $var3, ), $var4 ) = $array; // Bad x 1 - $c. + + // List with array assignments. + list( $foo['key'], $foo[ $c ] ) = $array; // OK. Variable array key should be ignored. +} + // phpcs:set WordPress.NamingConventions.PrefixAllGlobals prefixes[] diff --git a/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.php b/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.php index aa169b940d..1e1cbdb72b 100644 --- a/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.php +++ b/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.php @@ -70,6 +70,15 @@ public function getErrorList( $testFile = 'PrefixAllGlobalsUnitTest.1.inc' ) { 403 => 1, 415 => 1, 423 => 1, + 440 => 2, + 444 => 2, + 448 => 2, + 449 => 1, + 452 => 4, + 455 => 2, + 464 => 2, + 465 => 1, + 468 => 1, ); case 'PrefixAllGlobalsUnitTest.4.inc':