Enforce more early exits#2982
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR refactors conditional control flow across many files, replacing nested positive conditionals with inverted guard clauses (early return/continue) while preserving existing behaviors and signatures. Changes span controllers, helpers, models, extension code, and tests. 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
🧪 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 |
DeepSource Code ReviewWe reviewed changes in See full review on DeepSource ↗ Code Review Summary
|
 |
|
Overall Grade Focus Area: Hygiene |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP |  |
Feb 26, 2026 6:10p.m. | Review ↗ |
| JavaScript |  |
Feb 26, 2026 6:10p.m. | Review ↗ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
classes/controllers/FrmEntriesController.php (1)
560-560: Remove the unused destructured variable inuser_hidden_columns_for_form().Line 560 assigns
$field_keybut never uses it, which matches the PHPMD warning and adds avoidable noise.♻️ Proposed cleanup
- list( $form_prefix, $field_key ) = explode( '_', $r ); + list( $form_prefix ) = explode( '_', $r );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/controllers/FrmEntriesController.php` at line 560, In user_hidden_columns_for_form() the destructured assignment list( $form_prefix, $field_key ) = explode( '_', $r ); introduces an unused $field_key; remove the unused variable by changing the destructuring to only capture $form_prefix (e.g. use list($form_prefix) = explode('_', $r') or the short array syntax) so the function no longer declares the unused $field_key and the PHPMD warning is resolved.classes/helpers/FrmAppHelper.php (1)
2367-2369: Remove the unused loop value variable.Line 2367 declares
$frm_role_descriptionbut never uses it. This is also flagged by PHPMD.Suggested cleanup
- foreach ( $frm_roles as $frm_role => $frm_role_description ) { + foreach ( array_keys( $frm_roles ) as $frm_role ) { $role->add_cap( $frm_role ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/helpers/FrmAppHelper.php` around lines 2367 - 2369, The foreach loop in FrmAppHelper.php declares an unused $frm_role_description variable; update the loop that iterates over $frm_roles (the block that calls $role->add_cap($frm_role)) to remove the unused value variable (e.g., iterate only with $frm_role) so PHPMD warnings are resolved and the loop remains functionally identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@classes/helpers/FrmCSVExportHelper.php`:
- Around line 627-628: The current logic blindly replaces a non-array meta at
$entries[self::$entry->parent_item_id]->metas[$repeater_child->id] with an empty
array; instead, when that meta exists but is not an array, preserve its scalar
value by converting it into an array containing the original scalar (so it isn't
lost) before adding any placeholder/blank entries. Update the check around
$entries[self::$entry->parent_item_id]->metas[$repeater_child->id] to only
initialize to array() if it is unset, and if it exists and !is_array(...) wrap
the existing value into an array containing that value so the original data is
preserved.
---
Nitpick comments:
In `@classes/controllers/FrmEntriesController.php`:
- Line 560: In user_hidden_columns_for_form() the destructured assignment list(
$form_prefix, $field_key ) = explode( '_', $r ); introduces an unused
$field_key; remove the unused variable by changing the destructuring to only
capture $form_prefix (e.g. use list($form_prefix) = explode('_', $r') or the
short array syntax) so the function no longer declares the unused $field_key and
the PHPMD warning is resolved.
In `@classes/helpers/FrmAppHelper.php`:
- Around line 2367-2369: The foreach loop in FrmAppHelper.php declares an unused
$frm_role_description variable; update the loop that iterates over $frm_roles
(the block that calls $role->add_cap($frm_role)) to remove the unused value
variable (e.g., iterate only with $frm_role) so PHPMD warnings are resolved and
the loop remains functionally identical.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
classes/controllers/FrmAddonsController.phpclasses/controllers/FrmAppController.phpclasses/controllers/FrmDashboardController.phpclasses/controllers/FrmEntriesController.phpclasses/controllers/FrmFormActionsController.phpclasses/controllers/FrmFormTemplatesController.phpclasses/controllers/FrmFormsController.phpclasses/controllers/FrmInboxController.phpclasses/controllers/FrmSettingsController.phpclasses/helpers/FrmAppHelper.phpclasses/helpers/FrmCSVExportHelper.phpclasses/helpers/FrmFieldsHelper.phpclasses/helpers/FrmXMLHelper.phpclasses/models/FrmAddon.phpclasses/models/FrmAntiSpam.phpclasses/models/FrmCreateFile.phpclasses/models/FrmDb.phpclasses/models/FrmEntryFormatter.phpclasses/models/FrmEntryValidate.phpclasses/models/FrmField.phpclasses/models/FrmFieldFormHtml.phpclasses/models/FrmFieldValue.phpclasses/models/FrmForm.phpclasses/models/FrmFormAction.phpclasses/models/FrmInbox.phpclasses/models/FrmMigrate.phpclasses/models/FrmPluginSearch.phpclasses/models/FrmSettings.phpclasses/models/FrmTableHTMLGenerator.phpclasses/models/FrmUsage.phpclasses/models/fields/FrmFieldCombo.phpclasses/models/fields/FrmFieldNumber.phpclasses/models/fields/FrmFieldType.phpphpcs.xmlstripe/models/FrmStrpLiteSettings.phpstripe/models/FrmTransLiteDb.phptests/phpunit/test_wordpress_plugin_tests.php
💤 Files with no reviewable changes (1)
- phpcs.xml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stripe/helpers/FrmTransLiteAppHelper.php`:
- Around line 392-394: The guard clause currently uses a loose falsy check on
$value which hides legitimate 0 and '0' values; update the guard in
FrmTransLiteAppHelper.php to only return for truly absent values (e.g. null or
empty string) rather than any falsy value so calls like
show_in_table($subscription->fail_count, ...) will display 0; locate the block
using the $value variable and replace the loose check with a strict
existence/emptiness check (only return when $value === null or $value === '') so
numeric zero and the string '0' remain visible.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
classes/helpers/FrmListHelper.phpstripe/helpers/FrmTransLiteAppHelper.php
✅ Files skipped from review due to trivial changes (1)
- classes/helpers/FrmListHelper.php
Comment on lines
+392
to
394
| if ( ! $value ) { | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n stripe/helpers/FrmTransLiteAppHelper.php | sed -n '380,410p'Repository: Strategy11/formidable-forms
Length of output: 896
🏁 Script executed:
rg -nP --type=php -C5 'show_in_table' stripe/Repository: Strategy11/formidable-forms
Length of output: 2314
🏁 Script executed:
rg -nP --type=php -B10 -A15 'function show_in_table' stripe/Repository: Strategy11/formidable-forms
Length of output: 1710
Guard clause suppresses legitimate 0 and '0' values.
The condition if ( ! $value ) at line 392 treats 0 and '0' as falsy, causing valid zero values to be hidden. This affects calls like show_in_table( $subscription->fail_count, ... ) at stripe/views/subscriptions/show.php:78, where a "Fail Count" of 0 is meaningful data that should display.
Proposed fix
- if ( ! $value ) {
+ if ( '' === $value || null === $value ) {
return;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stripe/helpers/FrmTransLiteAppHelper.php` around lines 392 - 394, The guard
clause currently uses a loose falsy check on $value which hides legitimate 0 and
'0' values; update the guard in FrmTransLiteAppHelper.php to only return for
truly absent values (e.g. null or empty string) rather than any falsy value so
calls like show_in_table($subscription->fail_count, ...) will display 0; locate
the block using the $value variable and replace the loose check with a strict
existence/emptiness check (only return when $value === null or $value === '') so
numeric zero and the string '0' remain visible.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit