feat(1314): add unclosed ob_start check

[faisalahammad]

What?

Closes #1314

Adds a new performance check to detect ob_start() calls that are not paired with a corresponding buffer-closing function within the same scope. Prevents unpredictable behaviour caused by misaligned output buffers in shared WordPress environments.

Why?

Every ob_start() must be paired with a closing call (ob_get_clean(), ob_end_clean(), ob_get_flush(), or ob_end_flush()) in the same scope. Unclosed buffers break headers, lose output, and cause redirect issues in shared hosting. WordPress core, themes, and other plugins all share the buffer stack.

How?

Implemented as a PHPCS sniff (PluginCheck.CodeAnalysis.UnclosedObStart) following @westonruter's suggestion:

• Extends Abstract_PHP_CodeSniffer_Check like other sniff-based checks
• Uses PHPCS conditions data for scope tracking instead of hand-rolled brace-depth
• Resolves fully-qualified \ob_start() calls (PHP 7.4 two-token form + PHP 8+ T_NAME_FULLY_QUALIFIED)
• Flags conditional closes (e.g. ob_end_clean() only inside an if) as warnings
• Conservative hook-paired heuristic: suppresses warning when ob_start() is immediately followed by add_action()/add_filter()
• Registered in Default_Check_Repository under both CATEGORY_PERFORMANCE and CATEGORY_PLUGIN_REPO

Testing Instructions

1. Install the plugin from this branch
2. Run wp plugin check <plugin-slug> or use the Plugin Check admin UI
3. Verify the "unclosed_ob_start" check runs by default in the Plugin Directory view (CATEGORY_PLUGIN_REPO ensures it is selected automatically)
4. Test with a plugin that has an unpaired ob_start() — confirm a warning is reported on the exact line
5. Test with a plugin that has paired ob_start() / ob_end_clean() in the same function — confirm no warning
6. Test with \ob_start() in namespaced code — confirm detection works
7. Test the hook-paired case (ob_start() + add_action()) — confirm no warning

AI Usage Disclosure

• This PR was created without the help of AI tools
• This PR includes AI-assisted code or content

If AI tools were used, please describe how they were used:

• Claude Code assisted with PHPCS sniff architecture research (reviewing existing AbstractFunctionParameterSniff / AbstractFunctionRestrictionsSniff patterns in phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/)
• Drafted the scope-tracking logic (function/closure walk via $tokens[ $stackPtr ]['conditions'])
• Generated the PHP 7.4 vs 8.0 fully-qualified name handling (T_STRING + T_NS_SEPARATOR vs T_NAME_FULLY_QUALIFIED)
• Generated the PHPUnit fixture scenarios (paired, multiple, closure, conditional, FQN, hook-paired)
• Drafted the PR body and testing instructions

Screenshots or screencast

Before	After

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: faisalahammad <faisalahammad@git.wordpress.org>
Co-authored-by: davidperezgar <davidperez@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[davidperezgar]

Thanks for working on this. The new check is a good direction, but I found a few issues that should be addressed before merge:

1. Fully-qualified \ob_start() calls are currently missed. On PHP 8, \ob_start() tokenizes as T_NAME_FULLY_QUALIFIED, but the check only handles T_STRING, so namespaced plugins can bypass the check.

2. The implementation flags hook-paired buffers even when the open/close callbacks are immediately adjacent and statically obvious. Issue Unclosed ob_start() usage #1314 says those should not be considered issues, so this needs either support or a clarified scope.

3. The warning docs URL points to the old WordPress 5.7 template roadmap post, while the message recommends the WordPress 6.9 template enhancement output buffer. Please update the link to the 6.9 field-guide resource referenced in the issue.

[westonruter]

Could this be implemented as a PHPCS Sniff instead? Since it's walking over the tokens in the source file, this would seem more appropriate.

[faisalahammad]

Hi @davidperezgar @westonruter, thanks for the feedback. I reworked the check based on it.

Main change: following @westonruter's suggestion, I moved the logic out of the custom token_get_all() walker and into a real PHPCS sniff (PluginCheck.CodeAnalysis.UnclosedObStart). The check class now extends Abstract_PHP_CodeSniffer_Check, like the other sniff-based checks. This drops the hand-rolled brace-depth tracking and lets PHPCS handle scope via its conditions data.

The three issues from @davidperezgar:

1. Fully-qualified \ob_start() — the sniff now resolves the function name from both the T_NS_SEPARATOR + T_STRING form (PHP 7.4) and the single T_NAME_FULLY_QUALIFIED token (PHP 8+), guarded with defined() so 7.4 stays happy. Namespaced \ob_start() is no longer skipped.

2. Hook-paired buffers — per issue Unclosed ob_start() usage #1314, an ob_start() directly followed by add_action()/add_filter() is no longer flagged. This is intentionally narrow: only an immediately adjacent hook registration suppresses the warning, so we don't mask genuinely unclosed buffers. I kept it conservative because statically proving the callback closes the buffer is out of reach. If you'd prefer a wider or different signal here, happy to adjust.

3. Docs URL — updated to the WordPress 6.9 Frontend Performance Field Guide (the template enhancement output buffer), in both get_documentation_url() and the per-message link.

Pairing logic: a close only pairs with a start when they share the same nesting context, so a buffer closed only inside an if is still flagged. The sniff unit test covers the five cases from #1314 plus the FQN and hook-paired ones.

One thing I want your opinion on: the hook-paired heuristic is the softest part. I went with "next statement is add_action/add_filter" as the statically obvious signal, but I can see arguments for leaving it out entirely (flag everything, let the author // phpcs:ignore) or making it smarter. What do you think fits the project best?

If you'd rather keep the original token_get_all() approach, that's fine too — I can revert to it and just apply the three fixes there. Just let me know which direction you prefer.

[davidperezgar]

@faisalahammad The check is missing CATEGORY_PLUGIN_REPO in get_categories(), which means it won't run in the Plugin Directory UI by default (only when explicitly requested via --checks=unclosed_ob_start).

Other Performance checks that also apply to the plugin directory follow the same pattern of returning both categories. For example, Enqueued_Resources_Check:

public function get_categories() {
    return array(
        Check_Categories::CATEGORY_PLUGIN_REPO,
        Check_Categories::CATEGORY_PERFORMANCE,
    );
}

Please update Unclosed_Ob_Start_Check::get_categories() to include CATEGORY_PLUGIN_REPO alongside CATEGORY_PERFORMANCE.

[faisalahammad]

Hi @davidperezgar, pushed changes addressing your feedback:

1. CATEGORY_PLUGIN_REPO — added to get_categories() so the check runs by default in Plugin Directory UI. Added test_get_categories() to verify both categories.

2. Docs URL — DRYed add_result_message_for_file to use $this->get_documentation_url() (WordPress 6.9 field guide) when no docs URL is passed, so the link is consistent across all messages.

3. @SInCE tags — bumped from 1.4.0 to 2.1.0 across the check class and sniff to match the next release version.

Quality gates (PHPCS + PHPStan) pass clean. PR description updated to reflect the PHPCS sniff implementation. Ready for re-review when you have time.

[westonruter]

• This PR was created without the help of AI tools

Is this accurate?

[faisalahammad]

• This PR was created without the help of AI tools

Is this accurate?

Sorry, I unchecked the checkbox. Is there anything else I should fix before merging? Thank you.

[westonruter]

@faisalahammad then it seems you need to check this checkbox instead:

This PR includes AI-assisted code or content

Right? If so then explain how the tools were used, per the description:

If AI tools were used, please describe how they were used:

[faisalahammad]

Done. Checked the AI-assisted box and described Claude Code's role: PHPCS sniff architecture research, scope-tracking logic, PHP 7.4/8.0 FQN handling, PHPUnit fixture scenarios, PR body draft.

Could you please check again and share your feedback, @westonruter?