From a0ba22305b70c862692d114ef54f570aa4d0fcbb Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Sun, 24 May 2026 19:37:31 +0600 Subject: [PATCH 1/5] feat(1314): add unclosed ob_start check --- .../Performance/Unclosed_Ob_Start_Check.php | 290 ++++++++++++++++++ includes/Checker/Default_Check_Repository.php | 1 + .../test-plugin-unclosed-ob-start/load.php | 36 +++ .../Checks/Unclosed_Ob_Start_Check_Tests.php | 38 +++ 4 files changed, 365 insertions(+) create mode 100644 includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php create mode 100644 tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php create mode 100644 tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php diff --git a/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php b/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php new file mode 100644 index 000000000..50103f19f --- /dev/null +++ b/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php @@ -0,0 +1,290 @@ +check_file_for_unclosed_ob_start( $result, $file ); + } + } + + /** + * Checks a single PHP file for unclosed ob_start calls. + * + * @since 1.4.0 + * + * @param Check_Result $result The check result. + * @param string $file Absolute path to the file. + */ + private function check_file_for_unclosed_ob_start( Check_Result $result, string $file ) { + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents + $source = file_get_contents( $file ); + if ( false === $source || '' === $source ) { + return; + } + + $tokens = token_get_all( $source ); + + $brace_depth = 0; + $awaiting_function_brace = false; + + $scope_stack = array( + array( + 'type' => 'file', + 'start_depth' => 0, + 'ob_starts' => array(), + 'ob_closes' => array(), + ), + ); + + foreach ( $tokens as $index => $token ) { + if ( is_array( $token ) ) { + $token_type = $token[0]; + + if ( T_FUNCTION === $token_type ) { + $awaiting_function_brace = true; + } elseif ( T_STRING === $token_type ) { + $next_index = $this->get_next_significant_token_index( $tokens, $index ); + if ( null !== $next_index && '(' === $tokens[ $next_index ] ) { + if ( $this->is_global_function_call( $tokens, $index ) ) { + $name = strtolower( $token[1] ); + $line = (int) $token[2]; + + if ( 'ob_start' === $name ) { + $scope_stack[ count( $scope_stack ) - 1 ]['ob_starts'][] = array( + 'line' => $line, + 'depth' => $brace_depth, + ); + } elseif ( in_array( $name, array( 'ob_get_clean', 'ob_end_clean', 'ob_get_flush', 'ob_end_flush' ), true ) ) { + $scope_stack[ count( $scope_stack ) - 1 ]['ob_closes'][] = array( + 'line' => $line, + 'depth' => $brace_depth, + ); + } + } + } + } + continue; + } + + if ( '{' === $token ) { + if ( $awaiting_function_brace ) { + $scope_stack[] = array( + 'type' => 'function', + 'start_depth' => $brace_depth, + 'ob_starts' => array(), + 'ob_closes' => array(), + ); + $awaiting_function_brace = false; + } + $brace_depth++; + } elseif ( '}' === $token ) { + $brace_depth--; + $current_scope = end( $scope_stack ); + if ( 'function' === $current_scope['type'] && $current_scope['start_depth'] === $brace_depth ) { + $this->process_scope( $result, $file, $current_scope ); + array_pop( $scope_stack ); + } + } + } + + // Process any remaining scopes in case of unbalanced braces or top-level file scope. + while ( count( $scope_stack ) > 0 ) { + $current_scope = array_pop( $scope_stack ); + $this->process_scope( $result, $file, $current_scope ); + } + } + + /** + * Processes a completed scope to find unpaired ob_start calls. + * + * @since 1.4.0 + * + * @param Check_Result $result The check result. + * @param string $file Absolute path to the file. + * @param array $scope The scope data. + */ + private function process_scope( Check_Result $result, string $file, array $scope ) { + if ( empty( $scope['ob_starts'] ) ) { + return; + } + + $unpaired_ob_starts = $scope['ob_starts']; + + foreach ( $scope['ob_closes'] as $close ) { + // Find the most recent ob_start that is on or before this close's depth. + for ( $i = count( $unpaired_ob_starts ) - 1; $i >= 0; $i-- ) { + if ( $close['depth'] <= $unpaired_ob_starts[ $i ]['depth'] ) { + array_splice( $unpaired_ob_starts, $i, 1 ); + break; // Found the match for this close, move to the next close. + } + } + } + + foreach ( $unpaired_ob_starts as $ob_start ) { + $this->add_result_warning_for_file( + $result, + __( 'ob_start() was found without a corresponding closing call (ob_get_clean(), ob_end_clean(), ob_get_flush() or ob_end_flush()) in the same scope. Output buffering is a valid technique, but a buffer must not be left open. WordPress is a shared environment where core, themes and other plugins may also open or close buffers, and a misaligned buffer stack causes unpredictable behaviour (headers already sent, lost output, broken redirects, etc.). Please ensure every ob_start() is paired with a closing function within the same function scope, and that nothing (including hooks or early returns) can bypass that closing logic. If you need to modify the full response output, use the new template enhancement output buffer available since WordPress 6.9.', 'plugin-check' ), + 'unclosed_ob_start', + $file, + $ob_start['line'], + 0, + 'https://make.wordpress.org/core/2021/02/10/introducing-template-road-map-and-enhancements-in-wordpress-5-7/' + ); + } + } + + /** + * Checks whether a tokenized T_STRING is a global function call. + * + * @since 1.4.0 + * + * @param array $tokens Token stream. + * @param int $index Current token index. + * @return bool + */ + private function is_global_function_call( array $tokens, int $index ): bool { + $previous_index = $this->get_previous_significant_token_index( $tokens, $index ); + if ( null === $previous_index ) { + return true; + } + + $previous_token = $tokens[ $previous_index ]; + + if ( is_array( $previous_token ) ) { + if ( in_array( $previous_token[0], array( T_FUNCTION, T_NEW, T_OBJECT_OPERATOR, T_DOUBLE_COLON ), true ) ) { + return false; + } + + if ( T_NS_SEPARATOR === $previous_token[0] ) { + $before_namespace_index = $this->get_previous_significant_token_index( $tokens, $previous_index ); + if ( null === $before_namespace_index ) { + return true; + } + + $before_namespace_token = $tokens[ $before_namespace_index ]; + if ( is_array( $before_namespace_token ) && in_array( $before_namespace_token[0], array( T_STRING, T_NAMESPACE ), true ) ) { + return false; + } + } + } + + return true; + } + + /** + * Finds the next significant token index. + * + * @since 1.4.0 + * + * @param array $tokens Token stream. + * @param int $index Current token index. + * @return int|null + */ + private function get_next_significant_token_index( array $tokens, int $index ): ?int { + $count = count( $tokens ); + for ( $i = $index + 1; $i < $count; $i++ ) { + $token = $tokens[ $i ]; + if ( is_array( $token ) && T_WHITESPACE === $token[0] ) { + continue; + } + + return $i; + } + + return null; + } + + /** + * Finds the previous significant token index. + * + * @since 1.4.0 + * + * @param array $tokens Token stream. + * @param int $index Current token index. + * @return int|null + */ + private function get_previous_significant_token_index( array $tokens, int $index ): ?int { + for ( $i = $index - 1; $i >= 0; $i-- ) { + $token = $tokens[ $i ]; + + if ( is_array( $token ) && in_array( $token[0], array( T_WHITESPACE, T_COMMENT, T_DOC_COMMENT ), true ) ) { + continue; + } + + return $i; + } + + return null; + } + + /** + * Gets the description for the check. + * + * Every check must have a short description explaining what the check does. + * + * @since 1.4.0 + * + * @return string Description. + */ + public function get_description(): string { + return __( 'Detects calls to ob_start() that are not paired with a corresponding buffer-closing function within the same logical scope.', 'plugin-check' ); + } + + /** + * Gets the documentation URL for the check. + * + * Every check must have a URL with further information about the check. + * + * @since 1.4.0 + * + * @return string The documentation URL. + */ + public function get_documentation_url(): string { + return __( 'https://developer.wordpress.org/plugins/', 'plugin-check' ); + } +} diff --git a/includes/Checker/Default_Check_Repository.php b/includes/Checker/Default_Check_Repository.php index c1b7d420c..c12f579e3 100644 --- a/includes/Checker/Default_Check_Repository.php +++ b/includes/Checker/Default_Check_Repository.php @@ -103,6 +103,7 @@ private function register_default_checks() { 'direct_file_access' => new Checks\Plugin_Repo\Direct_File_Access_Check(), 'external_admin_menu_links' => new Checks\Plugin_Repo\External_Admin_Menu_Links_Check(), 'wp_functions_compatibility' => new Checks\Plugin_Repo\WP_Functions_Compatibility_Check(), + 'unclosed_ob_start' => new Checks\Performance\Unclosed_Ob_Start_Check(), ) ); diff --git a/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php b/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php new file mode 100644 index 000000000..b4eaf2846 --- /dev/null +++ b/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php @@ -0,0 +1,36 @@ +run( $check_result ); + + $warnings = $check_result->get_warnings(); + + $this->assertArrayHasKey( 'load.php', $warnings ); + $this->assertCount( 3, $warnings['load.php'] ); + + // 14: ob_start() at the top of the file with no closing call. + $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][0][0][0]['code'] ); + $this->assertEquals( 14, $warnings['load.php'][0][0][0]['line'] ); + + // 18: Multiple ob_start() in the same scope, only one closed. + $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][1][0][0]['code'] ); + $this->assertEquals( 18, $warnings['load.php'][1][0][0]['line'] ); + + // 33: ob_start() inside a function, closed only conditionally (if). + $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][2][0][0]['code'] ); + $this->assertEquals( 33, $warnings['load.php'][2][0][0]['line'] ); + } +} From 9da5b91856220b2dff433c74ee9960a6f26a9af8 Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Sun, 24 May 2026 20:17:43 +0600 Subject: [PATCH 2/5] fix(1314): resolve PHPCS linting and PHPUnit test failures --- .../Performance/Unclosed_Ob_Start_Check.php | 4 ++-- .../test-plugin-unclosed-ob-start/load.php | 2 +- .../Checks/Unclosed_Ob_Start_Check_Tests.php | 21 +++++++++++-------- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php b/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php index 50103f19f..b8ce25d61 100644 --- a/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php +++ b/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php @@ -121,9 +121,9 @@ private function check_file_for_unclosed_ob_start( Check_Result $result, string ); $awaiting_function_brace = false; } - $brace_depth++; + ++$brace_depth; } elseif ( '}' === $token ) { - $brace_depth--; + --$brace_depth; $current_scope = end( $scope_stack ); if ( 'function' === $current_scope['type'] && $current_scope['start_depth'] === $brace_depth ) { $this->process_scope( $result, $file, $current_scope ); diff --git a/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php b/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php index b4eaf2846..976af6f0d 100644 --- a/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php +++ b/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php @@ -27,7 +27,7 @@ function multiple_ob_start() { ob_get_clean(); }; -// 5. ob_start() inside a function, closed only conditionally (if) → flagged as warning (line 33). +// 5. ob_start() inside a function, closed only conditionally (if) → flagged as warning (line 32). function conditional_close() { ob_start(); if ( true ) { diff --git a/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php b/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php index 0c594e034..9100d0e78 100644 --- a/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php +++ b/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php @@ -24,15 +24,18 @@ public function test_run() { $this->assertCount( 3, $warnings['load.php'] ); // 14: ob_start() at the top of the file with no closing call. - $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][0][0][0]['code'] ); - $this->assertEquals( 14, $warnings['load.php'][0][0][0]['line'] ); - + $this->assertArrayHasKey( 14, $warnings['load.php'] ); + $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][14][0][0]['code'] ); + $this->assertEquals( 14, $warnings['load.php'][14][0][0]['line'] ); + // 18: Multiple ob_start() in the same scope, only one closed. - $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][1][0][0]['code'] ); - $this->assertEquals( 18, $warnings['load.php'][1][0][0]['line'] ); - - // 33: ob_start() inside a function, closed only conditionally (if). - $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][2][0][0]['code'] ); - $this->assertEquals( 33, $warnings['load.php'][2][0][0]['line'] ); + $this->assertArrayHasKey( 18, $warnings['load.php'] ); + $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][18][0][0]['code'] ); + $this->assertEquals( 18, $warnings['load.php'][18][0][0]['line'] ); + + // 32: ob_start() inside a function, closed only conditionally (if). + $this->assertArrayHasKey( 32, $warnings['load.php'] ); + $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][32][0][0]['code'] ); + $this->assertEquals( 32, $warnings['load.php'][32][0][0]['line'] ); } } From 32eccbe7cec6d1f5ec98692684e8e438d4715aeb Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Sun, 24 May 2026 20:21:34 +0600 Subject: [PATCH 3/5] fix(1314): resolve NPath complexity in checker and correct phpunit assertions --- .../Performance/Unclosed_Ob_Start_Check.php | 75 ++++++++++++------- .../Checks/Unclosed_Ob_Start_Check_Tests.php | 3 - 2 files changed, 50 insertions(+), 28 deletions(-) diff --git a/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php b/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php index b8ce25d61..17dc2ee0b 100644 --- a/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php +++ b/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php @@ -83,31 +83,7 @@ private function check_file_for_unclosed_ob_start( Check_Result $result, string foreach ( $tokens as $index => $token ) { if ( is_array( $token ) ) { - $token_type = $token[0]; - - if ( T_FUNCTION === $token_type ) { - $awaiting_function_brace = true; - } elseif ( T_STRING === $token_type ) { - $next_index = $this->get_next_significant_token_index( $tokens, $index ); - if ( null !== $next_index && '(' === $tokens[ $next_index ] ) { - if ( $this->is_global_function_call( $tokens, $index ) ) { - $name = strtolower( $token[1] ); - $line = (int) $token[2]; - - if ( 'ob_start' === $name ) { - $scope_stack[ count( $scope_stack ) - 1 ]['ob_starts'][] = array( - 'line' => $line, - 'depth' => $brace_depth, - ); - } elseif ( in_array( $name, array( 'ob_get_clean', 'ob_end_clean', 'ob_get_flush', 'ob_end_flush' ), true ) ) { - $scope_stack[ count( $scope_stack ) - 1 ]['ob_closes'][] = array( - 'line' => $line, - 'depth' => $brace_depth, - ); - } - } - } - } + $this->process_array_token( $token, $index, $tokens, $brace_depth, $scope_stack, $awaiting_function_brace ); continue; } @@ -139,6 +115,55 @@ private function check_file_for_unclosed_ob_start( Check_Result $result, string } } + /** + * Processes an array token to update function detection state and track ob_start/close calls. + * + * @since 1.4.0 + * + * @param array $token The array token. + * @param int $index Token index. + * @param array $tokens All tokens. + * @param int $brace_depth Current brace depth. + * @param array $scope_stack The scope stack. + * @param bool $awaiting_function_brace Whether awaiting a function's opening brace. + */ + private function process_array_token( array $token, int $index, array &$tokens, int $brace_depth, array &$scope_stack, bool &$awaiting_function_brace ) { + $token_type = $token[0]; + + if ( T_FUNCTION === $token_type ) { + $awaiting_function_brace = true; + return; + } + + if ( T_STRING !== $token_type ) { + return; + } + + $next_index = $this->get_next_significant_token_index( $tokens, $index ); + if ( null === $next_index || '(' !== $tokens[ $next_index ] ) { + return; + } + + if ( ! $this->is_global_function_call( $tokens, $index ) ) { + return; + } + + $name = strtolower( $token[1] ); + $line = (int) $token[2]; + + if ( 'ob_start' === $name ) { + $scope_stack[ count( $scope_stack ) - 1 ]['ob_starts'][] = array( + 'line' => $line, + 'depth' => $brace_depth, + ); + } elseif ( in_array( $name, array( 'ob_get_clean', 'ob_end_clean', 'ob_get_flush', 'ob_end_flush' ), true ) ) { + $scope_stack[ count( $scope_stack ) - 1 ]['ob_closes'][] = array( + 'line' => $line, + 'depth' => $brace_depth, + ); + } + } + /** * Processes a completed scope to find unpaired ob_start calls. * diff --git a/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php b/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php index 9100d0e78..3b69447e6 100644 --- a/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php +++ b/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php @@ -26,16 +26,13 @@ public function test_run() { // 14: ob_start() at the top of the file with no closing call. $this->assertArrayHasKey( 14, $warnings['load.php'] ); $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][14][0][0]['code'] ); - $this->assertEquals( 14, $warnings['load.php'][14][0][0]['line'] ); // 18: Multiple ob_start() in the same scope, only one closed. $this->assertArrayHasKey( 18, $warnings['load.php'] ); $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][18][0][0]['code'] ); - $this->assertEquals( 18, $warnings['load.php'][18][0][0]['line'] ); // 32: ob_start() inside a function, closed only conditionally (if). $this->assertArrayHasKey( 32, $warnings['load.php'] ); $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][32][0][0]['code'] ); - $this->assertEquals( 32, $warnings['load.php'][32][0][0]['line'] ); } } From ff7ff7227e2f7fee61c8447e99c50d71d6db2af8 Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Fri, 19 Jun 2026 03:05:21 +0600 Subject: [PATCH 4/5] Convert unclosed ob_start check to PHPCS sniff - Move logic into PluginCheck.CodeAnalysis.UnclosedObStart sniff - Check wrapper now extends Abstract_PHP_CodeSniffer_Check - Handle fully-qualified \ob_start() calls (T_NAME_FULLY_QUALIFIED + T_NS_SEPARATOR) - Skip warning when ob_start() is followed by add_action/add_filter (hook paired buffer) - Point docs URL to WordPress 6.9 field guide - Add sniff unit test and FQN/hook-paired test fixtures Addresses review feedback from davidperezgar and westonruter on #1314. --- .../Performance/Unclosed_Ob_Start_Check.php | 281 ++---------- .../CodeAnalysis/UnclosedObStartSniff.php | 402 ++++++++++++++++++ .../CodeAnalysis/UnclosedObStartUnitTest.inc | 42 ++ .../CodeAnalysis/UnclosedObStartUnitTest.php | 59 +++ phpcs-sniffs/PluginCheck/ruleset.xml | 1 + .../test-plugin-unclosed-ob-start/load.php | 17 + .../Checks/Unclosed_Ob_Start_Check_Tests.php | 13 +- 7 files changed, 561 insertions(+), 254 deletions(-) create mode 100644 phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/UnclosedObStartSniff.php create mode 100644 phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/UnclosedObStartUnitTest.inc create mode 100644 phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/UnclosedObStartUnitTest.php diff --git a/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php b/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php index 17dc2ee0b..e8abe5f02 100644 --- a/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php +++ b/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php @@ -9,8 +9,7 @@ use WordPress\Plugin_Check\Checker\Check_Categories; use WordPress\Plugin_Check\Checker\Check_Result; -use WordPress\Plugin_Check\Checker\Checks\Abstract_File_Check; -use WordPress\Plugin_Check\Traits\Amend_Check_Result; +use WordPress\Plugin_Check\Checker\Checks\Abstract_PHP_CodeSniffer_Check; use WordPress\Plugin_Check\Traits\Stable_Check; /** @@ -18,9 +17,8 @@ * * @since 1.4.0 */ -class Unclosed_Ob_Start_Check extends Abstract_File_Check { +class Unclosed_Ob_Start_Check extends Abstract_PHP_CodeSniffer_Check { - use Amend_Check_Result; use Stable_Check; /** @@ -37,254 +35,19 @@ public function get_categories() { } /** - * Amends the given result by running the check on the given list of files. + * Returns an associative array of arguments to pass to PHPCS. * * @since 1.4.0 * - * @param Check_Result $result The Check Result to amend. - * @param array $files Array of plugin files. + * @param Check_Result $result The check result to amend, including the plugin context to check. + * @return array An associative array of PHPCS CLI arguments. */ - protected function check_files( Check_Result $result, array $files ) { - $php_files = self::filter_files_by_extension( $files, 'php' ); - - foreach ( $php_files as $file ) { - $this->check_file_for_unclosed_ob_start( $result, $file ); - } - } - - /** - * Checks a single PHP file for unclosed ob_start calls. - * - * @since 1.4.0 - * - * @param Check_Result $result The check result. - * @param string $file Absolute path to the file. - */ - private function check_file_for_unclosed_ob_start( Check_Result $result, string $file ) { - // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents - $source = file_get_contents( $file ); - if ( false === $source || '' === $source ) { - return; - } - - $tokens = token_get_all( $source ); - - $brace_depth = 0; - $awaiting_function_brace = false; - - $scope_stack = array( - array( - 'type' => 'file', - 'start_depth' => 0, - 'ob_starts' => array(), - 'ob_closes' => array(), - ), + protected function get_args( Check_Result $result ) { + return array( + 'extensions' => 'php', + 'standard' => 'PluginCheck', + 'sniffs' => 'PluginCheck.CodeAnalysis.UnclosedObStart', ); - - foreach ( $tokens as $index => $token ) { - if ( is_array( $token ) ) { - $this->process_array_token( $token, $index, $tokens, $brace_depth, $scope_stack, $awaiting_function_brace ); - continue; - } - - if ( '{' === $token ) { - if ( $awaiting_function_brace ) { - $scope_stack[] = array( - 'type' => 'function', - 'start_depth' => $brace_depth, - 'ob_starts' => array(), - 'ob_closes' => array(), - ); - $awaiting_function_brace = false; - } - ++$brace_depth; - } elseif ( '}' === $token ) { - --$brace_depth; - $current_scope = end( $scope_stack ); - if ( 'function' === $current_scope['type'] && $current_scope['start_depth'] === $brace_depth ) { - $this->process_scope( $result, $file, $current_scope ); - array_pop( $scope_stack ); - } - } - } - - // Process any remaining scopes in case of unbalanced braces or top-level file scope. - while ( count( $scope_stack ) > 0 ) { - $current_scope = array_pop( $scope_stack ); - $this->process_scope( $result, $file, $current_scope ); - } - } - - /** - * Processes an array token to update function detection state and track ob_start/close calls. - * - * @since 1.4.0 - * - * @param array $token The array token. - * @param int $index Token index. - * @param array $tokens All tokens. - * @param int $brace_depth Current brace depth. - * @param array $scope_stack The scope stack. - * @param bool $awaiting_function_brace Whether awaiting a function's opening brace. - */ - private function process_array_token( array $token, int $index, array &$tokens, int $brace_depth, array &$scope_stack, bool &$awaiting_function_brace ) { - $token_type = $token[0]; - - if ( T_FUNCTION === $token_type ) { - $awaiting_function_brace = true; - return; - } - - if ( T_STRING !== $token_type ) { - return; - } - - $next_index = $this->get_next_significant_token_index( $tokens, $index ); - if ( null === $next_index || '(' !== $tokens[ $next_index ] ) { - return; - } - - if ( ! $this->is_global_function_call( $tokens, $index ) ) { - return; - } - - $name = strtolower( $token[1] ); - $line = (int) $token[2]; - - if ( 'ob_start' === $name ) { - $scope_stack[ count( $scope_stack ) - 1 ]['ob_starts'][] = array( - 'line' => $line, - 'depth' => $brace_depth, - ); - } elseif ( in_array( $name, array( 'ob_get_clean', 'ob_end_clean', 'ob_get_flush', 'ob_end_flush' ), true ) ) { - $scope_stack[ count( $scope_stack ) - 1 ]['ob_closes'][] = array( - 'line' => $line, - 'depth' => $brace_depth, - ); - } - } - - /** - * Processes a completed scope to find unpaired ob_start calls. - * - * @since 1.4.0 - * - * @param Check_Result $result The check result. - * @param string $file Absolute path to the file. - * @param array $scope The scope data. - */ - private function process_scope( Check_Result $result, string $file, array $scope ) { - if ( empty( $scope['ob_starts'] ) ) { - return; - } - - $unpaired_ob_starts = $scope['ob_starts']; - - foreach ( $scope['ob_closes'] as $close ) { - // Find the most recent ob_start that is on or before this close's depth. - for ( $i = count( $unpaired_ob_starts ) - 1; $i >= 0; $i-- ) { - if ( $close['depth'] <= $unpaired_ob_starts[ $i ]['depth'] ) { - array_splice( $unpaired_ob_starts, $i, 1 ); - break; // Found the match for this close, move to the next close. - } - } - } - - foreach ( $unpaired_ob_starts as $ob_start ) { - $this->add_result_warning_for_file( - $result, - __( 'ob_start() was found without a corresponding closing call (ob_get_clean(), ob_end_clean(), ob_get_flush() or ob_end_flush()) in the same scope. Output buffering is a valid technique, but a buffer must not be left open. WordPress is a shared environment where core, themes and other plugins may also open or close buffers, and a misaligned buffer stack causes unpredictable behaviour (headers already sent, lost output, broken redirects, etc.). Please ensure every ob_start() is paired with a closing function within the same function scope, and that nothing (including hooks or early returns) can bypass that closing logic. If you need to modify the full response output, use the new template enhancement output buffer available since WordPress 6.9.', 'plugin-check' ), - 'unclosed_ob_start', - $file, - $ob_start['line'], - 0, - 'https://make.wordpress.org/core/2021/02/10/introducing-template-road-map-and-enhancements-in-wordpress-5-7/' - ); - } - } - - /** - * Checks whether a tokenized T_STRING is a global function call. - * - * @since 1.4.0 - * - * @param array $tokens Token stream. - * @param int $index Current token index. - * @return bool - */ - private function is_global_function_call( array $tokens, int $index ): bool { - $previous_index = $this->get_previous_significant_token_index( $tokens, $index ); - if ( null === $previous_index ) { - return true; - } - - $previous_token = $tokens[ $previous_index ]; - - if ( is_array( $previous_token ) ) { - if ( in_array( $previous_token[0], array( T_FUNCTION, T_NEW, T_OBJECT_OPERATOR, T_DOUBLE_COLON ), true ) ) { - return false; - } - - if ( T_NS_SEPARATOR === $previous_token[0] ) { - $before_namespace_index = $this->get_previous_significant_token_index( $tokens, $previous_index ); - if ( null === $before_namespace_index ) { - return true; - } - - $before_namespace_token = $tokens[ $before_namespace_index ]; - if ( is_array( $before_namespace_token ) && in_array( $before_namespace_token[0], array( T_STRING, T_NAMESPACE ), true ) ) { - return false; - } - } - } - - return true; - } - - /** - * Finds the next significant token index. - * - * @since 1.4.0 - * - * @param array $tokens Token stream. - * @param int $index Current token index. - * @return int|null - */ - private function get_next_significant_token_index( array $tokens, int $index ): ?int { - $count = count( $tokens ); - for ( $i = $index + 1; $i < $count; $i++ ) { - $token = $tokens[ $i ]; - if ( is_array( $token ) && T_WHITESPACE === $token[0] ) { - continue; - } - - return $i; - } - - return null; - } - - /** - * Finds the previous significant token index. - * - * @since 1.4.0 - * - * @param array $tokens Token stream. - * @param int $index Current token index. - * @return int|null - */ - private function get_previous_significant_token_index( array $tokens, int $index ): ?int { - for ( $i = $index - 1; $i >= 0; $i-- ) { - $token = $tokens[ $i ]; - - if ( is_array( $token ) && in_array( $token[0], array( T_WHITESPACE, T_COMMENT, T_DOC_COMMENT ), true ) ) { - continue; - } - - return $i; - } - - return null; } /** @@ -310,6 +73,28 @@ public function get_description(): string { * @return string The documentation URL. */ public function get_documentation_url(): string { - return __( 'https://developer.wordpress.org/plugins/', 'plugin-check' ); + return __( 'https://make.wordpress.org/core/2025/11/18/wordpress-6-9-frontend-performance-field-guide/#introduce-the-template-enhancement-output-buffer', 'plugin-check' ); + } + + /** + * Amends the given result with a message for the specified file, including the + * documentation URL for this check. + * + * @since 1.4.0 + * + * @param Check_Result $result The check result to amend, including the plugin context to check. + * @param bool $error Whether it is an error or notice. + * @param string $message Error message. + * @param string $code Error code. + * @param string $file Absolute path to the file where the issue was found. + * @param int $line The line on which the message occurred. Default is 0 (unknown line). + * @param int $column The column on which the message occurred. Default is 0 (unknown column). + * @param string $docs URL for further information about the message. + * @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 ) { + $docs = __( 'https://make.wordpress.org/core/2025/11/18/wordpress-6-9-frontend-performance-field-guide/#introduce-the-template-enhancement-output-buffer', 'plugin-check' ); + + parent::add_result_message_for_file( $result, $error, $message, $code, $file, $line, $column, $docs, $severity ); } } diff --git a/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/UnclosedObStartSniff.php b/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/UnclosedObStartSniff.php new file mode 100644 index 000000000..c290fc754 --- /dev/null +++ b/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/UnclosedObStartSniff.php @@ -0,0 +1,402 @@ + + */ + private $closing_functions = array( + 'ob_get_clean' => true, + 'ob_end_clean' => true, + 'ob_get_flush' => true, + 'ob_end_flush' => true, + ); + + /** + * Scope-keyed collection of ob_start() and closing calls for the current file. + * + * Populated during the token walk and evaluated once the file has been + * fully processed. + * + * @since 1.4.0 + * + * @var array, closes: array}> + */ + private $scopes = array(); + + /** + * Ordered list of scope start pointers, used to process scopes in source order. + * + * @since 1.4.0 + * + * @var array + */ + private $scope_order = array(); + + /** + * Whether the current file has been finalized (scopes evaluated). + * + * @since 1.4.0 + * + * @var bool + */ + private $finalized = false; + + /** + * Returns an array of tokens this test wants to listen for. + * + * @since 1.4.0 + * + * @return array + */ + public function register() { + $tokens = array( T_STRING ); + + // T_NAME_FULLY_QUALIFIED was introduced in PHP 8.0 for namespaced names like \ob_start(). + if ( defined( 'T_NAME_FULLY_QUALIFIED' ) ) { + $tokens[] = constant( 'T_NAME_FULLY_QUALIFIED' ); + } + + return $tokens; + } + + /** + * Processes this test, when one of its tokens is encountered. + * + * @since 1.4.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. + */ + public function process_token( $stackPtr ) { + $name = $this->get_global_function_name( $stackPtr ); + if ( null === $name ) { + $this->maybe_finalize( $stackPtr ); + return; + } + + $lower_name = strtolower( $name ); + + if ( 'ob_start' === $lower_name ) { + $this->record_call( $stackPtr, true ); + } elseif ( isset( $this->closing_functions[ $lower_name ] ) ) { + $this->record_call( $stackPtr, false ); + } + + $this->maybe_finalize( $stackPtr ); + } + + /** + * Finalizes the file once the last registered token has been processed. + * + * PHPCS only invokes process_token() for tokens returned by register(), so the + * final file token (often trailing whitespace) is never visited. This finalizes as + * soon as no further registered tokens remain ahead, ensuring all collected calls + * are available before pairing. + * + * @since 1.4.0 + * + * @param int $stackPtr The position of the current token in the stack. + */ + private function maybe_finalize( $stackPtr ) { + if ( $this->finalized ) { + return; + } + + $next = $this->phpcsFile->findNext( $this->register(), ( $stackPtr + 1 ) ); + if ( false !== $next ) { + return; + } + + $this->finalize_file(); + } + + /** + * Resolves the global function name being called at the given pointer, or null if + * the token is not a global call to a tracked function. + * + * Handles both the legacy T_STRING + T_NS_SEPARATOR sequence (PHP < 8.0) and the + * single T_NAME_FULLY_QUALIFIED token (PHP >= 8.0) so that \ob_start() in namespaced + * code is detected consistently. + * + * @since 1.4.0 + * + * @param int $stackPtr The position of the current token in the stack. + * @return string|null The lowercased function name, or null when not a global call. + */ + private function get_global_function_name( $stackPtr ) { + $token_code = $this->tokens[ $stackPtr ]['code']; + + $name = null; + + if ( T_STRING === $token_code ) { + $previous = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); + if ( false === $previous ) { + return null; + } + + $previous_code = $this->tokens[ $previous ]['code']; + + $disallowed_previous = array( T_FUNCTION, T_NEW, T_OBJECT_OPERATOR, T_DOUBLE_COLON ); + + // T_NULLSAFE_OBJECT_OPERATOR (?->) was introduced in PHP 8.0. + if ( defined( 'T_NULLSAFE_OBJECT_OPERATOR' ) ) { + $disallowed_previous[] = constant( 'T_NULLSAFE_OBJECT_OPERATOR' ); + } + + // Skip method calls, constructors and function declarations. + if ( in_array( $previous_code, $disallowed_previous, true ) ) { + return null; + } + + // Handle the legacy \Namespace\func() two-token form. + if ( T_NS_SEPARATOR === $previous_code ) { + $before_ns = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $previous - 1 ), null, true ); + if ( false !== $before_ns ) { + $before_code = $this->tokens[ $before_ns ]['code']; + if ( in_array( $before_code, array( T_STRING, T_NAMESPACE ), true ) ) { + return null; // Namespaced call, not a global one. + } + } + } + + $name = $this->tokens[ $stackPtr ]['content']; + } elseif ( defined( 'T_NAME_FULLY_QUALIFIED' ) && constant( 'T_NAME_FULLY_QUALIFIED' ) === $token_code ) { + // PHP >= 8.0: \ob_start() tokenizes as a single T_NAME_FULLY_QUALIFIED token. + $name = ltrim( $this->tokens[ $stackPtr ]['content'], '\\' ); + } + + if ( null === $name ) { + return null; + } + + // Must be followed by an opening parenthesis to be a function call. + $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); + if ( false === $next || T_OPEN_PARENTHESIS !== $this->tokens[ $next ]['code'] ) { + return null; + } + + return $name; + } + + /** + * Records an ob_start() or closing call against its enclosing scope. + * + * The file scope is represented by a pointer of -1. Function, method and closure + * scopes use the pointer of their opening condition token. + * + * @since 1.4.0 + * + * @param int $stackPtr The position of the call. + * @param bool $is_start Whether this is an ob_start() (true) or a closing call (false). + */ + private function record_call( $stackPtr, $is_start ) { + $scope_ptr = $this->get_scope_pointer( $stackPtr ); + + if ( ! isset( $this->scopes[ $scope_ptr ] ) ) { + $this->scopes[ $scope_ptr ] = array( + 'starts' => array(), + 'closes' => array(), + ); + $this->scope_order[] = $scope_ptr; + } + + $line = $this->tokens[ $stackPtr ]['line']; + $conditions = isset( $this->tokens[ $stackPtr ]['conditions'] ) ? $this->tokens[ $stackPtr ]['conditions'] : array(); + + if ( $is_start ) { + $this->scopes[ $scope_ptr ]['starts'][] = array( + 'ptr' => $stackPtr, + 'line' => $line, + 'conditions' => $conditions, + ); + } else { + $this->scopes[ $scope_ptr ]['closes'][] = array( + 'ptr' => $stackPtr, + 'line' => $line, + 'conditions' => $conditions, + ); + } + } + + /** + * Gets the scope pointer that owns the token at the given position. + * + * Returns the pointer of the nearest enclosing function, method or closure, or -1 + * for the file (top-level) scope. + * + * @since 1.4.0 + * + * @param int $stackPtr The position of the token. + * @return int The scope pointer, or -1 for the file scope. + */ + private function get_scope_pointer( $stackPtr ) { + if ( empty( $this->tokens[ $stackPtr ]['conditions'] ) ) { + return -1; + } + + $conditions = array_reverse( $this->tokens[ $stackPtr ]['conditions'], true ); + foreach ( $conditions as $condition_ptr => $condition_code ) { + if ( in_array( $condition_code, array( T_FUNCTION, T_CLOSURE ), true ) ) { + return $condition_ptr; + } + } + + return -1; + } + + /** + * Evaluates collected scopes and reports unpaired ob_start() calls. + * + * Called once when the end of the file is reached so that the full scope contents + * are available for pairing. + * + * @since 1.4.0 + */ + private function finalize_file() { + $this->finalized = true; + + foreach ( $this->scope_order as $scope_ptr ) { + $scope = $this->scopes[ $scope_ptr ]; + if ( empty( $scope['starts'] ) ) { + continue; + } + + $unpaired = $this->pair_starts_and_closes( $scope['starts'], $scope['closes'] ); + + foreach ( $unpaired as $start ) { + if ( $this->is_hook_paired( $start['ptr'] ) ) { + continue; + } + + $this->phpcsFile->addWarning( + 'ob_start() was found without a corresponding closing call (ob_get_clean(), ob_end_clean(), ob_get_flush() or ob_end_flush()) in the same scope. Output buffering is a valid technique, but a buffer must not be left open. WordPress is a shared environment where core, themes and other plugins may also open or close buffers, and a misaligned buffer stack causes unpredictable behaviour (headers already sent, lost output, broken redirects, etc.). Please ensure every ob_start() is paired with a closing function within the same function scope, and that nothing (including hooks or early returns) can bypass that closing logic. If you need to modify the full response output, use the new template enhancement output buffer available since WordPress 6.9.', + $start['ptr'], + 'UnclosedObStart' + ); + } + } + + // Reset state so the sniff can be reused across multiple files. + $this->scopes = array(); + $this->scope_order = array(); + $this->finalized = false; + } + + /** + * Pairs ob_start() calls with closing calls within the same scope. + * + * A closing call pairs with the most recent unpaired ob_start() that appears before + * it in source order and that sits at the same nesting level (identical set of + * enclosing conditions). This ensures a buffer that is only closed conditionally, + * for example inside an if block that may not execute, is treated as unpaired and + * flagged. + * + * ob_start() calls that remain after all closes have been consumed are returned as + * unpaired. + * + * @since 1.4.0 + * + * @param array $starts The ob_start() calls. + * @param array $closes The closing calls. + * @return array The unpaired ob_start() calls. + */ + private function pair_starts_and_closes( array $starts, array $closes ) { + $unpaired = $starts; + + foreach ( $closes as $close ) { + for ( $i = ( count( $unpaired ) - 1 ); $i >= 0; $i-- ) { + $start = $unpaired[ $i ]; + + if ( $close['ptr'] <= $start['ptr'] ) { + continue; + } + + // Only pair when both calls share the exact same nesting context, so a + // close inside a conditional block does not silently pair with a start + // at the enclosing level. + if ( $this->same_nesting( $start['conditions'], $close['conditions'] ) ) { + array_splice( $unpaired, $i, 1 ); + break; + } + } + } + + return $unpaired; + } + + /** + * Determines whether two condition sets represent the same nesting level. + * + * Compares the full set of enclosing condition pointers and their token codes, so + * that two calls in different conditional branches (same depth, different blocks) + * are not treated as nested identically. + * + * @since 1.4.0 + * + * @param array $a The first set of conditions (token pointer => token code). + * @param array $b The second set of conditions (token pointer => token code). + * @return bool True when both calls sit at the same nesting level. + */ + private function same_nesting( array $a, array $b ) { + return $a === $b; + } + + /** + * Best-effort heuristic for hook-paired buffers that should not be flagged. + * + * Per issue #1314, an ob_start() that is intentionally closed via a hook callback + * registered immediately next to it, in a way that is statically obvious, should + * not be considered an issue. This checks whether the statement immediately + * following the ob_start() is a call to add_action() or add_filter(), which is the + * statically obvious signal that the buffer is managed by a hooked callback. + * + * This is intentionally conservative: it only suppresses the warning when the hook + * registration directly follows the ob_start() call, to avoid masking genuinely + * unclosed buffers. Resolving the registered callback and verifying it closes the + * buffer is out of scope for static analysis. + * + * @since 1.4.0 + * + * @param int $start_ptr The position of the ob_start() call. + * @return bool True when the buffer appears to be closed via an adjacent hook callback. + */ + private function is_hook_paired( $start_ptr ) { + $next_statement = $this->phpcsFile->findEndOfStatement( $start_ptr ); + if ( false === $next_statement ) { + return false; + } + + $hook_ptr = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $next_statement + 1 ), null, true ); + if ( false === $hook_ptr ) { + return false; + } + + $name = $this->get_global_function_name( $hook_ptr ); + if ( null === $name ) { + return false; + } + + return in_array( strtolower( $name ), array( 'add_action', 'add_filter' ), true ); + } +} diff --git a/phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/UnclosedObStartUnitTest.inc b/phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/UnclosedObStartUnitTest.inc new file mode 100644 index 000000000..8a5b4df3b --- /dev/null +++ b/phpcs-sniffs/PluginCheck/Tests/CodeAnalysis/UnclosedObStartUnitTest.inc @@ -0,0 +1,42 @@ + Key is the line number and value is the number of expected errors. + */ + public function getErrorList() { + return array(); + } + + /** + * Returns the lines where warnings should occur. + * + * @return array Key is the line number and value is the number of expected warnings. + */ + public function getWarningList() { + return array( + 14 => 1, // Case: ob_start() at file scope with no closing call. + 17 => 1, // Case: multiple ob_start() in the same scope, only one closed. + 29 => 1, // Case: ob_start() closed only conditionally via an if block. + ); + } + + /** + * Returns the fully qualified class name (FQCN) of the sniff. + * + * @return string The fully qualified class name of the sniff. + */ + protected function get_sniff_fqcn() { + return UnclosedObStartSniff::class; + } + + /** + * Sets the parameters for the sniff. + * + * @throws \RuntimeException If unable to set the ruleset parameters required for the test. + * + * @param Sniff $sniff The sniff being tested. + */ + public function set_sniff_parameters( Sniff $sniff ) { + } +} diff --git a/phpcs-sniffs/PluginCheck/ruleset.xml b/phpcs-sniffs/PluginCheck/ruleset.xml index 047e63acf..3abdeaab3 100644 --- a/phpcs-sniffs/PluginCheck/ruleset.xml +++ b/phpcs-sniffs/PluginCheck/ruleset.xml @@ -10,6 +10,7 @@ + diff --git a/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php b/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php index 976af6f0d..3b66517bc 100644 --- a/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php +++ b/tests/phpunit/testdata/plugins/test-plugin-unclosed-ob-start/load.php @@ -34,3 +34,20 @@ function conditional_close() { ob_end_clean(); } } + +// 6. Fully-qualified \ob_start() paired in the same function → no issue (PHP 8 FQN handling). +function fqn_paired() { + \ob_start(); + echo 'hello'; + \ob_end_clean(); +} + +// 7. ob_start() closed via an immediately adjacent hook callback → no issue (statically obvious). +function hook_paired() { + ob_start(); + add_action( 'template_redirect', 'test_plugin_unclosed_ob_start_close' ); +} + +function test_plugin_unclosed_ob_start_close() { + ob_end_clean(); +} diff --git a/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php b/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php index 3b69447e6..432c59f29 100644 --- a/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php +++ b/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php @@ -11,28 +11,29 @@ class Unclosed_Ob_Start_Check_Tests extends WP_UnitTestCase { - public function test_run() { + public function test_run_with_errors() { + $check = new Unclosed_Ob_Start_Check(); $check_context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-unclosed-ob-start/load.php' ); $check_result = new Check_Result( $check_context ); - $check = new Unclosed_Ob_Start_Check(); $check->run( $check_result ); $warnings = $check_result->get_warnings(); + $this->assertNotEmpty( $warnings ); $this->assertArrayHasKey( 'load.php', $warnings ); - $this->assertCount( 3, $warnings['load.php'] ); + $this->assertEquals( 3, $check_result->get_warning_count() ); // 14: ob_start() at the top of the file with no closing call. $this->assertArrayHasKey( 14, $warnings['load.php'] ); - $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][14][0][0]['code'] ); + $this->assertEquals( 'PluginCheck.CodeAnalysis.UnclosedObStart.UnclosedObStart', $warnings['load.php'][14][1][0]['code'] ); // 18: Multiple ob_start() in the same scope, only one closed. $this->assertArrayHasKey( 18, $warnings['load.php'] ); - $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][18][0][0]['code'] ); + $this->assertEquals( 'PluginCheck.CodeAnalysis.UnclosedObStart.UnclosedObStart', $warnings['load.php'][18][2][0]['code'] ); // 32: ob_start() inside a function, closed only conditionally (if). $this->assertArrayHasKey( 32, $warnings['load.php'] ); - $this->assertEquals( 'unclosed_ob_start', $warnings['load.php'][32][0][0]['code'] ); + $this->assertEquals( 'PluginCheck.CodeAnalysis.UnclosedObStart.UnclosedObStart', $warnings['load.php'][32][2][0]['code'] ); } } From d218081a2aee01f9e38f3dc3dde8196de91758b8 Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Sun, 21 Jun 2026 19:02:46 +0600 Subject: [PATCH 5/5] fix(checks): add CATEGORY_PLUGIN_REPO, DRY docs URL, bump @since to 2.1.0 - Add CATEGORY_PLUGIN_REPO so check runs in Plugin Directory UI by default - Override add_result_message_for_file to use get_documentation_url() when no docs URL provided - Bump @since tags from 1.4.0 to 2.1.0 in check and sniff - Add test_get_categories() asserting both categories Refs #1318 --- .../Performance/Unclosed_Ob_Start_Check.php | 21 ++++++++----- .../CodeAnalysis/UnclosedObStartSniff.php | 30 +++++++++---------- .../Checks/Unclosed_Ob_Start_Check_Tests.php | 11 +++++++ 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php b/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php index e8abe5f02..7215375e6 100644 --- a/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php +++ b/includes/Checker/Checks/Performance/Unclosed_Ob_Start_Check.php @@ -15,7 +15,7 @@ /** * Check for unclosed ob_start() calls. * - * @since 1.4.0 + * @since 2.1.0 */ class Unclosed_Ob_Start_Check extends Abstract_PHP_CodeSniffer_Check { @@ -26,18 +26,21 @@ class Unclosed_Ob_Start_Check extends Abstract_PHP_CodeSniffer_Check { * * Every check must have at least one category. * - * @since 1.4.0 + * @since 2.1.0 * * @return array The categories for the check. */ public function get_categories() { - return array( Check_Categories::CATEGORY_PERFORMANCE ); + return array( + Check_Categories::CATEGORY_PERFORMANCE, + Check_Categories::CATEGORY_PLUGIN_REPO, + ); } /** * Returns an associative array of arguments to pass to PHPCS. * - * @since 1.4.0 + * @since 2.1.0 * * @param Check_Result $result The check result to amend, including the plugin context to check. * @return array An associative array of PHPCS CLI arguments. @@ -55,7 +58,7 @@ protected function get_args( Check_Result $result ) { * * Every check must have a short description explaining what the check does. * - * @since 1.4.0 + * @since 2.1.0 * * @return string Description. */ @@ -68,7 +71,7 @@ public function get_description(): string { * * Every check must have a URL with further information about the check. * - * @since 1.4.0 + * @since 2.1.0 * * @return string The documentation URL. */ @@ -80,7 +83,7 @@ public function get_documentation_url(): string { * Amends the given result with a message for the specified file, including the * documentation URL for this check. * - * @since 1.4.0 + * @since 2.1.0 * * @param Check_Result $result The check result to amend, including the plugin context to check. * @param bool $error Whether it is an error or notice. @@ -93,7 +96,9 @@ 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 ) { - $docs = __( 'https://make.wordpress.org/core/2025/11/18/wordpress-6-9-frontend-performance-field-guide/#introduce-the-template-enhancement-output-buffer', 'plugin-check' ); + if ( '' === $docs ) { + $docs = $this->get_documentation_url(); + } parent::add_result_message_for_file( $result, $error, $message, $code, $file, $line, $column, $docs, $severity ); } diff --git a/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/UnclosedObStartSniff.php b/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/UnclosedObStartSniff.php index c290fc754..8f704b14a 100644 --- a/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/UnclosedObStartSniff.php +++ b/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/UnclosedObStartSniff.php @@ -14,14 +14,14 @@ * Detects calls to ob_start() that are not paired with a corresponding buffer-closing * function within the same logical scope. * - * @since 1.4.0 + * @since 2.1.0 */ final class UnclosedObStartSniff extends Sniff { /** * Buffer-closing functions that pair with ob_start(). * - * @since 1.4.0 + * @since 2.1.0 * * @var array */ @@ -38,7 +38,7 @@ final class UnclosedObStartSniff extends Sniff { * Populated during the token walk and evaluated once the file has been * fully processed. * - * @since 1.4.0 + * @since 2.1.0 * * @var array, closes: array}> */ @@ -47,7 +47,7 @@ final class UnclosedObStartSniff extends Sniff { /** * Ordered list of scope start pointers, used to process scopes in source order. * - * @since 1.4.0 + * @since 2.1.0 * * @var array */ @@ -56,7 +56,7 @@ final class UnclosedObStartSniff extends Sniff { /** * Whether the current file has been finalized (scopes evaluated). * - * @since 1.4.0 + * @since 2.1.0 * * @var bool */ @@ -65,7 +65,7 @@ final class UnclosedObStartSniff extends Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @since 1.4.0 + * @since 2.1.0 * * @return array */ @@ -83,7 +83,7 @@ public function register() { /** * Processes this test, when one of its tokens is encountered. * - * @since 1.4.0 + * @since 2.1.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. @@ -114,7 +114,7 @@ public function process_token( $stackPtr ) { * soon as no further registered tokens remain ahead, ensuring all collected calls * are available before pairing. * - * @since 1.4.0 + * @since 2.1.0 * * @param int $stackPtr The position of the current token in the stack. */ @@ -139,7 +139,7 @@ private function maybe_finalize( $stackPtr ) { * single T_NAME_FULLY_QUALIFIED token (PHP >= 8.0) so that \ob_start() in namespaced * code is detected consistently. * - * @since 1.4.0 + * @since 2.1.0 * * @param int $stackPtr The position of the current token in the stack. * @return string|null The lowercased function name, or null when not a global call. @@ -205,7 +205,7 @@ private function get_global_function_name( $stackPtr ) { * The file scope is represented by a pointer of -1. Function, method and closure * scopes use the pointer of their opening condition token. * - * @since 1.4.0 + * @since 2.1.0 * * @param int $stackPtr The position of the call. * @param bool $is_start Whether this is an ob_start() (true) or a closing call (false). @@ -245,7 +245,7 @@ private function record_call( $stackPtr, $is_start ) { * Returns the pointer of the nearest enclosing function, method or closure, or -1 * for the file (top-level) scope. * - * @since 1.4.0 + * @since 2.1.0 * * @param int $stackPtr The position of the token. * @return int The scope pointer, or -1 for the file scope. @@ -271,7 +271,7 @@ private function get_scope_pointer( $stackPtr ) { * Called once when the end of the file is reached so that the full scope contents * are available for pairing. * - * @since 1.4.0 + * @since 2.1.0 */ private function finalize_file() { $this->finalized = true; @@ -315,7 +315,7 @@ private function finalize_file() { * ob_start() calls that remain after all closes have been consumed are returned as * unpaired. * - * @since 1.4.0 + * @since 2.1.0 * * @param array $starts The ob_start() calls. * @param array $closes The closing calls. @@ -352,7 +352,7 @@ private function pair_starts_and_closes( array $starts, array $closes ) { * that two calls in different conditional branches (same depth, different blocks) * are not treated as nested identically. * - * @since 1.4.0 + * @since 2.1.0 * * @param array $a The first set of conditions (token pointer => token code). * @param array $b The second set of conditions (token pointer => token code). @@ -376,7 +376,7 @@ private function same_nesting( array $a, array $b ) { * unclosed buffers. Resolving the registered callback and verifying it closes the * buffer is out of scope for static analysis. * - * @since 1.4.0 + * @since 2.1.0 * * @param int $start_ptr The position of the ob_start() call. * @return bool True when the buffer appears to be closed via an adjacent hook callback. diff --git a/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php b/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php index 432c59f29..6f2b04416 100644 --- a/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php +++ b/tests/phpunit/tests/Checker/Checks/Unclosed_Ob_Start_Check_Tests.php @@ -5,12 +5,23 @@ * @package plugin-check */ +use WordPress\Plugin_Check\Checker\Check_Categories; use WordPress\Plugin_Check\Checker\Check_Context; use WordPress\Plugin_Check\Checker\Check_Result; use WordPress\Plugin_Check\Checker\Checks\Performance\Unclosed_Ob_Start_Check; class Unclosed_Ob_Start_Check_Tests extends WP_UnitTestCase { + public function test_get_categories() { + $check = new Unclosed_Ob_Start_Check(); + $categories = $check->get_categories(); + + $this->assertIsArray( $categories ); + $this->assertContains( Check_Categories::CATEGORY_PERFORMANCE, $categories ); + $this->assertContains( Check_Categories::CATEGORY_PLUGIN_REPO, $categories ); + $this->assertCount( 2, $categories ); + } + public function test_run_with_errors() { $check = new Unclosed_Ob_Start_Check(); $check_context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-unclosed-ob-start/load.php' );