Skip to content

New sniff to remove redundant empty on loop vars#2840

Merged
Crabcyborg merged 1 commit into
masterfrom
new_sniff_to_remove_redundant_empty_on_loop_vars
Jan 20, 2026
Merged

New sniff to remove redundant empty on loop vars#2840
Crabcyborg merged 1 commit into
masterfrom
new_sniff_to_remove_redundant_empty_on_loop_vars

Conversation

@Crabcyborg
Copy link
Copy Markdown
Contributor

@Crabcyborg Crabcyborg commented Jan 20, 2026

Summary by CodeRabbit

  • New Features

    • Added a code quality checker to detect and automatically fix redundant empty() validation on loop variables.
  • Bug Fixes

    • Improved handling of the string value "0" which was previously treated as blank in certain form operations and data filtering scenarios.
  • Chores

    • Updated code quality analysis rules for consistency.

✏️ 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

This PR systematically replaces empty() checks with simpler truthiness checks (!$var) across multiple source files, and introduces a new PHP_CodeSniffer sniff to automatically detect and fix redundant empty() calls on foreach loop variables.

Changes

Cohort / File(s) Summary
Source code truthiness checks
classes/controllers/FrmEntriesController.php, classes/controllers/FrmXMLController.php, classes/helpers/FrmListHelper.php, classes/models/FrmEntry.php, classes/models/FrmInbox.php, classes/models/fields/FrmFieldCombo.php, classes/views/frm-fields/back-end/layout-classes.php, tests/phpunit/entries/test_FrmShowEntryShortcode.php
Replaced empty() checks with direct truthiness checks (!$var). Note: this treats values like the string "0" as truthy, which may have different behavior from prior empty() calls.
PHP_CodeSniffer sniff
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnLoopVariableSniff.php
New sniff implementation (+298 lines) that detects redundant empty() calls on foreach loop variables and auto-fixes them. Includes token-level AST traversal, loop variable detection, reassignment tracking, and whitespace-aware transformations.
Sniffs ruleset
phpcs-sniffs/Formidable/ruleset.xml
Removed RedundantParentheses rule; added RedundantEmptyOnLoopVariable and RedundantIntermediateVariable CodeAnalysis rules.

Possibly related PRs

Poem

🐰 A rabbit hops through empty() calls so deep,
Replacing them with truthiness so sweet,
A sniff that catches loops from end to start,
Making the code cleaner, pure art!

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 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: introducing a new PHP_CodeSniffer sniff that detects and removes redundant empty() calls on foreach loop variables.
Docstring Coverage ✅ Passed Docstring coverage is 92.86% 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.

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: 1

🤖 Fix all issues with AI agents
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnLoopVariableSniff.php`:
- Around line 186-207: The current wasVariableReassigned function only detects
simple T_EQUAL and misses compound assignments and increments/decrements; update
it to also treat compound-assignment tokens (e.g. T_PLUS_EQUAL, T_MINUS_EQUAL,
T_MUL_EQUAL, T_DIV_EQUAL, T_CONCAT_EQUAL, T_MOD_EQUAL, T_POW_EQUAL, T_AND_EQUAL,
T_OR_EQUAL, T_XOR_EQUAL, T_SL_EQUAL, T_SR_EQUAL) as reassignment when they
appear immediately after the variable, and detect increment/decrement tokens
(T_INC, T_DEC) when they appear immediately before or after the variable;
implement this by examining the next non-whitespace token for any
assignment/compound-assignment token and the previous/next non-whitespace token
for T_INC/T_DEC (and treat =&/reference-style assignments similarly if present)
inside wasVariableReassigned to return true for these cases.
🧹 Nitpick comments (1)
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnLoopVariableSniff.php (1)

220-220: Unused parameter $variableName in applyFix method.

The $variableName parameter is passed but never used. The method independently locates the variable token using findNext. Consider removing this parameter to clean up the API.

Proposed fix

Update the method signature and call site:

-private function applyFix( File $phpcsFile, $emptyToken, $openParen, $closeParen, $variableName ) {
+private function applyFix( File $phpcsFile, $emptyToken, $openParen, $closeParen ) {

And at line 110:

-$this->applyFix( $phpcsFile, $stackPtr, $openParen, $closeParen, $variableName );
+$this->applyFix( $phpcsFile, $stackPtr, $openParen, $closeParen );

@Crabcyborg Crabcyborg merged commit 70b3765 into master Jan 20, 2026
37 of 38 checks passed
@Crabcyborg Crabcyborg deleted the new_sniff_to_remove_redundant_empty_on_loop_vars branch January 20, 2026 02:05
stephywells pushed a commit that referenced this pull request Apr 4, 2026
…t_empty_on_loop_vars

New sniff to remove redundant empty on loop vars
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