New sniff to add early returns to functions that are one big if

[Crabcyborg]

Summary by CodeRabbit

• Refactor

  • Streamlined page template fallback and onboarding wizard flows by adopting early-return patterns for clearer, more predictable rendering and initialization.

• Chores

  • Added a new code-analysis rule to detect long single-if blocks and suggest or auto-apply transforms to early returns, improving code clarity and consistency.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new PHP_CodeSniffer sniff that flips single-if-bodied functions to early-return form, registers it in the ruleset, and applies the early-return refactor to two controller methods by inverting conditionals and using early returns.

Changes

Cohort / File(s)	Summary
New PHPCS sniff phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php	New sniff class implementing detection and automatic fix to convert functions whose entire body is a single if into early-return form. Adds configuration property minStatements and helper methods for negation, dedent, statement counting, and fix application.
Ruleset update phpcs-sniffs/Formidable/ruleset.xml	Registers the new Formidable.CodeAnalysis.FlipIfToEarlyReturn rule in the Formidable ruleset.
Controller edits classes/controllers/FrmFormsController.php, classes/controllers/FrmOnboardingWizardController.php	Refactors two methods by inverting conditionals and introducing early returns: fallback_when_page_template_part_is_not_supported_by_theme and maybe_load_page. Control flow reflowed (header/content/footer ordering and filter placement adjusted in FrmFormsController).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Fix issue with multiple forms appearing in the in-theme preview #2382: Modifies fallback_when_page_template_part_is_not_supported_by_theme in FrmFormsController.php, including changes around the frm_filter_final_form filter — directly overlapping controller edits.
• Move formidable sniffs to separate ruleset file #2801: Changes PHPCS sniffs/ruleset configuration; related to adding and registering new sniffs.
• New sniff to prefer ob_get_clean #2764: Adds a code analysis sniff (PreferObGetClean); similar pattern of introducing automated sniffs and fixes.

Poem

🐰 I hopped through code at break of dawn,
Found an if that hugged its lawn,
Flipped it quick to early return,
Less nesting now — no need to yearn,
A tidy path, a brighter morn ✨

🚥 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 accurately captures the main purpose of this PR: introducing a new PHP_CodeSniffer sniff that converts functions where the entire body is within a single if statement into early return patterns.
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

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

50-52: Remove T_CLOSURE from registration since it's immediately skipped.

The sniff registers for T_CLOSURE but immediately returns without processing in process(). This causes unnecessary invocations for every closure in the codebase.

Suggested fix

 	public function register() {
-		return array( T_FUNCTION, T_CLOSURE );
+		return array( T_FUNCTION );
 	}

And remove the now-unnecessary check in process():

-		// Skip closures.
-		if ( $tokens[ $stackPtr ]['code'] === T_CLOSURE ) {
-			return;
-		}
-

Also applies to: 66-68

---

98-104: Remove unused hasElseOrElseif() method or refactor to use it.

The else/elseif check is performed inline in process() (lines 98-104), making the hasElseOrElseif() method dead code. Static analysis confirms this method is unused.

Option 1: Use the existing method

 		// Check if this if has an else or elseif - skip those.
-		if ( isset( $tokens[ $ifToken ]['scope_closer'] ) ) {
-			$next = $phpcsFile->findNext( T_WHITESPACE, $tokens[ $ifToken ]['scope_closer'] + 1, null, true );
-
-			if ( false !== $next && in_array( $tokens[ $next ]['code'], array( T_ELSE, T_ELSEIF ), true ) ) {
-				return;
-			}
+		if ( $this->hasElseOrElseif( $phpcsFile, $ifToken ) ) {
+			return;
 		}

Option 2: Remove the unused method entirely

-	/**
-	 * Check if an if statement has an else or elseif clause.
-	 *
-	 * `@param` File $phpcsFile The file being scanned.
-	 * `@param` int  $ifToken   The position of the if token.
-	 *
-	 * `@return` bool
-	 */
-	private function hasElseOrElseif( File $phpcsFile, $ifToken ) {
-		$tokens = $phpcsFile->getTokens();
-
-		if ( ! isset( $tokens[ $ifToken ]['scope_closer'] ) ) {
-			return false;
-		}
-
-		$scopeCloser = $tokens[ $ifToken ]['scope_closer'];
-
-		// Look for else or elseif after the if's closing brace.
-		$next = $phpcsFile->findNext( T_WHITESPACE, $scopeCloser + 1, null, true );
-
-		if ( false === $next ) {
-			return false;
-		}
-
-		return in_array( $tokens[ $next ]['code'], array( T_ELSE, T_ELSEIF ), true );
-	}

Also applies to: 320-337

---

159-159: Remove unused parameters $funcOpener and $funcCloser.

These parameters are passed to applyFix() but never used within the method body.

Suggested fix

Update the method signature:

-	private function applyFix( File $phpcsFile, $ifToken, $funcOpener, $funcCloser ) {
+	private function applyFix( File $phpcsFile, $ifToken ) {

Update the call site:

-		$this->applyFix( $phpcsFile, $ifToken, $scopeOpener, $scopeCloser );
+		$this->applyFix( $phpcsFile, $ifToken );

Also update the docblock to remove the parameter documentation.

Also applies to: 145-145

---

219-234: Consider handling cross-platform line endings.

The method uses hardcoded "\n" for line splitting, which may leave trailing \r characters on Windows-style (\r\n) files. This could result in inconsistent line endings in the output.

Suggested fix

 	private function dedentCode( $code ) {
-		$lines  = explode( "\n", $code );
+		// Normalize line endings before splitting.
+		$code   = str_replace( "\r\n", "\n", $code );
+		$code   = str_replace( "\r", "\n", $code );
+		$lines  = explode( "\n", $code );
 		$result = array();

---

369-378: Consider adding T_DO to control structures list for completeness.

The do-while loop (T_DO) is not included in the control structures list. While the trailing semicolon of a do-while would still be counted, adding T_DO would make the counting behavior consistent with other control structures.

Suggested fix

 			// Also count control structures as statements.
-			if ( in_array( $tokens[ $i ]['code'], array( T_IF, T_FOREACH, T_FOR, T_WHILE, T_SWITCH, T_TRY ), true ) ) {
+			if ( in_array( $tokens[ $i ]['code'], array( T_IF, T_FOREACH, T_FOR, T_WHILE, T_DO, T_SWITCH, T_TRY ), true ) ) {

---

📜 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 44bafef and 0c50d33.

📒 Files selected for processing (1)

• 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

🪛 PHPMD (2.15.0)

phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php

159-159: Avoid unused parameters such as '$funcOpener'. (undefined)

(UnusedFormalParameter)

---

159-159: Avoid unused parameters such as '$funcCloser'. (undefined)

(UnusedFormalParameter)

---

320-337: Avoid unused private methods such as 'hasElseOrElseif'. (undefined)

(UnusedPrivateMethod)

⏰ 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: Run Rector inspection
• GitHub Check: Run ESLint
• GitHub Check: Run PHP Syntax inspection (8.4)
• GitHub Check: PHP 7.4 tests in WP 6.9
• GitHub Check: PHP 8 tests in WP 6.9
• GitHub Check: PHPStan

_{✏️ 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 10.52632% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.09%. Comparing base (09d59eb) to head (0c50d33).
⚠️ Report is 141 commits behind head on master.

Files with missing lines	Patch %	Lines
classes/controllers/FrmFormsController.php	0.00%	9 Missing ⚠️
...sses/controllers/FrmOnboardingWizardController.php	20.00%	8 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2803      +/-   ##
============================================
- Coverage     27.10%   27.09%   -0.02%     
+ Complexity     8847     8840       -7     
============================================
  Files           145      145              
  Lines         29816    29760      -56     
============================================
- Hits           8083     8063      -20     
+ Misses        21733    21697      -36     

☔ 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.