New sniff to return early before expensive code

[Crabcyborg]

Summary by CodeRabbit

• New Features

  • Added a new static analysis rule that detects and suggests safer optimization patterns in conditional checks.

• Bug Fixes

  • Improved duplicate-entry detection logic to avoid unnecessary checks and reduce false negatives.

• Chores

  • Minor code formatting and control-flow simplifications for clearer behavior.

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

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds an early-return in FrmEntry::is_duplicate when item_meta is absent after the frm_duplicate_check_val filter, implements a new PHPCS sniff MoveSimpleCheckBeforeExpensiveCallSniff to detect-and-fix cheap-check-before-expensive-call patterns, and registers that sniff in the project's ruleset.

Changes

Cohort / File(s)	Summary
Duplicate check change classes/models/FrmEntry.php	Adds an early return when $values['item_meta'] is not set after applying the frm_duplicate_check_val filter; simplifies final duplicate-existence logic to rely solely on whether matching entries exist (removes requirement for item_meta in the final conditional).
PHPCS sniff implementation phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/MoveSimpleCheckBeforeExpensiveCallSniff.php	New sniff class that detects if conditions combining simple checks (e.g., empty(), isset(), is_null()) with expensive calls assigned to a variable immediately prior, emits a fixable error, and auto-fixes by splitting into a cheap-check early-return followed by the expensive-call branch while preserving assignment and indentation.
Sniff registration phpcs-sniffs/Formidable/ruleset.xml	Registers the new Formidable.CodeAnalysis.MoveSimpleCheckBeforeExpensiveCall rule in the PHPCS ruleset.

Sequence Diagram(s)

(Skipped — changes do not introduce a multi-component runtime flow suitable for sequence diagrams.)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• New sniff to remove redundant empty on loop vars #2840: Modifies FrmEntry::is_duplicate duplicate-check control flow and item_meta handling — strong overlap with the duplicate-check changes here.
• Add new sniff to remove wpdb prefix from FrmDb functions #2794: Also touches FrmEntry::is_duplicate (duplicate lookup arguments / metadata handling) and may conflict or complement the adjustments in this PR.

Poem

🐰 I nibbled at code both tidy and grand,

Moved quick checks forward by careful hand.
If meta is missing, I hop away fast,
Sniffs tidy the burrow — no slow calls to last.
🥕 Hooray for clean hops in the code-land!

🚥 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 describes the main change: a new PHP_CodeSniffer sniff that detects and flags opportunities to move simple checks before expensive function calls in 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

---

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

[coderabbitai]

Actionable comments posted: 5

🤖 Fix all issues with AI agents

In `@classes/models/FrmEntry.php`:
- Around line 90-93: There is an extra/missing blank line around the new
early-return after applying filters which triggers the
blank_line_before_statement rule; adjust spacing so there is exactly one blank
line before the "if ( ! isset( $values['item_meta'] ) ) { return; }" statement
(or run the project's PHP CS fixer) immediately after the apply_filters call in
the same block where $check_val is set and the early-return is added; ensure the
apply_filters($check_val) line and the if ( ! isset( $values['item_meta'] ) )
return; follow the repository's blank-line conventions.
- Around line 90-99: The is_duplicate method currently has two early exits that
use bare returns (return;) which yield null; change both early exits to
explicitly return false so the function matches its `@return` bool signature and
works with strict comparisons—specifically update the branches that check for
missing $values['item_meta'] and for !$entry_exists in FrmEntry::is_duplicate
(references: $values['item_meta'] check and $entry_exists from FrmDb::get_col)
to return false instead of a bare return.

In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/MoveSimpleCheckBeforeExpensiveCallSniff.php`:
- Around line 366-455: The phpdoc union types in this file are ordered
incorrectly and trip the phpdoc_types_order rule; update the `@return` annotations
so falses come first (e.g., change "string|false" to "false|string" on
extractVariableName(), change "array|false" to "false|array" on
findPrecedingAssignment(), and likewise any "int|false" to "false|int" in the
following docblock for findStatementStart()), or run the project's phpdoc fixer
to apply the same ordering across the file.
- Around line 98-127: isEarlyReturnPattern currently treats any "return <expr>;"
as an early-return but applyFix always inserts "return;" which can change
behavior; update the logic so you either (A) restrict isEarlyReturnPattern to
only match bare returns by checking that the token immediately after T_RETURN is
a semicolon (i.e. confirm no expression tokens between T_RETURN and the
semicolon) before returning true, or (B) preserve the return expression by, in
applyFix, extracting the tokens between the T_RETURN and the semicolon inside
the matched scope and re-inserting that exact expression when rebuilding the
moved early-return; refer to isEarlyReturnPattern and applyFix to locate the
checks and the insertion logic and implement one of these two safer approaches.

🧹 Nitpick comments (2)

phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/MoveSimpleCheckBeforeExpensiveCallSniff.php (2)

180-182: Remove unused $tokens in checkForOptimization.

PHPMD flags $tokens as unused. It can be dropped safely.

♻️ Suggested cleanup

-		$tokens = $phpcsFile->getTokens();
-

---

229-256: Drop unused parameters/variables in applyFix.

$conditionOpener, $conditionCloser, and $scopeOpener aren’t used. Removing them quiets PHPMD and simplifies the signature.

♻️ Suggested cleanup

-			$this->applyFix( $phpcsFile, $stackPtr, $assignmentData, $simpleCheckPart, $variableCheckPart, $conditionOpener, $conditionCloser );
+			$this->applyFix( $phpcsFile, $stackPtr, $assignmentData, $simpleCheckPart, $variableCheckPart );
@@
-	private function applyFix( File $phpcsFile, $stackPtr, $assignmentData, $simpleCheckPart, $variableCheckPart, $conditionOpener, $conditionCloser ) {
+	private function applyFix( File $phpcsFile, $stackPtr, $assignmentData, $simpleCheckPart, $variableCheckPart ) {
@@
-		$scopeOpener = $tokens[ $stackPtr ]['scope_opener'];
 		$scopeCloser = $tokens[ $stackPtr ]['scope_closer'];