Update sniff to enforce more early returns

[Crabcyborg]

This wasn't counting lines inside of conditions, where this is most important since there is more indentation.

[coderabbitai]

📝 Walkthrough

Walkthrough

Three files are refactored to implement an early-return guard pattern instead of traditional if-block logic. Permission checks and error handling now return early when conditions are unmet. A PHPCS sniff's statement counter is updated to count all semicolons recursively rather than only at the immediate nesting level.

Changes

Cohort / File(s)	Summary
Early-return guard refactoring classes/controllers/FrmFormsController.php, classes/models/FrmInstallerSkin.php	Permission check and error-handling logic inverted to use early returns. In FrmFormsController, unauthorized users trigger an early exit before button rendering. In FrmInstallerSkin, the error handler returns early when no error exists. Functional behavior preserved; control flow restructured.
Statement counter logic update phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php	countStatementsInScope method changed to count all semicolons recursively across nested scopes instead of only at the immediate nesting level. Target-level tracking and conditional nesting logic removed. May affect detection of statement density in sniff analysis.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• New sniff to remove extra white space before closing brace #2807: Modifies the same PHPCS sniff file (FlipIfToEarlyReturnSniff.php) with operator-flipping logic changes.
• New sniff to add early returns to functions that are one big if #2803: Implements the same early-return guard refactoring pattern and also modifies statement counting behavior in the sniff.

Poem

🐰 Early returns hop through the code,
Guards at the gates, a clearer road,
Nesting levels now fully deep,
Counts recursive, promises to keep,
Logic flows with purpose clear—hop, hop, hooray! 🌿

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Update sniff to enforce more early returns' accurately reflects the primary change: improvements to the code style enforcement sniff to promote early return patterns, demonstrated by refactoring in both the sniff itself and example files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

classes/models/FrmInstallerSkin.php (1)

77-88: Consider using wp_json_encode instead of json_encode.

WordPress recommends using wp_json_encode() for consistency and to handle potential encoding issues more gracefully.

Suggested change

-		echo json_encode(
+		echo wp_json_encode(
 			array(
 				'error'   => $errors,
 				'message' => $errors,
 				'success' => false,
 			)
 		);

phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php (1)

383-411: Docblock says "recursively" but implementation counts all semicolons linearly.

The implementation simply iterates through all tokens within the scope and counts semicolons at any nesting level. This is intentional behavior change, but the docblock term "recursively" may be slightly misleading since there's no actual recursion—it's a linear scan.

Consider updating the docblock to better reflect the actual behavior:

Suggested docblock clarification

 	/**
-	 * Count the number of statements inside a scope (recursively).
+	 * Count the number of statements inside a scope (including nested blocks).
 	 *
 	 * `@param` File $phpcsFile The file being scanned.
 	 * `@param` int  $scopeToken The token with the scope to count.
 	 *
 	 * `@return` int
 	 */

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa3667e and c95b629.

📒 Files selected for processing (3)

• classes/controllers/FrmFormsController.php
• classes/models/FrmInstallerSkin.php
• phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php

🧰 Additional context used

📓 Path-based instructions (1)

**/*.php

⚙️ CodeRabbit configuration file

**/*.php: - Do not make suggestions when we use x.x for a version placeholder. This is used like "@SInCE x.x" in a PHP doc comment, but also like 'x.x' when functions like _deprecated_function() are used.

• Treat these placeholders as acceptable in this codebase.

Files:

• phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php
• classes/controllers/FrmFormsController.php
• classes/models/FrmInstallerSkin.php

🧬 Code graph analysis (1)

classes/controllers/FrmFormsController.php (1)

classes/helpers/FrmAppHelper.php (5)

• current_user_can (2263-2281)
• FrmAppHelper (6-5030)
• load_admin_wide_js (3804-3825)
• svg_logo (324-339)
• kses (1052-1055)

⏰ 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: 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: Cypress
• GitHub Check: Cypress
• GitHub Check: PHP 8 tests in WP 6.9
• GitHub Check: PHP 7.4 tests in WP 6.9

🔇 Additional comments (3)

classes/models/FrmInstallerSkin.php (1)

73-75: LGTM on the early return guard.

The early return for the falsy $errors case is a clean improvement that makes the control flow easier to follow.

classes/controllers/FrmFormsController.php (1)

1065-1079: LGTM on the early return permission check.

The early return guard for the permission check is a clean improvement. It clearly separates the authorization logic from the main functionality and makes the code more readable.

phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php (1)

403-408: Verify: This change will count semicolons from nested control structures.

The new logic counts all semicolons regardless of nesting level. For example, a for loop like for ($i = 0; $i < 10; $i++) contains two semicolons in its header, and any statements inside the loop body would also be counted.

This means an if block containing a single for loop with 3+ iterations could easily exceed the minStatements threshold of 5, triggering the sniff even when the if block has relatively simple logic.

Is this the intended behavior? If so, this is fine—just confirming this is the expected stricter enforcement.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 0% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.09%. Comparing base (0c50d33) to head (c95b629).
⚠️ Report is 43 commits behind head on master.

Files with missing lines	Patch %	Lines
classes/models/FrmInstallerSkin.php	0.00%	16 Missing ⚠️
classes/controllers/FrmFormsController.php	0.00%	8 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2818      +/-   ##
============================================
- Coverage     27.09%   27.09%   -0.01%     
  Complexity     8840     8840              
============================================
  Files           145      145              
  Lines         29760    29758       -2     
============================================
- Hits           8063     8062       -1     
+ Misses        21697    21696       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.