New rector rule for useless variables#2665
Conversation
WalkthroughThis PR applies widespread micro-refactors replacing assignment-then-return patterns with direct inline returns across controllers, helpers, models, and integration code; it also re-enabled Rector's SimplifyUselessVariableRector in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas that may need extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
classes/models/FrmFormState.php (1)
178-187: Use CBC or GCM mode instead of AES-128-ECB for stronger encryption.ECB has a critical weakness: identical plaintext blocks produce identical ciphertext blocks, revealing patterns in the encrypted data, and ECB should never be used for encrypting anything beyond single blocks of random data. NIST's guidance states that the use of ECB to encrypt confidential information constitutes a severe security vulnerability.
Current code (lines 186 and 141) uses AES-128-ECB. CBC (Cipher Block Chaining) addresses ECB's weakness by XORing each plaintext block with the previous ciphertext block before encryption, creating a chain effect where each block depends on all previous blocks, and requires an initialization vector (IV) — a random value used to encrypt the first block.
Example migration to CBC mode:
private function get_state_string() { if ( ! self::open_ssl_is_installed() ) { return ''; } $secret = self::get_encryption_secret(); $compressed_state = $this->compressed_state(); $json_encoded = json_encode( $compressed_state ); $iv = openssl_random_pseudo_bytes( openssl_cipher_iv_length( 'AES-128-CBC' ) ); $encrypted = openssl_encrypt( $json_encoded, 'AES-128-CBC', $secret, 0, $iv ); return base64_encode( $iv . $encrypted ); }Update
get_state_from_request()(line 141) to decrypt using CBC mode and extract the IV.Note: This is a pre-existing issue, not introduced by this PR.
🧹 Nitpick comments (2)
classes/helpers/FrmListHelper.php (1)
502-522: LGTM! Clean refactoring that simplifies the return statement.The inline concatenation correctly preserves the original behavior while removing the intermediate variable assignment, consistent with the PR's objective.
Optional observation: The parentheses around the button HTML string are unnecessary since the concatenation operator has appropriate precedence. However, they're harmless and may improve clarity.
If you prefer, you could simplify to:
- return $out . ( '<button type="button" class="toggle-row"><span class="screen-reader-text">' . __( 'Show more details', 'formidable' ) . '</span></button>' ); + return $out . '<button type="button" class="toggle-row"><span class="screen-reader-text">' . __( 'Show more details', 'formidable' ) . '</span></button>';square/controllers/FrmSquareLiteAppController.php (1)
124-126: LGTM! Clean refactoring that eliminates the intermediate variable.The direct return of
prepare_amount()correctly removes the useless intermediate reassignment to$amount, making the code more concise while preserving functionality.Optional refinement: The comment on line 123 could be updated to something like "Calculate amount based on field shortcodes" since we're no longer updating the
$amountvariable in place, but this is a minor nitpick.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (56)
classes/controllers/FrmAddonsController.php(1 hunks)classes/controllers/FrmEmailStylesController.php(2 hunks)classes/controllers/FrmFieldsController.php(1 hunks)classes/controllers/FrmFormActionsController.php(1 hunks)classes/controllers/FrmFormsController.php(3 hunks)classes/controllers/FrmSMTPController.php(1 hunks)classes/controllers/FrmStylesController.php(3 hunks)classes/controllers/FrmXMLController.php(2 hunks)classes/helpers/FrmAppHelper.php(1 hunks)classes/helpers/FrmCSVExportHelper.php(3 hunks)classes/helpers/FrmEntriesHelper.php(1 hunks)classes/helpers/FrmFieldsHelper.php(10 hunks)classes/helpers/FrmFormsHelper.php(8 hunks)classes/helpers/FrmFormsListHelper.php(1 hunks)classes/helpers/FrmListHelper.php(1 hunks)classes/helpers/FrmStylesHelper.php(4 hunks)classes/helpers/FrmTipsHelper.php(5 hunks)classes/helpers/FrmXMLHelper.php(1 hunks)classes/models/FrmAntiSpam.php(1 hunks)classes/models/FrmDb.php(3 hunks)classes/models/FrmEmail.php(2 hunks)classes/models/FrmEmailSummary.php(1 hunks)classes/models/FrmEntry.php(9 hunks)classes/models/FrmEntryFormatter.php(1 hunks)classes/models/FrmEntryMeta.php(1 hunks)classes/models/FrmEntryShortcodeFormatter.php(2 hunks)classes/models/FrmEntryValidate.php(3 hunks)classes/models/FrmField.php(1 hunks)classes/models/FrmFieldFormHtml.php(1 hunks)classes/models/FrmForm.php(6 hunks)classes/models/FrmFormAction.php(3 hunks)classes/models/FrmFormApi.php(1 hunks)classes/models/FrmFormState.php(1 hunks)classes/models/FrmHoneypot.php(1 hunks)classes/models/FrmPersonalData.php(1 hunks)classes/models/FrmStyleApi.php(1 hunks)classes/models/fields/FrmFieldCaptcha.php(1 hunks)classes/models/fields/FrmFieldCombo.php(2 hunks)classes/models/fields/FrmFieldCreditCard.php(1 hunks)classes/models/fields/FrmFieldSubmit.php(2 hunks)classes/models/fields/FrmFieldType.php(3 hunks)rector.php(0 hunks)square/controllers/FrmSquareLiteActionsController.php(2 hunks)square/controllers/FrmSquareLiteAppController.php(1 hunks)stripe/controllers/FrmStrpLiteActionsController.php(2 hunks)stripe/controllers/FrmStrpLiteEventsController.php(2 hunks)stripe/controllers/FrmStrpLitePaymentsController.php(1 hunks)stripe/controllers/FrmTransLiteActionsController.php(3 hunks)stripe/helpers/FrmStrpLiteConnectHelper.php(3 hunks)stripe/helpers/FrmStrpLiteSubscriptionHelper.php(2 hunks)stripe/helpers/FrmTransLiteAppHelper.php(3 hunks)stripe/helpers/FrmTransLiteListHelper.php(2 hunks)stripe/models/FrmStrpLiteAuth.php(1 hunks)stripe/models/FrmTransLiteAction.php(1 hunks)stripe/models/FrmTransLitePayment.php(1 hunks)stripe/models/FrmTransLiteSubscription.php(1 hunks)
💤 Files with no reviewable changes (1)
- rector.php
🧰 Additional context used
🧬 Code graph analysis (12)
stripe/helpers/FrmTransLiteListHelper.php (1)
classes/helpers/FrmFormsHelper.php (2)
FrmFormsHelper(6-2214)edit_form_link(1285-1294)
classes/controllers/FrmEmailStylesController.php (1)
classes/models/FrmEntryShortcodeFormatter.php (1)
content(167-181)
classes/models/FrmEntryShortcodeFormatter.php (1)
classes/models/FrmTableHTMLGenerator.php (1)
generate_table_footer(360-362)
classes/models/FrmAntiSpam.php (1)
classes/models/FrmFormState.php (1)
get(86-88)
classes/helpers/FrmStylesHelper.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5083)pro_is_installed(399-401)
classes/models/fields/FrmFieldCaptcha.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5083)array_to_html_params(1525-1534)
stripe/controllers/FrmStrpLiteEventsController.php (2)
stripe/models/FrmTransLiteDb.php (1)
get_one(167-177)stripe/helpers/FrmTransLiteAppHelper.php (2)
FrmTransLiteAppHelper(6-632)count_completed_payments(539-549)
stripe/helpers/FrmStrpLiteSubscriptionHelper.php (1)
stripe/helpers/FrmStrpLiteAppHelper.php (2)
FrmStrpLiteAppHelper(6-161)call_stripe_helper_class(40-45)
stripe/controllers/FrmTransLiteActionsController.php (1)
classes/models/FrmField.php (2)
FrmField(6-1638)create(383-441)
stripe/models/FrmTransLiteAction.php (1)
classes/models/FrmField.php (2)
FrmField(6-1638)getAll(1068-1121)
stripe/helpers/FrmStrpLiteConnectHelper.php (1)
square/helpers/FrmSquareLiteConnectHelper.php (1)
post_with_authenticated_body(534-555)
classes/controllers/FrmStylesController.php (1)
classes/models/FrmStyle.php (4)
get_one(488-516)FrmStyle(6-868)get_default_style(598-608)get_all(525-591)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: Cypress
🔇 Additional comments (77)
classes/models/FrmAntiSpam.php (1)
177-179: Excellent simplification of the return path.Removing the intermediate variable and returning the concatenated string directly makes the intent clearer without changing behavior. The parentheses effectively communicate the precedence and operator grouping. This change aligns well with the
SimplifyUselessVariableRectorrefactoring.classes/models/FrmHoneypot.php (1)
273-275: Solid simplification.Removing the intermediate variable assignment while keeping
$classfor readability is the right balance. The change preserves identical behavior while reducing unnecessary local variable scope.classes/helpers/FrmFieldsHelper.php (1)
316-320: Refactoring consistently applies SimplifyUselessVariableRector across multiple methods.All ten changes systematically remove intermediate variables in return statements by inlining computed results. Each refactor preserves behavior, maintains readability, and avoids unnecessary variable assignments. The changes are applied consistently across helper and utility methods (e.g.,
default_unique_msg,allowed_shortcodes,get_error_msg) without altering public APIs or method signatures.Also applies to: 327-331, 372-379, 479-484, 495-509, 1020-1029, 1061-1086, 1454-1469, 1773-1786, 1858-1869
square/controllers/FrmSquareLiteActionsController.php (2)
190-206: Excellent refactoring: direct return of object creation.Removing the intermediate
$payment_idvariable improves clarity while preserving the method's return type contract (int). The refactoring is safe and aligns with the project-wide simplification effort.
309-330: Excellent refactoring: direct return of object creation.Removing the intermediate variable assignment and returning
$frm_payment->create( $new_values )directly is a clean simplification. The return type contract (int) remains intact and behavior is unchanged.classes/models/FrmStyleApi.php (1)
40-43: LGTM! Clean refactoring that improves readability.The removal of the intermediate variable assignment makes the code more concise while maintaining identical behavior. The method now directly returns the result of
fill_missing_style_settings, which is clearer and aligns with the PR's objective of eliminating useless variables.classes/models/FrmPersonalData.php (1)
164-164: LGTM! Clean refactor.The direct return eliminates an unnecessary intermediate variable assignment while preserving the same functionality. This aligns perfectly with the PR objective of simplifying useless variables.
classes/helpers/FrmFormsListHelper.php (1)
358-358: LGTM! Clean refactoring.The direct return eliminates the unnecessary intermediate variable assignment while maintaining the same functionality.
classes/models/FrmEntryMeta.php (1)
303-303: LGTM!The refactoring correctly eliminates an intermediate variable and directly returns the result. This makes both return paths in the function consistent (line 283 already uses this pattern).
classes/models/FrmEmailSummary.php (1)
106-106: LGTM! Clean refactoring that removes unnecessary intermediate variable.The direct inline return is functionally equivalent to the previous assignment-plus-return pattern and improves code clarity by eliminating the redundant variable.
classes/models/FrmEntryFormatter.php (1)
957-961: LGTM! Clean refactoring to remove useless intermediate variable.The change correctly eliminates the intermediate assignment by returning the result of
strip_html()directly. The logic and behavior remain identical.stripe/helpers/FrmTransLiteListHelper.php (2)
434-440: Direct return is correct and behavior-preserving.The refactoring correctly removes the intermediate variable and maintains identical behavior. The return type remains consistent across both branches (string).
459-467: Direct inline return is appropriate and maintains identical behavior.The refactoring correctly inlines the computed date values directly into the return expression without changing the output. Both intermediate variables are used once, making them candidates for removal per the Rector rule.
classes/models/FrmEntryShortcodeFormatter.php (2)
216-221: LGTM! Clean refactoring of return statement.The direct return with concatenation eliminates the intermediate append operation while maintaining identical behavior. This aligns perfectly with the PR's objective to remove useless intermediate steps.
249-255: LGTM! Textbook example of removing a useless intermediate variable.The direct return eliminates the unnecessary
$rowvariable that was simply storing the return value before immediately returning it. This is exactly the type of simplification the Rector rule targets.classes/models/FrmFormAction.php (3)
257-262: LGTM! Clean refactoring.The final field name suffix is correctly inlined into the return statement, eliminating an intermediate variable without changing behavior.
902-907: LGTM! Filter application correctly inlined.The filter result is now returned directly, removing the intermediate variable assignment without affecting the filter's behavior.
964-969: LGTM! Direct return simplifies the fallback logic.The intermediate
$stopvariable has been correctly eliminated, withfalsenow returned directly when the Pro controller method is not callable.classes/models/FrmFieldFormHtml.php (1)
320-320: LGTM!The direct return simplifies the code while preserving the exact behavior. The substring extraction logic correctly extracts the body between the
[if error]and[/if error]tags.classes/controllers/FrmXMLController.php (2)
152-159: LGTM! Clean refactor.The direct return of
json_decode()eliminates an unnecessary intermediate variable without changing functionality.
599-610: LGTM! Simplified return statement.The direct return of
wp_list_pluck()removes an unnecessary intermediate variable. The$parent_slugsvariable on line 601 is still appropriately used for the early return guard.classes/models/FrmEntryValidate.php (3)
365-372: LGTM! Clean refactor of callback return.The direct return of
do_shortcode( $option_value )is more concise than an intermediate assignment. The callback logic is preserved correctly.
488-488: LGTM! Cleaner pattern delimiter wrapping.The direct return of the delimited pattern is more concise. The pattern is correctly wrapped for
preg_matchusage.
535-535: LGTM! Concise pattern anchoring.The direct return with anchors
^and$is cleaner than an intermediate assignment. The regex pattern construction logic is preserved correctly.classes/helpers/FrmCSVExportHelper.php (3)
122-122: LGTM! Clean simplification.The direct return eliminates an unnecessary intermediate variable assignment while preserving the same functionality.
363-370: LGTM! Clean simplification.The direct return eliminates an unnecessary intermediate variable assignment while preserving the same functionality.
888-888: LGTM! Clean simplification.The direct return eliminates an unnecessary intermediate variable assignment while preserving the same functionality.
classes/helpers/FrmXMLHelper.php (1)
390-401: LGTM! Clean refactoring.The direct return of
$old_fieldseliminates an unnecessary intermediate variable assignment, improving code clarity without changing behavior.stripe/helpers/FrmStrpLiteSubscriptionHelper.php (2)
74-74: LGTM! Clean refactoring.Directly returning the result of
$frm_sub->create()eliminates the unnecessary intermediate variable without any functional impact.
196-196: LGTM! Clean refactoring.Directly returning the result of
call_stripe_helper_class()eliminates the unnecessary intermediate variable without any functional impact.classes/models/fields/FrmFieldCreditCard.php (1)
30-35: LGTM! Clean refactor.The direct return of the array is more concise and maintains identical behavior.
classes/models/fields/FrmFieldSubmit.php (2)
46-50: LGTM! Direct heredoc return is idiomatic.Returning the heredoc string directly is clean and maintains the same output.
59-67: LGTM! Consistent with other field types.The direct array return matches the refactoring pattern applied across other field classes.
classes/models/fields/FrmFieldCombo.php (2)
228-235: LGTM! Consistent refactoring.Direct array return aligns with the refactoring pattern applied to other field type classes.
289-289: LGTM! Clean simplification.Returning
ob_get_clean()directly eliminates the unnecessary intermediate variable while maintaining the same behavior.classes/models/fields/FrmFieldType.php (3)
218-227: LGTM! Consistent with subclass refactors.Returning the heredoc directly matches the pattern applied in
FrmFieldSubmit::default_html()and maintains the same output.
1463-1463: LGTM! Concise return statement.Combining the closing tag concatenation with the return statement is clean and maintains identical behavior.
1677-1677: LGTM! Direct boolean return.Returning the
in_array()result directly is more concise and maintains the same logic.stripe/models/FrmStrpLiteAuth.php (1)
64-67: LGTM! Clean refactoring to eliminate intermediate mutation.The change correctly transforms the filter to return the concatenated result directly instead of mutating the
$messageparameter. This aligns with the PR's objective of removing useless variables and produces more functional, side-effect-free code.classes/models/FrmFormState.php (1)
186-186: LGTM! Refactoring aligns with PR objectives.The direct return eliminates the intermediate variable while preserving the encryption behavior.
stripe/controllers/FrmStrpLitePaymentsController.php (1)
28-30: LGTM! Clean refactoring.The direct return eliminates the intermediate mutation step while preserving the same output. This aligns with the PR's objective to simplify useless variable assignments.
classes/controllers/FrmFieldsController.php (1)
706-708: LGTM! Simplified return.The direct return of the null-coalescing expression is cleaner and maintains identical behavior.
classes/helpers/FrmEntriesHelper.php (1)
956-956: LGTM! Direct return simplifies the code.The inline return preserves the same array merging behavior while eliminating the unnecessary intermediate variable.
classes/controllers/FrmAddonsController.php (1)
1249-1258: LGTM! Inline array return.The direct return of the array literal is cleaner and maintains the exact same response structure.
classes/models/FrmEmail.php (2)
224-231: LGTM! Cleaner return flow.The direct return eliminates the redundant reassignment while maintaining the same formatting behavior.
647-660: LGTM! Simplified return.The inline return of
str_replaceis more concise and preserves identical behavior.classes/models/FrmFormApi.php (1)
297-318: LGTM! Direct return of decoded value.The inline return eliminates the intermediate variable assignment while preserving the exact same JSON decoding behavior.
classes/models/fields/FrmFieldCaptcha.php (1)
99-124: LGTM! Inline HTML return.The direct return of the constructed HTML string is cleaner and maintains identical output.
classes/controllers/FrmSMTPController.php (1)
82-85: LGTM! Simplified link transformation.The direct return of the
str_replaceresult eliminates the unnecessary intermediate variable while preserving the exact same link modification behavior.classes/models/FrmDb.php (1)
260-260: LGTM! Clean refactor to remove intermediate variables.The changes eliminate temporary variables that were only used for return statements in
get_var(),generate_cache_key(), andget_associative_array_results(). The logic remains identical.Also applies to: 285-285, 475-475
stripe/models/FrmTransLitePayment.php (1)
15-68: LGTM! Simplified return in get_defaults().The method now directly returns the array instead of assigning it to an intermediate
$valuesvariable. The returned data structure is unchanged.stripe/models/FrmTransLiteSubscription.php (1)
15-76: LGTM! Simplified return in get_defaults().The method now directly returns the array instead of assigning it to an intermediate
$valuesvariable. The returned data structure is unchanged.classes/controllers/FrmFormActionsController.php (1)
796-796: LGTM! Direct return of concatenated string.The change eliminates the intermediate concatenation step by returning
$where . $wpdb->prepare(...)directly. The result is identical to the previous two-step approach.classes/controllers/FrmStylesController.php (3)
478-478: LGTM! Simplified return in get_default_style().The method now directly returns
$frm_style->get_one()instead of assigning it to$default_stylefirst.
890-893: LGTM! Simplified return in get_custom_css().The method now directly returns
$style->post_content['custom_css']instead of assigning it to an intermediate$custom_cssvariable.
1217-1217: LGTM! Simplified return in get_style_opts().The method now directly returns
$frm_style->get_all()instead of assigning it to$stylesfirst.stripe/helpers/FrmTransLiteAppHelper.php (1)
162-162: LGTM! Clean refactor removing intermediate variables.The changes in
get_action_setting(),process_shortcodes(), andget_gateways()eliminate temporary variables that were only used for return statements. The logic remains identical.Also applies to: 215-215, 555-555
classes/models/FrmField.php (1)
1309-1311: LGTM! Simplified boolean return.The method now directly returns the boolean expression instead of assigning it to an intermediate
$is_multi_value_fieldvariable. The logic is unchanged.classes/controllers/FrmEmailStylesController.php (2)
216-216: LGTM! Inline string concatenation in return.The closing HTML tags (
</head><body>...</body></html>) are now concatenated directly in the return statement instead of being appended to$wrapped_contentfirst. The output is identical.
450-450: LGTM! Inline string concatenation in return.The closing
</div>tags are now concatenated directly in the return statement instead of being appended to$new_messagefirst. The output is identical.stripe/helpers/FrmStrpLiteConnectHelper.php (3)
771-774: refund_payment boolean return simplification looks correct
refund_paymentstill returns a simplebool(is_object( $data )), with only the unnecessary intermediate variable removed. No behavior change.
801-805: cancel_subscription return expression is safely inlinedReturning
false !== $datadirectly preserves the previous truthiness check while dropping the redundant$successvariable.
892-895: update_intent success check remains unchangedThe method still treats any non‑
falseresponse as success; only the extra variable was removed. Behavior and return type are intact.classes/helpers/FrmAppHelper.php (1)
3259-3309: human_time_diff cleanup is purely cosmeticInline
implode( ' ', $time_strings )after slicing is equivalent to the prior$time_ago_stringvariable; hooks and diff logic are unaffected.stripe/controllers/FrmStrpLiteEventsController.php (2)
230-268: prepare_from_invoice now returns the fetched payment directlySwitching to
return $frm_payment->get_one( $payment_id );after the update/create logic keeps the same return value while dropping an unused local. No behavioral change.
334-339: get_payments_count refactor is behavior‑preservingDirectly returning
FrmTransLiteAppHelper::count_completed_payments( $all_payments )is equivalent to the previous$countvariable usage.classes/controllers/FrmFormsController.php (3)
897-926: bulk_trash message construction unchanged aside from removing a tempThe function still returns the correctly localized trash message with an Undo bulk‑untrash link; only
$messagewas inlined into thereturn.
1793-1814: get_advanced_shortcodes direct filter return is fineReturning
apply_filters( 'frm_advanced_shortcodes', $adv_shortcodes )directly is equivalent to the previous assign‑then‑return pattern and keeps the filter contract intact.
1936-1960: filter_content: inline filtered content return keeps semanticsYou still:
- Replace
[form_name],- Resolve
$entry,- Derive
$form/parent form,- Compute
$shortcodes,- And pass everything through
frm_replace_content_shortcodes.Returning the filter result directly instead of via
$contentis a no‑op behavior‑wise.classes/models/FrmForm.php (1)
498-508: Direct-return refactors in sanitize_calc, getName, get_key_by_id, get_published_forms, get_current_form_id, and show_submit look behaviorally identicalAll these changes simply inline the previously returned value into the
returnstatement without altering conditions or data flow. No functional, type, or side-effect differences detected.Also applies to: 801-806, 835-844, 978-987, 1192-1200, 1313-1317
classes/helpers/FrmStylesHelper.php (1)
158-181: Icon class, brightness, and style editor wrapper changes preserve existing behavior
icon_key_to_class: still builds the same base class, conditionally appends$key, then'_icon'.get_color_brightness: same RGB extraction and luminance formula, now returned directly.style_editor_get_wrapper_classname: resulting classname (includingfrm_hiddenand conditionalfrm-pro) is identical; only construction is streamlined.No regressions or edge-case behavior changes apparent.
Also applies to: 418-441, 1094-1100
classes/helpers/FrmTipsHelper.php (1)
120-174: Tip array builders now return directly without temps; arrays are unchangedEach of
get_builder_tip,get_form_settings_tip,get_form_action_tip,get_styling_tip,get_entries_tip, andget_import_tipstill returns the same arrays (or merged arrays), only without intermediate$tipsvariables. No functional impact.Also applies to: 176-206, 208-302, 304-334, 339-368, 371-384
classes/helpers/FrmFormsHelper.php (1)
32-36: FrmFormsHelper direct-return cleanups retain original semantics
- URL, HTML, and shortcode builders now return their strings directly.
get_form_style_classstill derives the same$classand passes it intofrm_add_form_style_class; returning the filter result directly is equivalent to assigning then returning.get_form_style’s ternary return mirrors the prior pattern of conditionally overriding$styleand then returning it.edit_form_link,status_nice_name, andget_plan_requiredstill output the same values.No logic, filter, or type changes detected.
Also applies to: 317-341, 539-541, 1011-1043, 1125-1146, 1285-1294, 1548-1560, 1746-1758
classes/models/FrmEntry.php (1)
22-24: Entry create/update/destroy helpers now use direct returns; behavior is preservedRefactors in
create,create_entry,update,destroy,exists,before_insert_entry_in_database,package_entry_to_update,create_entry_from_xml, andupdate_entry_from_xmlonly remove temporary variables:
- Control flow (including early-exit on missing entry in
destroy) is the same.- Filter hooks (
frm_pre_create_entry,frm_update_entry, etc.) are still invoked with identical arguments.- Return values (
bool|int) are unchanged.Looks good.
Also applies to: 34-43, 366-375, 600-616, 792-801, 1194-1222, 1267-1269, 1282-1284
stripe/controllers/FrmStrpLiteActionsController.php (1)
315-332: Stripe Lite action save and plan ID refactors are no-op behavior-wise
before_save_settings: for Stripe-enabled actions, still lowercases currency, normalizes gateway array, forcesstripe_link = 1, then passes throughcreate_plans; only temp assignment removed.create_plan_id: plan ID string structure is identical; direct concatenation in the return has no semantic change.Safe refactors.
Also applies to: 364-367
stripe/models/FrmTransLiteAction.php (1)
115-137: get_field_options direct-return is equivalentThe method still:
- Normalizes
$form_id,- Runs the
frm_trans_action_get_field_options_form_idfilter,- Queries
FrmField::getAllwith the same where/order arguments.Only the intermediate
$form_fieldsvariable was removed. No behavioral changes.stripe/controllers/FrmTransLiteActionsController.php (1)
158-167: Payment error, amount parsing, and auto-field creation refactors keep logic identical
replace_success_messagestill uses the same error source and default text, just inlines the<div>wrapper in the return.get_amount_from_string’s regex and selection of the last match are unchanged; the ternary on$matcheshas the same truthiness behavior as the oldifblock.add_a_fieldstill prepares new field values and delegates toFrmField::create; only the temporary$field_idvariable was removed.No functional differences detected.
Also applies to: 358-363, 649-653
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
classes/helpers/FrmListHelper.php (1)
521-521: LGTM! Improved escaping and simplified return.The refactoring correctly simplifies the return pattern by using inline concatenation, and the switch from
__()toesc_html__()is a best practice improvement for HTML output.Optional nitpick: The outer parentheses around the button HTML string are unnecessary and could be removed for cleaner code:
-return $out . ( '<button type="button" class="toggle-row"><span class="screen-reader-text">' . esc_html__( 'Show more details', 'formidable' ) . '</span></button>' ); +return $out . '<button type="button" class="toggle-row"><span class="screen-reader-text">' . esc_html__( 'Show more details', 'formidable' ) . '</span></button>';
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
classes/controllers/FrmStylesController.php(3 hunks)classes/helpers/FrmListHelper.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: Cypress
🔇 Additional comments (3)
classes/controllers/FrmStylesController.php (3)
476-479: LGTM! Clean refactor.The direct return eliminates an unnecessary intermediate variable while maintaining the same behavior. The refactor aligns well with the PR's objective.
1214-1217: LGTM! Clean refactor.The direct return eliminates an unnecessary intermediate variable while maintaining the same behavior. The refactor aligns well with the PR's objective.
890-893: Add null safety guard before accessing get_default_style() result.The refactor correctly simplifies the code, but directly accessing
$style->post_content['custom_css']at line 893 without verifying thatget_default_style()returns a non-null value risks a fatal error. Although other code assumes default styles always exist, consider adding a null check or document why a null return is impossible here.
…variables New rector rule for useless variables
1 participant
No description provided.