New sniff to prefer ob_get_clean#2764
Conversation
WalkthroughThis PR refactors output buffer management across the codebase by systematically replacing two-step buffer operations (ob_get_contents(); ob_end_clean();) with single ob_get_clean() calls. Additionally, a new PHP_CodeSniffer sniff is introduced to detect and enforce this pattern going forward. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
phpcs.xmlis excluded by!**/*.xml
📒 Files selected for processing (16)
classes/controllers/FrmEntriesAJAXSubmitController.phpclasses/controllers/FrmFieldsController.phpclasses/controllers/FrmFormsController.phpclasses/helpers/FrmAppHelper.phpclasses/models/FrmStyle.phpclasses/models/fields/FrmFieldCreditCard.phpclasses/models/fields/FrmFieldDefault.phpclasses/models/fields/FrmFieldType.phpphpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferObGetCleanSniff.phpstripe/helpers/FrmStrpLiteConnectHelper.phpstripe/models/FrmStrpLiteAuth.phptests/phpunit/base/FrmUnitTest.phptests/phpunit/forms/test_FrmFormsController.phptests/phpunit/misc/test_FrmAppController.phptests/phpunit/misc/test_FrmAppHelper.phptests/phpunit/styles/test_FrmStylesController.php
🧰 Additional context used
🧬 Code graph analysis (2)
classes/models/fields/FrmFieldType.php (2)
classes/controllers/FrmFieldsController.php (1)
input_html(527-552)classes/models/fields/FrmFieldRadio.php (1)
input_html(42-44)
classes/models/fields/FrmFieldDefault.php (4)
classes/controllers/FrmFieldsController.php (1)
input_html(527-552)classes/models/fields/FrmFieldType.php (1)
input_html(227-229)classes/models/fields/FrmFieldCheckbox.php (1)
input_html(37-39)classes/models/fields/FrmFieldRadio.php (1)
input_html(42-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: Cypress
🔇 Additional comments (24)
classes/controllers/FrmEntriesAJAXSubmitController.php (1)
81-83: LGTM!Clean refactoring to consolidate output buffering into a single
ob_get_clean()call. The change maintains the same functionality while being more concise.classes/models/FrmStyle.php (1)
437-439: LGTM!The inline use of
ob_get_clean()within the nested function calls is clean and maintains the same functionality as the previous two-step pattern.stripe/helpers/FrmStrpLiteConnectHelper.php (1)
643-645: LGTM!Consistent application of the
ob_get_clean()pattern for capturing template output. The refactoring is correct and aligns with the PR's objective.stripe/models/FrmStrpLiteAuth.php (1)
71-73: LGTM!The consolidation to
ob_get_clean()is correct and maintains the same behavior while being more concise.classes/helpers/FrmAppHelper.php (2)
1535-1544: LGTM!The refactored control flow using a single return statement with
ob_get_clean()is cleaner and maintains the same functionality. The logic is correct: when$echois false, it captures and returns the buffer; otherwise returns null.
2661-2669: LGTM!Clean consolidation of the buffer capture pattern. The direct return of
ob_get_clean()is correct and more concise than the previous two-step approach.classes/models/fields/FrmFieldType.php (2)
201-203: LGTM!The refactor to
ob_get_clean()correctly consolidates the two-step buffer operation into a single call while preserving the same semantics.
1522-1524: LGTM!The refactor to
ob_get_clean()correctly consolidates the buffer operation while preserving the same return value and behavior.phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferObGetCleanSniff.php (2)
22-31: LGTM!The sniff structure follows PHP_CodeSniffer conventions correctly. Registering for
T_STRINGtokens is appropriate for detecting function calls likeob_get_contents().
86-98: Auto-fix logic is sound.The fixer correctly:
- Replaces
ob_get_contentswithob_get_clean- Removes the redundant
ob_end_clean()call and everything between the two statementsThe implementation achieves the PR's objective of consolidating the two-step buffer operation.
classes/controllers/FrmFieldsController.php (1)
51-53: LGTM!The refactor to
ob_get_clean()correctly consolidates the buffer operation while maintaining the same behavior for capturing field HTML.tests/phpunit/styles/test_FrmStylesController.php (1)
22-24: LGTM!Both test methods correctly refactor to
ob_get_clean(), consolidating the buffer operations while preserving the same test behavior and assertions.Also applies to: 72-74
classes/models/fields/FrmFieldCreditCard.php (1)
55-66: LGTM!Directly returning
ob_get_clean()is a clean simplification that eliminates the need for an intermediate variable while maintaining the same functionality.tests/phpunit/misc/test_FrmAppHelper.php (3)
463-466: LGTM! Clean buffer management.The refactoring from
ob_get_contents()+ob_end_clean()toob_get_clean()is correct and more concise.
479-482: LGTM! Consistent refactoring.Correctly applies the same buffer consolidation pattern.
496-499: LGTM! Consistent pattern applied.All three buffer management refactorings in this file follow the same correct pattern.
tests/phpunit/forms/test_FrmFormsController.php (1)
370-373: LGTM! Simplified buffer handling.The
post_new_entrymethod correctly usesob_get_clean()to capture and return the output fromFrmEntriesController::process_entry()while cleaning the buffer in a single operation.classes/controllers/FrmFormsController.php (1)
2506-2518: LGTM! Production code refactoring is safe.The
get_formmethod correctly consolidates buffer operations. The change fromob_get_contents()+ob_end_clean()toob_get_clean()maintains identical behavior while being more concise.tests/phpunit/base/FrmUnitTest.php (2)
462-466: LGTM! Clean buffer management in test utility.The
get_footer_outputmethod correctly usesob_get_clean()to capture and return footer output.
586-596: LGTM! Buffer refactoring plus resource management improvement.The change to
ob_get_clean()on line 588 is correct. Additionally, the explicitfclose($fw)on line 595 is a good improvement for proper resource cleanup, even though it's beyond the scope of this PR's primary objective.tests/phpunit/misc/test_FrmAppController.php (2)
25-28: LGTM! Consistent buffer handling.The
test_menumethod correctly usesob_get_clean()to capture menu output.
84-88: LGTM! Consistent pattern applied.The
test_load_wp_admin_stylemethod correctly usesob_get_clean()to capture style output.classes/models/fields/FrmFieldDefault.php (2)
38-41: LGTM! Clean refactoring to idiomatic PHP.Using
ob_get_clean()is the preferred PHP pattern for capturing buffered output, combiningob_get_contents()andob_end_clean()into a single call. This improves code clarity without changing behavior.
61-64: LGTM! Consistent refactoring with proper safety checks.The
ob_get_clean()refactoring matches the pattern inshow_on_form_builder. The defensiveis_string()check on line 66 appropriately handles the (unlikely) case whereob_get_clean()returnsfalse.
…lean New sniff to prefer ob_get_clean
1 participant
No description provided.