Update sniff to improve whitespace consistency#2768
Conversation
WalkthroughThis PR updates a PHPCS whitespace sniff to enforce blank lines after closing braces and function calls, then applies resulting formatting improvements across multiple files in the codebase and test suite. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 0
🧹 Nitpick comments (2)
phpcs-sniffs/Formidable/Sniffs/WhiteSpace/BlankLineAfterClosingBraceSniff.php (2)
172-191: Potential cross-platform issue with newline detection.The check for blank lines uses
$tokens[ $i ]['content'] === "\n"which may not match"\r\n"on Windows or in files with Windows line endings.Consider using a more robust newline check
- for ( $i = $functionStart + 1; $i < $functionEnd; $i++ ) { - if ( $tokens[ $i ]['code'] === T_WHITESPACE && $tokens[ $i ]['content'] === "\n" ) { + for ( $i = $functionStart + 1; $i < $functionEnd; $i++ ) { + if ( $tokens[ $i ]['code'] === T_WHITESPACE && preg_match( '/\r?\n/', $tokens[ $i ]['content'] ) ) {Alternatively, you could check if the content contains only whitespace and spans multiple lines by comparing line numbers of adjacent tokens.
193-199: Error message doesn't reflect expanded scope.The error message still says "variable assignment" but the sniff now also handles function calls. Consider updating for clarity.
Suggested update
$fix = $phpcsFile->addFixableError( - 'Expected blank line after closing brace before variable assignment', + 'Expected blank line after closing brace before statement', $stackPtr, 'MissingBlankLine' );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
classes/controllers/FrmFormsController.phpclasses/controllers/FrmSettingsController.phpclasses/helpers/FrmAppHelper.phpclasses/helpers/FrmFormsHelper.phpclasses/models/FrmMigrate.phpclasses/models/fields/FrmFieldCombo.phpphpcs-sniffs/Formidable/Sniffs/WhiteSpace/BlankLineAfterClosingBraceSniff.phpsquare/controllers/FrmSquareLiteEventsController.phpsquare/helpers/FrmSquareLiteConnectHelper.phpstripe/controllers/FrmStrpLiteEventsController.phpstripe/helpers/FrmStrpLiteConnectHelper.phptests/phpunit/misc/test_FrmSpamCheckDenylist.php
⏰ 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). (6)
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 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
🔇 Additional comments (18)
classes/controllers/FrmSettingsController.php (1)
266-267: Formatting improvement to enhance code clarity.The addition of a blank line between the function call and the final
wp_die()improves code readability by visually separating the main logic from the exit statement.classes/models/FrmMigrate.php (2)
651-652: Formatting improvement to enhance code clarity.The blank line between the loop processing and the
update_option()call improves readability by clearly separating the data processing from the final save operation.
769-770: Formatting improvement to enhance code clarity.The blank line between the loop processing and the
update_option()call improves readability and maintains consistency with the similar pattern inrevert_widget_field_size().classes/helpers/FrmFormsHelper.php (1)
1843-1844: Formatting improvement to separate early-return logic from main processing.The blank line visually separates the early return guard clause from the subsequent logic, improving code clarity without changing any behavior.
classes/models/fields/FrmFieldCombo.php (1)
404-408: Formatting change aligns with sniff goals.The added blank line before the
do_action()call follows the PR objective to enforce whitespace consistency after logical code segments. No functional changes detected.stripe/controllers/FrmStrpLiteEventsController.php (2)
315-324: Formatting improvement for blank line after conditional block.The added blank line at line 323 before the
remove_filter()call improves readability after the subscription cancellation conditional block. Functional behavior is unchanged.
454-463: Formatting change maintains existing behavior.The blank line added before
wp_send_json_success()follows the sniff enforcement for spacing after conditional logic. Event processing logic remains unaffected.square/controllers/FrmSquareLiteEventsController.php (1)
50-59: Whitespace consistency applied to event processing method.The blank line added before
wp_send_json_success()follows the sniff's enforcement pattern for spacing after conditional blocks. The change is formatting-only and preserves all functional behavior.classes/helpers/FrmAppHelper.php (2)
2366-2376: Whitespace-only change inFrmAppHelper::permission_check()looks safe.
The added blank line improves readability without affecting thewp_die()path.
4651-4660: Whitespace-only change inFrmAppHelper::set_current_screen_and_hook_suffix()looks safe.
The blank line separation beforeset_current_screen()is purely formatting; behavior is unchanged.square/helpers/FrmSquareLiteConnectHelper.php (1)
786-805: Whitespace-only change inFrmSquareLiteConnectHelper::verify()looks safe.
Adds a blank line after the success response; no control-flow change.classes/controllers/FrmFormsController.php (1)
28-38: Whitespace-only change inFrmFormsController::menu()looks safe.
Improves visual separation between conditional label tweak andadd_submenu_page()call.tests/phpunit/misc/test_FrmSpamCheckDenylist.php (1)
307-336: LGTM - Formatting changes applied correctly.The blank lines added after the inline filter functions (lines 313 and 333) align with the updated sniff rules for blank lines after closing braces.
Note: Defining functions at the global scope inside a test method is unconventional and could cause "cannot redeclare function" errors if the test runs multiple times in the same process. Consider using anonymous functions or class methods instead. However, this is existing code and not part of the current changes.
stripe/helpers/FrmStrpLiteConnectHelper.php (2)
336-343: LGTM - Formatting change applied correctly.The added blank line after the success path's closing brace (where
wp_send_json_success()is called) follows the updated whitespace rules. The control flow is correct sincewp_send_json_success()terminates execution, makingwp_send_json_error()on line 342 only reachable when the condition on line 338 is false.
960-964: LGTM - Formatting change applied correctly.The blank line added after the success conditional block aligns with the updated sniff rules. The logic remains correct:
wp_send_json_success()terminates, sowp_send_json_error()is only reached when the identifier check fails.phpcs-sniffs/Formidable/Sniffs/WhiteSpace/BlankLineAfterClosingBraceSniff.php (3)
38-40: LGTM - Registration expanded to handle function calls.Good addition of
T_STRINGto catch function calls in addition to variable assignments, broadening the sniff's coverage.
60-87: LGTM - Clean differentiation between variable assignments and function calls.The logic correctly handles both cases:
- Variables require a following assignment operator
- Strings (function names) require a following opening parenthesis
The comprehensive list of assignment operators ensures all assignment patterns are covered.
139-167: Scope detection logic is sound.The backward search for containing function/closure with scope boundary validation is well-implemented. Decrementing
$searchPtrafter each iteration ensures all potential containing scopes are checked.
…espace_consistency Update sniff to improve whitespace consistency
1 participant
No description provided.