diff --git a/includes/Checker/Checks/Plugin_Repo/Prefixing_Check.php b/includes/Checker/Checks/Plugin_Repo/Prefixing_Check.php index b27179ed0..0cc51957e 100644 --- a/includes/Checker/Checks/Plugin_Repo/Prefixing_Check.php +++ b/includes/Checker/Checks/Plugin_Repo/Prefixing_Check.php @@ -25,6 +25,14 @@ class Prefixing_Check extends Abstract_PHP_CodeSniffer_Check { use Prefix_Utils; use Stable_Check; + /** + * Cache of file contents for line-level checks. + * + * @since n.e.x.t + * @var array> + */ + private $file_line_cache = array(); + /** * Gets the categories for the check. * @@ -106,10 +114,150 @@ public function get_documentation_url(): string { * @param int $severity Severity level. Default is 5. */ protected function add_result_message_for_file( Check_Result $result, $error, $message, $code, $file, $line = 0, $column = 0, string $docs = '', $severity = 5 ) { + // Suppress false positives where a variable sits inside a `foreach` `as` clause + // or a `for` loop initializer at the top level of a file. PHPCS' WPCS sniff + // treats these as global variable definitions because the enclosing function + // early-return does not apply to file top-level scope, but they are loop-local. + if ( 'WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound' === $code + && $this->is_variable_in_loop_control( $file, $line ) + ) { + return; + } + // Update error type and severity. $error = false; $severity = 6; parent::add_result_message_for_file( $result, $error, $message, $code, $file, $line, $column, $docs, $severity ); } + + /** + * Determines whether the variable on the given file/line sits inside a + * `foreach` `as` clause or a `for` loop initializer. + * + * Used to filter out false-positive prefix warnings for loop-local variables + * at the top level of a plugin file, where the WPCS sniff's normal + * function-scope early-return does not apply. + * + * @since n.e.x.t + * + * @param string $file Absolute path to the file. + * @param int $line 1-based line number to inspect. + * @return bool True when the variable lives inside a loop control structure. + */ + private function is_variable_in_loop_control( $file, $line ) { + if ( $line <= 0 || ! is_string( $file ) || '' === $file || ! file_exists( $file ) ) { + return false; + } + + if ( ! isset( $this->file_line_cache[ $file ] ) ) { + $contents = file_get_contents( $file ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents + if ( false === $contents ) { + return false; + } + $this->file_line_cache[ $file ] = preg_split( '/\r\n|\r|\n/', $contents ); + } + + $lines = $this->file_line_cache[ $file ]; + if ( ! isset( $lines[ $line - 1 ] ) ) { + return false; + } + + $source = $lines[ $line - 1 ]; + + if ( $this->is_in_single_line_loop_header( $source ) ) { + return true; + } + + return $this->is_in_multiline_loop_header( $lines, $line ); + } + + /** + * Checks whether a single source line contains a `foreach ... as $var` or + * `for ( ... $var = ...` loop header. + * + * @since n.e.x.t + * + * @param string $source Source line content. + * @return bool True when the line is a single-line loop header. + */ + private function is_in_single_line_loop_header( $source ) { + // foreach ( $items as $key => $value ). + if ( preg_match( '/\bforeach\s*\([^)]*\bas\s+\$/', $source ) ) { + return true; + } + + // for ( $i = 0, $j = 10; ... ) — match any `$var =` inside parens. + if ( preg_match( '/\bfor\s*\([^)]*?\$\w+\s*=/', $source ) ) { + return true; + } + + return false; + } + + /** + * Checks whether the reported line is part of a multi-line `foreach` or + * `for` header whose opener sits on a previous line. + * + * Looks back up to 10 lines for `foreach (` or `for (` with an unclosed + * opening parenthesis, then walks forward to the reported line checking + * for the `as` keyword (foreach) or `$var =` initializer (for). + * + * @since n.e.x.t + * + * @param array $lines File contents split by line. + * @param int $line 1-based line number being inspected. + * @return bool True when the line is inside a multi-line loop header. + */ + private function is_in_multiline_loop_header( $lines, $line ) { + $min_back = max( 0, $line - 11 ); + + for ( $i = $line - 2; $i >= $min_back; $i-- ) { + if ( ! isset( $lines[ $i ] ) ) { + continue; + } + if ( ! preg_match( '/\b(foreach|for)\s*\(\s*$/', $lines[ $i ] ) ) { + continue; + } + if ( $this->loop_header_contains_variable( $lines, $i + 1, $line ) ) { + return true; + } + } + + return false; + } + + /** + * Walks forward from the loop opener to the reported line and checks + * for an `as` keyword (foreach) or `$var =` initializer (for) before + * any statement-terminating `;` or paren-closing `)`. + * + * @since n.e.x.t + * + * @param array $lines File contents split by line. + * @param int $start 1-based line index just after the opener. + * @param int $end 1-based line index of the reported line. + * @return bool True when a loop header variable is found. + */ + private function loop_header_contains_variable( $lines, $start, $end ) { + for ( $j = $start; $j <= $end; $j++ ) { + if ( ! isset( $lines[ $j - 1 ] ) ) { + continue; + } + $scan = $lines[ $j - 1 ]; + if ( preg_match( '/\bas\b/', $scan ) ) { + return true; + } + if ( preg_match( '/\$\w+\s*=/', $scan ) ) { + return true; + } + if ( strpos( $scan, ';' ) !== false ) { + return false; + } + if ( strpos( $scan, ')' ) !== false ) { + return false; + } + } + return false; + } } diff --git a/tests/phpunit/testdata/plugins/test-plugin-prefixing-foreach/load.php b/tests/phpunit/testdata/plugins/test-plugin-prefixing-foreach/load.php new file mode 100644 index 000000000..add467afe --- /dev/null +++ b/tests/phpunit/testdata/plugins/test-plugin-prefixing-foreach/load.php @@ -0,0 +1,78 @@ + $value ) { + echo $value; +} + +// foreach without key: $value is loop-local, must not be flagged. +foreach ( $items as $value ) { + echo $value; +} + +// for: $i is loop-local, must not be flagged. +for ( $i = 0; $i < 10; $i++ ) { + echo $i; +} + +// Multi-line foreach: opener on previous line, $key/$value on the +// `as` line. Both loop-local, must not be flagged. +foreach ( + $items + as $key => $value +) { + echo $value; +} + +// Multi-line for: opener on previous line, $i on the init line. +// Loop-local, must not be flagged. +for ( + $i = 0; + $i < 10; + $i++ +) { + echo $i; +} + +// Multi-line foreach with split `as` and `$var` on different lines. +// Opener on line 1, `as` on line 3, `$key` on line 4. $key is still +// a loop header variable, must not be flagged. +foreach ( + $items + as + $key => $value +) { + echo $value; +} + +// Multi-line for with multi-init on different lines. $i and $j both +// declared in init; both loop-local, must not be flagged. +for ( + $i = 0, + $j = 10; + $i < $j; + $i++, $j-- +) { + echo $i; +} + +function test_pp_foreach_helper() { + return true; +} diff --git a/tests/phpunit/tests/Checker/Checks/Prefixing_Check_Tests.php b/tests/phpunit/tests/Checker/Checks/Prefixing_Check_Tests.php index 09c9e7f79..5cc88e895 100644 --- a/tests/phpunit/tests/Checker/Checks/Prefixing_Check_Tests.php +++ b/tests/phpunit/tests/Checker/Checks/Prefixing_Check_Tests.php @@ -28,4 +28,34 @@ public function test_run_with_errors() { $this->assertCount( 1, wp_list_filter( $warnings['load.php'][28][1], array( 'code' => 'WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound' ) ) ); $this->assertCount( 1, wp_list_filter( $warnings['load.php'][41][1], array( 'code' => 'WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound' ) ) ); } + + public function test_loop_variables_not_flagged() { + $check = new Prefixing_Check(); + $check_context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-prefixing-foreach/load.php' ); + $check_result = new Check_Result( $check_context ); + + $check->run( $check_result ); + + $warnings = $check_result->get_warnings(); + + // Single-line foreach/for header lines: variables declared in + // `as $key => $value` or `for ( $i = 0; ...` are loop-local and + // must not be reported. + $this->assertArrayNotHasKey( 21, $warnings['load.php'] ?? array() ); + $this->assertArrayNotHasKey( 26, $warnings['load.php'] ?? array() ); + $this->assertArrayNotHasKey( 31, $warnings['load.php'] ?? array() ); + + // Multi-line foreach/for with `as`/`$var` on the same line. + $this->assertArrayNotHasKey( 39, $warnings['load.php'] ?? array() ); + $this->assertArrayNotHasKey( 47, $warnings['load.php'] ?? array() ); + + // Multi-line foreach with `as` and `$var` on separate lines. + // $key on line 60 is still a loop header variable. + $this->assertArrayNotHasKey( 60, $warnings['load.php'] ?? array() ); + + // Multi-line for with multi-init on separate lines. + // $i (line 68) and $j (line 69) are both init expressions. + $this->assertArrayNotHasKey( 68, $warnings['load.php'] ?? array() ); + $this->assertArrayNotHasKey( 69, $warnings['load.php'] ?? array() ); + } }