Apply slevomat early exits#2948
Conversation
📝 WalkthroughWalkthroughThis PR consistently converts nested conditionals into early-return/guard clauses across controllers, helpers, models, integrations, tests, and config—reordering checks so methods exit immediately when prerequisites are missing and proceed only when required conditions are met. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
🧪 Generate unit tests (beta)
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 |
|
Here's the code health analysis summary for commits Analysis Summary
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
classes/models/FrmEntryValidate.php (1)
281-281:⚠️ Potential issue | 🟡 MinorPipeline failure:
trim()receivesmixed|numericinstead ofstring.Psalm flags
$current_valueasmixed|numericbecause it comes from iterating over(array) $value(line 263), whose elements aren't guaranteed to be strings. This also affects line 287 and the call on line 293.Casting to string at the start of the inner loop iteration would resolve this across all downstream
trim()calls:Proposed fix
foreach ( $value as $current_value ) { + $current_value = (string) $current_value; $match = false;tests/phpunit/base/FrmUnitTest.php (1)
453-468:⚠️ Potential issue | 🟡 MinorReset
$frm_varseven whenFrmProEddControlleris absent.
The early return skips clearing plugin globals, which can leak state between tests if the class isn’t loaded.🔧 Suggested fix
- if ( ! class_exists( 'FrmProEddController' ) ) { - return; - } - - $frm_vars = array( + $frm_vars = array( 'load_css' => false, 'forms_loaded' => array(), 'created_entries' => array(), 'pro_is_authorized' => false, 'next_page' => array(), 'prev_page' => array(), ); + if ( ! class_exists( 'FrmProEddController' ) ) { + return; + } + $frmedd_update = new FrmProEddController(); $frm_vars['pro_is_authorized'] = $frmedd_update->pro_is_authorized();
🤖 Fix all issues with AI agents
In `@classes/controllers/FrmAddonsController.php`:
- Around line 665-679: Guard against missing entries in the API response by
checking isset($addon_info[$download_id]) before using it: when calling
$api->get_api_info() (addon_info) verify the download key exists prior to
assigning to $version_info[$download_id] and prior to reading
$addon_info['error']; if the key is absent, skip assignment or set a safe
default (e.g., an error entry) so that references to $version_info[$download_id]
and accesses in subsequent code (the block that sets ['error'] from
$addon_info['error']) do not trigger undefined index notices.
In `@phpcs.xml`:
- Around line 170-176: The PHPCS rule block for
SlevomatCodingStandard.ControlStructures.EarlyExit contains an unsupported
property; remove the property named ignoreTrailingIfWithOneInstruction and leave
only the two valid properties ignoreStandaloneIfInScope and
ignoreOneLineTrailingIf within the <rule
ref="SlevomatCodingStandard.ControlStructures.EarlyExit"> block to match the
supported sniff configuration.
🧹 Nitpick comments (2)
classes/models/FrmAddon.php (1)
346-353: Early return is correct; minor nit on the redundant reset.The guard clause correctly mirrors the original nested logic. One observation: Line 353 (
$this->should_clear_cache = true) is only reachable when$this->should_clear_cacheis alreadytrue, making the assignment a no-op. Consider removing it or, if the intent is to reset state after an external caller sets itfalse, note that the early return on Line 346 prevents that reset from ever firing in that scenario.This is pre-existing, not introduced by this PR — just flagging since you're touching this block.
classes/models/fields/FrmFieldName.php (1)
231-233:$hidden_fieldis unused in the foreach loop.Static analysis flags
$hidden_fieldas unused — only$nameis referenced in the loop body. This is pre-existing (not introduced by this PR), but since you're touching these lines, consider using$_or just the key.♻️ Optional fix
- foreach ( $hidden_fields as $name => $hidden_field ) { + foreach ( array_keys( $hidden_fields ) as $name ) { $args['sub_fields'][ $name ]['wrapper_classes'] .= ' frm_hidden'; }
Apply slevomat early exits
1 participant
Summary by CodeRabbit