Add new sniff to simplify count equals 0 checks#2844
Conversation
📝 WalkthroughWalkthroughA new PHP_CodeSniffer sniff is introduced to enforce empty array comparisons using Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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/PreferEmptyArrayComparisonSniff.php`:
- Around line 45-51: The sniff currently treats any T_STRING with content
'count' as the global count() function and can false-positive on method calls
like $obj->count(); update the process(File $phpcsFile, $stackPtr) logic to
ensure the token at $stackPtr is a standalone function call by verifying the
previous meaningful token is not an object operator or scope resolution operator
(e.g., T_OBJECT_OPERATOR '->' or T_DOUBLE_COLON '::')—use
$phpcsFile->findPrevious or inspect $tokens[$prev]['code'] against those token
types (and skip empty tokens) before proceeding to treat it as count().
🧹 Nitpick comments (1)
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferEmptyArrayComparisonSniff.php (1)
217-227: Remove unused parameter and add changeset wrapper for consistency.The
$closeParenparameter is unused (as flagged by static analysis). Additionally, for consistency withfixZeroEqualsCount(), consider wrapping the fixer operations in a changeset.♻️ Proposed fix
- private function fixCountEqualsZero( File $phpcsFile, $countStart, $closeParen, $zeroToken, $varContent, $isStrict ) { + private function fixCountEqualsZero( File $phpcsFile, $countStart, $zeroToken, $varContent, $isStrict ) { $fixer = $phpcsFile->fixer; $comparison = $isStrict ? '===' : '=='; // Replace from count to 0 with $var === array(). + $fixer->beginChangeset(); + for ( $i = $countStart; $i <= $zeroToken; $i++ ) { $fixer->replaceToken( $i, '' ); } $fixer->addContent( $countStart, $varContent . ' ' . $comparison . ' array()' ); + + $fixer->endChangeset(); }Also update the call site at line 103:
- $this->fixCountEqualsZero( $phpcsFile, $stackPtr, $closeParen, $zeroToken, $varContent, $isStrict ); + $this->fixCountEqualsZero( $phpcsFile, $stackPtr, $zeroToken, $varContent, $isStrict );
…unt_equals_0_checks Add new sniff to simplify count equals 0 checks
If the compared arrays are large, calling
countcan be more expensive, though it looks like it's fairly negligible.