Skip to content

Update empty on param sniff to catch all params#2838

Merged
Crabcyborg merged 2 commits into
masterfrom
update_empty_on_param_sniff_to_catch_all_params
Jan 20, 2026
Merged

Update empty on param sniff to catch all params#2838
Crabcyborg merged 2 commits into
masterfrom
update_empty_on_param_sniff_to_catch_all_params

Conversation

@Crabcyborg
Copy link
Copy Markdown
Contributor

@Crabcyborg Crabcyborg commented Jan 20, 2026

Summary by CodeRabbit

  • Refactor

    • Simplified and standardized conditional checks across multiple areas to make truthiness handling more consistent and reduce subtle branching differences.
  • Chores

    • Improved static analysis logic to better detect complex condition patterns and reduce false positives in code-quality checks.

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 20, 2026

📝 Walkthrough

Walkthrough

Replaced several empty() checks with general PHP truthiness checks across controllers and models, and refactored a PHPCS sniff to track parenthesis depth when detecting empty() inside if/elseif conditions.

Changes

Cohort / File(s) Summary
Addon / License checks
classes/controllers/FrmAddonsController.php, classes/models/FrmFormApi.php
Changed if ( ! $download_id && ! empty( $addons ) )if ( ! $download_id && $addons ), relying on PHP truthiness of $addons instead of empty() specifically.
Core truthiness/guard updates
classes/models/FrmDb.php, classes/models/FrmForm.php, stripe/models/FrmStrpLiteAuth.php, classes/models/FrmEntryValidate.php, classes/models/fields/FrmFieldType.php
Replaced empty(...) checks with general falsiness/truthiness checks (! $query, ! $global_load, ! $intents, $errors, $type) altering when branches execute by accepting broader falsy values (null/false/0/'' etc.).
PHPCS sniff (analysis)
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnParameterSniff.php
Rewrote isInIfCondition() to track parentheses depth, expanded allowed token types within conditions, and added early exits to more reliably detect empty() used directly in if/elseif conditions.

Sequence Diagram(s)

(omitted — changes are not a new multi-component feature nor a substantial control-flow interaction that benefits from a sequence diagram)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped through code with a twitchy nose,
Swapped empty() for truth where the boolean grows.
Parens counted, sniff refined with care,
I nibbled through branches and left them fair.
A little rabbit cheer for cleaner air 🌿

🚥 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 empty on param sniff to catch all params' directly reflects the main change: expanding the RedundantEmptyOnParameterSniff to detect empty() usage more broadly in conditional contexts.
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

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 20, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.04%. Comparing base (44874d3) to head (e6d1ed9).
⚠️ Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
classes/controllers/FrmAddonsController.php 0.00% 1 Missing ⚠️
classes/models/FrmFormApi.php 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2838      +/-   ##
============================================
- Coverage     27.05%   27.04%   -0.01%     
  Complexity     8857     8857              
============================================
  Files           145      145              
  Lines         29817    29812       -5     
============================================
- Hits           8066     8063       -3     
+ Misses        21751    21749       -2     

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnParameterSniff.php (1)

240-309: Nested parentheses prevent detection in calls like if ( foo( empty( $param ) ) ).

The backward scan stops at the first unmatched open parenthesis and immediately returns false if it doesn't belong to if/elseif, never checking any parent parentheses. For if ( foo( empty( $param ) ) ), the scan hits empty( first and fails.

Use nested_parenthesis to check all enclosing parentheses instead of stopping at the first one.

🔧 Suggested fix (use nested_parenthesis)
 private function isInIfCondition( File $phpcsFile, $stackPtr ) {
 	$tokens = $phpcsFile->getTokens();
 
-	// Track parenthesis nesting to find the outermost condition parenthesis.
-	$parenDepth = 0;
-
-	for ( $i = $stackPtr - 1; $i >= 0; $i-- ) {
-		$code = $tokens[ $i ]['code'];
-
-		// Track parenthesis nesting.
-		if ( $code === T_CLOSE_PARENTHESIS ) {
-			++$parenDepth;
-			continue;
-		}
-
-		if ( $code === T_OPEN_PARENTHESIS ) {
-			if ( $parenDepth > 0 ) {
-				--$parenDepth;
-				continue;
-			}
-
-			// This is an unmatched open paren - check if it belongs to if/elseif.
-			$beforeParen = $phpcsFile->findPrevious( T_WHITESPACE, $i - 1, null, true );
-
-			if ( false !== $beforeParen ) {
-				$beforeCode = $tokens[ $beforeParen ]['code'];
-
-				if ( $beforeCode === T_IF || $beforeCode === T_ELSEIF ) {
-					return true;
-				}
-			}
-
-			// This parenthesis doesn't belong to if/elseif.
-			return false;
-		}
-
-		// Skip tokens that are valid inside an if condition.
-		if ( $code === T_WHITESPACE
-			|| $code === T_BOOLEAN_NOT
-			|| $code === T_BOOLEAN_AND
-			|| $code === T_BOOLEAN_OR
-			|| $code === T_LOGICAL_AND
-			|| $code === T_LOGICAL_OR
-			|| $code === T_VARIABLE
-			|| $code === T_STRING
-			|| $code === T_LNUMBER
-			|| $code === T_DNUMBER
-			|| $code === T_CONSTANT_ENCAPSED_STRING
-			|| $code === T_TRUE
-			|| $code === T_FALSE
-			|| $code === T_NULL
-			|| $code === T_ISSET
-			|| $code === T_EMPTY
-			|| $code === T_IS_EQUAL
-			|| $code === T_IS_NOT_EQUAL
-			|| $code === T_IS_IDENTICAL
-			|| $code === T_IS_NOT_IDENTICAL
-			|| $code === T_GREATER_THAN
-			|| $code === T_LESS_THAN
-			|| $code === T_IS_GREATER_OR_EQUAL
-			|| $code === T_IS_SMALLER_OR_EQUAL
-			|| $code === T_OBJECT_OPERATOR
-			|| $code === T_NULLSAFE_OBJECT_OPERATOR
-			|| $code === T_OPEN_SQUARE_BRACKET
-			|| $code === T_CLOSE_SQUARE_BRACKET
-			|| $code === T_DOUBLE_COLON
-		) {
-			continue;
-		}
-
-		// Hit something unexpected, stop searching.
-		return false;
-	}
+	$nested = $tokens[ $stackPtr ]['nested_parenthesis'] ?? array();
+	if ( empty( $nested ) ) {
+		return false;
+	}
+
+	foreach ( array_reverse( array_keys( $nested ) ) as $openParen ) {
+		$beforeParen = $phpcsFile->findPrevious( T_WHITESPACE, $openParen - 1, null, true );
+		if ( false !== $beforeParen && in_array( $tokens[ $beforeParen ]['code'], array( T_IF, T_ELSEIF ), true ) ) {
+			return true;
+		}
+	}
 
 	return false;
 }

@Crabcyborg Crabcyborg merged commit 465cd9b into master Jan 20, 2026
18 checks passed
@Crabcyborg Crabcyborg deleted the update_empty_on_param_sniff_to_catch_all_params branch January 20, 2026 01:18
stephywells pushed a commit that referenced this pull request Apr 4, 2026
…to_catch_all_params

Update empty on param sniff to catch all params
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant