New sniff to prefer positive ternaries#2864
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe PR introduces a new PHP_CodeSniffer sniff that detects and automatically fixes negated ternary conditions by suggesting a positive form. The sniff is then systematically applied across the codebase, flipping multiple instances of negated ternary operators and conditional logic to improve code readability. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@classes/helpers/FrmAppHelper.php`:
- Line 1395: The ternary branches were reversed when the negation was removed
for computing $class using str_contains($icon, ' '): locate the assignment that
sets $class (uses str_contains and $icon in FrmAppHelper.php) and swap the
ternary outcomes so icons that do NOT contain a space get the prefixed class
(i.e., preserve the original behavior by returning ' ' . $icon for the no-space
case and '' for the space-containing case); alternatively, restore the original
logical sense by reinserting the negation on str_contains and keep the current
branch ordering.
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferPositiveTernaryConditionSniff.php`:
- Around line 1-10: Add the missing blank line before the first statement
following the file-level docblock in the
Formidable_Sniffs_CodeAnalysis_PreferPositiveTernaryConditionSniff file: ensure
there's a blank line between the closing */ of the header comment and the next
statement (e.g., the class declaration or any php statement that follows) or
simply run php-cs-fixer fix to apply the blank_line_before_statement rule
automatically for the
Formidable_Sniffs_CodeAnalysis_PreferPositiveTernaryConditionSniff code.
- Line 473: The method applyFix currently declares an unused parameter
$conditionStart; remove $conditionStart from the applyFix signature so it
becomes applyFix(File $phpcsFile, $notToken, $ternaryOperator, $colonOperator,
$ternaryEnd, $thenContent, $elseContent) and update every call site that passes
$conditionStart to stop supplying that argument (ensure calls to applyFix match
the new parameter list). Also update any related docblocks or PHPDoc for
applyFix to reflect the removed parameter.
In `@stripe/views/action-settings/payments-options.php`:
- Around line 103-106: The visibility logic for gateways in the foreach loop is
inverted causing all gateways to be hidden for one-time payments; update the
construction of $gateway_classes (in the loop over $gateways where
$gateway_name/$gateway and $gateway_id are used) so that 'frm_hidden' is only
appended when the form action type is 'recurring' AND the gateway does NOT
support recurring payments (i.e. check $form_action->post_content['type'] ===
'recurring' && !$gateway['recurring']), leaving it visible for one-time
payments.
🧹 Nitpick comments (2)
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferPositiveTernaryConditionSniff.php (2)
316-316: Avoid callingcount()in loop condition.Calling
count($tokens)on every iteration is inefficient. Cache the count before the loop.Proposed fix
+ $tokenCount = count( $tokens ); - for ( $i = $start; $i < count( $tokens ); $i++ ) { + for ( $i = $start; $i < $tokenCount; $i++ ) {The same issue exists at line 372 in
findTernaryEnd.
376-390: Redundant condition checks inside parenthesis tracking.Inside the
if ( $code === T_OPEN_PARENTHESIS )block (line 376), the check$tokens[ $i ]['code'] !== T_WHITESPACEat line 378 is always true because$codeisT_OPEN_PARENTHESIS. Similarly for lines 384-390.Proposed fix
if ( $code === T_OPEN_PARENTHESIS ) { ++$parenDepth; - if ( $tokens[ $i ]['code'] !== T_WHITESPACE ) { - $end = $i; - } + $end = $i; continue; } if ( $code === T_CLOSE_PARENTHESIS ) { if ( $parenDepth > 0 ) { --$parenDepth; - if ( $tokens[ $i ]['code'] !== T_WHITESPACE ) { - $end = $i; - } + $end = $i; continue; }
…_ternaries New sniff to prefer positive ternaries
1 participant
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.