Update redundant empty on param sniff to catch boolean assignemnts an…#2858
Conversation
📝 WalkthroughWalkthroughThis PR simplifies conditional logic across multiple helper and model files by replacing explicit Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnParameterSniff.php`:
- Around line 54-56: The current boolean-context guard (call to
isInBooleanContext in RedundantEmptyOnParameterSniff) skips cases like
assignments and ternaries (e.g., "$flag = empty($param);" or "empty($param) ?
'a' : 'b'"); update isInBooleanContext to also detect assignment operators ('=')
and ternary tokens ('?' / ':') as valid boolean contexts (or remove the guard
entirely to always check parameter-only empty() usage), then adjust the earlier
return that uses isInBooleanContext to allow these cases; apply the same change
to the other occurrences in the file (the logic used between the ranges
referenced around lines 229-369) so assignments and ternaries are flagged too.
| // Only process if empty() is in a boolean context (if/elseif condition or with || / &&). | ||
| if ( ! $this->isInBooleanContext( $phpcsFile, $stackPtr ) ) { | ||
| return; |
There was a problem hiding this comment.
Boolean-context gate still misses plain assignments/ternaries.
The new logic only returns true for if/elseif or when empty() is adjacent to &&/||. Cases like $flag = empty( $param ); or empty( $param ) ? 'yes' : 'no' won’t be flagged, which conflicts with the PR intent to catch boolean assignments/ternaries. Consider broadening isInBooleanContext to cover ternary/assignment usage or dropping the boolean-context guard for parameter-only empty().
Also applies to: 229-369
🤖 Prompt for AI Agents
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnParameterSniff.php`
around lines 54 - 56, The current boolean-context guard (call to
isInBooleanContext in RedundantEmptyOnParameterSniff) skips cases like
assignments and ternaries (e.g., "$flag = empty($param);" or "empty($param) ?
'a' : 'b'"); update isInBooleanContext to also detect assignment operators ('=')
and ternary tokens ('?' / ':') as valid boolean contexts (or remove the guard
entirely to always check parameter-only empty() usage), then adjust the earlier
return that uses isInBooleanContext to allow these cases; apply the same change
to the other occurrences in the file (the logic used between the ranges
referenced around lines 229-369) so assignments and ternaries are flagged too.
…ram_sniff_to_catch_boolean_assignments Update redundant empty on param sniff to catch boolean assignemnts an…
…d ternaries
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.