Add a new sniff for grouping simple assignments#2760
Conversation
|
Warning Rate limit exceeded@Crabcyborg has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 14 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (6)
WalkthroughThis PR introduces a new PHP_CodeSniffer sniff to enforce consistent spacing in consecutive variable assignments and applies this formatting pattern across the entire codebase, while also including minor functional fixes in email migration, form action triggering, and response structures. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The diff is predominantly homogeneous (same formatting pattern repeated across ~100 files), which lowers review complexity. However, the new 477-line PHP_CodeSniffer sniff class requires careful logic review, and scattered functional changes in FrmAddonsController, FrmMigrate.php, FrmFieldType.php, and FrmTransLiteActionsController demand attention to verify correctness and backward compatibility. Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
classes/controllers/FrmSettingsController.php (1)
48-58: Remove unused$target_pathvariable. (Line 50)The template file does not reference
$target_pathor$uploads. Since$target_pathis not used anywhere, remove the unused variable assignment on line 50:$uploads = wp_upload_dir(); $target_path = $uploads['basedir'] . '/formidable/css'; // Remove this lineThe
$sectionsand$currentvariables are both correctly used in the template.
🤖 Fix all issues with AI agents
In @classes/controllers/FrmEntriesController.php:
- Around line 948-949: Remove the unused variable $id initialized to 0 in the
FrmEntriesController code block (it’s later reassigned from $entry->id but never
used); either delete the "$id = 0" initialization or, if $id is meant for
future/external use, add a brief comment explaining its purpose so its presence
is justified—ensure the include that uses $data remains unchanged.
In @classes/views/shared/toggle.php:
- Around line 17-25: The $checked computation can call str_contains on a
non-string and throw a TypeError; update the $checked assignment to first test
that $args['checked'] is strictly true OR is a string that contains
'checked="checked"'. In other words, change the logic that sets $checked
(currently referencing $args['checked'] and str_contains) to guard with
is_string($args['checked']) before calling str_contains (or cast to string
safely), so $checked becomes: isset($args['checked']) && ( true ===
$args['checked'] || (is_string($args['checked']) &&
str_contains($args['checked'], 'checked="checked"')) ).
🧹 Nitpick comments (4)
classes/views/frm-fields/front-end/radio-field.php (1)
51-53: Whitespace-only alignment change looks safe.No behavior change here;
$checkedremains a controlled string derived fromFrmAppHelper::check_selected(...)and is safe to echo without escaping given it’s not user input.
Optional: consider making$checkedeither''or' checked="checked"'(no embedded spacing) to avoid accidental double-spaces when concatenating later.stripe/helpers/FrmStrpLiteConnectApiAdapter.php (1)
74-75: LGTM! Formatting alignment improves readability.These changes add alignment spacing to consecutive variable assignments at the beginning of the method, consistent with the PR's objective to standardize spacing. The logic and functionality remain unchanged.
Optional consistency check:
Line 76 (
$customer_id_error_message = '';) is also a consecutive assignment but wasn't aligned with lines 74-75. You may want to verify whether this is intentional (e.g., grouping by assignment complexity) or if it should be aligned for consistency.classes/views/shared/mb_adv_info.php (1)
240-243: Avoid overwriting theforeachkey variable ($code) for readability.Possible refactor
-foreach ( $helper['codes'] as $code => $code_label ) { - $code = isset( $uid ) ? str_replace( '|user_id|', $uid, $code ) : str_replace( '|user_id|', 'x', $code ); - $include_x = strpos( $code, ' ' ) ? '' : 'x '; +foreach ( $helper['codes'] as $code_template => $code_label ) { + $code = isset( $uid ) ? str_replace( '|user_id|', $uid, $code_template ) : str_replace( '|user_id|', 'x', $code_template ); + $include_x = strpos( $code, ' ' ) ? '' : 'x ';classes/helpers/FrmAppHelper.php (1)
2461-2467: Preferarray_filloverprepare_array_values+explodefor building$function. (Line 2464)
Current code works, but this is simpler and avoids string churn.Proposed diff
- $original_function = $function; - $function = count( $value ) ? explode( ', ', FrmDb::prepare_array_values( $value, $function ) ) : array( $function ); + $original_function = $function; + $function = array_fill( 0, count( $value ), $function );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
phpcs.xmlis excluded by!**/*.xml
📒 Files selected for processing (108)
classes/controllers/FrmAddonsController.phpclasses/controllers/FrmAppController.phpclasses/controllers/FrmApplicationsController.phpclasses/controllers/FrmDashboardController.phpclasses/controllers/FrmEmailStylesController.phpclasses/controllers/FrmEntriesController.phpclasses/controllers/FrmFieldsController.phpclasses/controllers/FrmFormActionsController.phpclasses/controllers/FrmFormsController.phpclasses/controllers/FrmHooksController.phpclasses/controllers/FrmInboxController.phpclasses/controllers/FrmSMTPController.phpclasses/controllers/FrmSettingsController.phpclasses/controllers/FrmStylesController.phpclasses/controllers/FrmXMLController.phpclasses/helpers/FrmAppHelper.phpclasses/helpers/FrmCSVExportHelper.phpclasses/helpers/FrmDashboardHelper.phpclasses/helpers/FrmEmailSummaryHelper.phpclasses/helpers/FrmEntriesHelper.phpclasses/helpers/FrmEntriesListHelper.phpclasses/helpers/FrmFieldsHelper.phpclasses/helpers/FrmFormsHelper.phpclasses/helpers/FrmFormsListHelper.phpclasses/helpers/FrmListHelper.phpclasses/helpers/FrmOnSubmitHelper.phpclasses/helpers/FrmStylesCardHelper.phpclasses/helpers/FrmStylesPreviewHelper.phpclasses/helpers/FrmSubmitHelper.phpclasses/helpers/FrmXMLHelper.phpclasses/models/FrmCreateFile.phpclasses/models/FrmDb.phpclasses/models/FrmEmail.phpclasses/models/FrmEmailStats.phpclasses/models/FrmEmailSummary.phpclasses/models/FrmEntry.phpclasses/models/FrmEntryFormatter.phpclasses/models/FrmEntryMeta.phpclasses/models/FrmEntryShortcodeFormatter.phpclasses/models/FrmEntryValidate.phpclasses/models/FrmField.phpclasses/models/FrmFieldFormHtml.phpclasses/models/FrmForm.phpclasses/models/FrmFormAction.phpclasses/models/FrmFormMigrator.phpclasses/models/FrmHoneypot.phpclasses/models/FrmMigrate.phpclasses/models/FrmNotification.phpclasses/models/FrmPersonalData.phpclasses/models/FrmPluginSearch.phpclasses/models/FrmReviews.phpclasses/models/FrmSolution.phpclasses/models/FrmSpamCheckStopForumSpam.phpclasses/models/FrmStyle.phpclasses/models/FrmTableHTMLGenerator.phpclasses/models/FrmUsage.phpclasses/models/fields/FrmFieldCaptcha.phpclasses/models/fields/FrmFieldCombo.phpclasses/models/fields/FrmFieldName.phpclasses/models/fields/FrmFieldType.phpclasses/views/frm-entries/_sidebar-shared-pub.phpclasses/views/frm-fields/back-end/radio-field.phpclasses/views/frm-fields/front-end/dropdown-field.phpclasses/views/frm-fields/front-end/radio-field.phpclasses/views/frm-form-actions/form_action.phpclasses/views/shared/mb_adv_info.phpclasses/views/shared/toggle.phpcss/_single_theme.css.phpphpcs-sniffs/Formidable/Sniffs/WhiteSpace/ConsecutiveAssignmentSpacingSniff.phpsquare/controllers/FrmSquareLiteActionsController.phpsquare/controllers/FrmSquareLiteAppController.phpsquare/controllers/FrmSquareLiteEventsController.phpstripe/controllers/FrmStrpLiteActionsController.phpstripe/controllers/FrmStrpLiteEventsController.phpstripe/controllers/FrmStrpLiteLinkController.phpstripe/controllers/FrmStrpLitePaymentsController.phpstripe/controllers/FrmTransLiteActionsController.phpstripe/controllers/FrmTransLiteAppController.phpstripe/controllers/FrmTransLiteListsController.phpstripe/controllers/FrmTransLitePaymentsController.phpstripe/helpers/FrmStrpLiteConnectApiAdapter.phpstripe/helpers/FrmTransLiteListHelper.phpstripe/models/FrmStrpLiteAuth.phptests/phpunit/base/FrmUnitTest.phptests/phpunit/base/frm_factory.phptests/phpunit/database/test_FrmMigrate.phptests/phpunit/emails/test_FrmEmail.phptests/phpunit/emails/test_FrmEmailStylesController.phptests/phpunit/entries/test_FrmEntriesController.phptests/phpunit/entries/test_FrmEntryFormatter.phptests/phpunit/entries/test_FrmPersonalData.phptests/phpunit/entries/test_FrmShowEntryShortcode.phptests/phpunit/entries/test_FrmTableHTMLGenerator.phptests/phpunit/fields/test_FrmFieldName.phptests/phpunit/fields/test_FrmFieldType.phptests/phpunit/fields/test_FrmFieldValidate.phptests/phpunit/fields/test_FrmFieldsController.phptests/phpunit/fields/test_FrmFieldsHelper.phptests/phpunit/forms/test_FrmForm.phptests/phpunit/forms/test_FrmFormsController.phptests/phpunit/forms/test_FrmFormsControllerAjax.phptests/phpunit/misc/test_FrmAppHelper.phptests/phpunit/misc/test_FrmCSVExportHelper.phptests/phpunit/settings/test_FrmSettings.phptests/phpunit/stripe/test_FrmTransLiteActionsController.phptests/phpunit/test_ajax.phptests/phpunit/views/shortcodes/test_FrmFieldShortcodes.phptests/phpunit/xml/test_FrmXMLHelper.php
💤 Files with no reviewable changes (2)
- stripe/models/FrmStrpLiteAuth.php
- classes/views/frm-entries/_sidebar-shared-pub.php
🧰 Additional context used
🧬 Code graph analysis (42)
classes/helpers/FrmListHelper.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5049)get_server_value(631-633)
stripe/controllers/FrmStrpLiteActionsController.php (2)
square/controllers/FrmSquareLiteActionsController.php (1)
prepare_amount(433-437)stripe/controllers/FrmTransLiteActionsController.php (1)
prepare_amount(301-325)
classes/helpers/FrmSubmitHelper.php (1)
classes/helpers/FrmFieldsHelper.php (2)
FrmFieldsHelper(6-2805)setup_new_vars(21-54)
classes/controllers/FrmAppController.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5049)simple_get(807-816)
stripe/controllers/FrmStrpLiteLinkController.php (2)
stripe/models/FrmTransLiteDb.php (1)
get_one_by(186-204)stripe/helpers/FrmStrpLiteAppHelper.php (2)
FrmStrpLiteAppHelper(6-162)call_stripe_helper_class(40-45)
classes/models/fields/FrmFieldCombo.php (1)
classes/helpers/FrmFieldsHelper.php (2)
FrmFieldsHelper(6-2805)get_error_msg(454-483)
tests/phpunit/entries/test_FrmTableHTMLGenerator.php (1)
tests/phpunit/base/FrmUnitTest.php (1)
get_private_property(668-671)
tests/phpunit/base/FrmUnitTest.php (3)
classes/controllers/FrmXMLController.php (1)
form(314-341)classes/models/FrmField.php (3)
get_id_by_key(1529-1533)FrmField(6-1625)get_all_for_form(953-995)classes/models/FrmForm.php (1)
get_id_by_key(813-815)
stripe/helpers/FrmTransLiteListHelper.php (1)
classes/helpers/FrmEntriesListHelper.php (1)
get_form_ids(194-196)
classes/helpers/FrmEntriesHelper.php (2)
classes/helpers/FrmShortcodeHelper.php (2)
FrmShortcodeHelper(9-157)get_shortcode_attribute_array(20-32)classes/controllers/FrmEntriesController.php (2)
FrmEntriesController(6-973)show_entry_shortcode(894-940)
classes/controllers/FrmFormActionsController.php (3)
classes/models/FrmFormAction.php (2)
form(95-99)prepare_new(293-321)classes/models/FrmForm.php (2)
FrmForm(6-1407)getOne(856-884)classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5049)get_param(737-774)
classes/views/shared/toggle.php (3)
classes/helpers/FrmAppHelper.php (1)
checked(2433-2437)classes/controllers/FrmFieldsController.php (1)
input_html(528-553)classes/models/fields/FrmFieldType.php (1)
input_html(230-232)
classes/controllers/FrmEntriesController.php (1)
classes/helpers/FrmListHelper.php (1)
get_pagenum(561-569)
stripe/controllers/FrmTransLiteActionsController.php (1)
classes/models/FrmEntry.php (2)
FrmEntry(6-1284)getOne(478-506)
classes/views/frm-fields/front-end/radio-field.php (1)
classes/helpers/FrmAppHelper.php (3)
checked(2433-2437)FrmAppHelper(6-5049)check_selected(2445-2452)
stripe/controllers/FrmTransLitePaymentsController.php (1)
classes/helpers/FrmAppHelper.php (1)
FrmAppHelper(6-5049)
classes/models/FrmPersonalData.php (1)
classes/models/FrmEntry.php (2)
FrmEntry(6-1284)getOne(478-506)
tests/phpunit/emails/test_FrmEmail.php (3)
classes/models/FrmField.php (2)
FrmField(6-1625)get_id_by_key(1529-1533)classes/models/FrmEntry.php (2)
get_id_by_key(1263-1267)FrmEntry(6-1284)classes/models/FrmForm.php (1)
get_id_by_key(813-815)
classes/helpers/FrmFormsHelper.php (1)
classes/helpers/FrmFieldsHelper.php (2)
FrmFieldsHelper(6-2805)get_shortcodes(1026-1043)
stripe/helpers/FrmStrpLiteConnectApiAdapter.php (1)
stripe/helpers/FrmStrpLiteAppHelper.php (2)
FrmStrpLiteAppHelper(6-162)get_customer_id_meta_name(69-77)
tests/phpunit/entries/test_FrmShowEntryShortcode.php (1)
classes/models/FrmField.php (2)
FrmField(6-1625)get_all_for_form(953-995)
tests/phpunit/fields/test_FrmFieldValidate.php (2)
tests/phpunit/base/frm_factory.php (1)
generate_entry_array(103-117)classes/models/FrmEntryValidate.php (2)
FrmEntryValidate(6-1150)validate(21-71)
classes/views/frm-fields/back-end/radio-field.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5049)check_selected(2445-2452)
tests/phpunit/forms/test_FrmFormsControllerAjax.php (1)
classes/models/FrmForm.php (1)
FrmForm(6-1407)
tests/phpunit/forms/test_FrmFormsController.php (1)
classes/models/FrmForm.php (2)
FrmForm(6-1407)getOne(856-884)
classes/models/FrmSpamCheckStopForumSpam.php (1)
classes/models/FrmFormApi.php (1)
api_url(230-236)
classes/models/fields/FrmFieldName.php (1)
classes/models/fields/FrmFieldCombo.php (1)
get_sub_field_input_attrs(523-540)
tests/phpunit/fields/test_FrmFieldName.php (1)
classes/models/fields/FrmFieldName.php (1)
FrmFieldName(14-362)
tests/phpunit/settings/test_FrmSettings.php (1)
classes/models/FrmSettings.php (1)
update_setting(642-653)
css/_single_theme.css.php (2)
classes/controllers/FrmStylesController.php (1)
important_style(1326-1330)classes/helpers/FrmStylesHelper.php (2)
FrmStylesHelper(6-1163)get_submit_image_bg_url(1109-1122)
square/controllers/FrmSquareLiteAppController.php (1)
stripe/models/FrmStrpLiteAuth.php (1)
generate_false_entry(410-434)
stripe/controllers/FrmTransLiteListsController.php (3)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5049)get_menu_name(296-300)classes/models/FrmForm.php (1)
list_page_params(1116-1133)classes/helpers/FrmListHelper.php (1)
get_pagenum(561-569)
classes/models/FrmEntryMeta.php (1)
classes/models/FrmDb.php (2)
FrmDb(6-867)get_var(248-261)
classes/models/FrmField.php (1)
classes/models/FrmDb.php (2)
FrmDb(6-867)esc_limit(602-624)
classes/controllers/FrmFieldsController.php (3)
classes/factories/FrmFieldFactory.php (2)
FrmFieldFactory(9-143)get_field_factory(42-52)classes/models/FrmFieldValueSelector.php (1)
display(253-259)classes/helpers/FrmFieldsHelper.php (2)
FrmFieldsHelper(6-2805)setup_edit_vars(72-76)
tests/phpunit/forms/test_FrmForm.php (1)
classes/models/FrmForm.php (2)
FrmForm(6-1407)duplicate(84-137)
tests/phpunit/views/shortcodes/test_FrmFieldShortcodes.php (1)
classes/models/FrmField.php (1)
FrmField(6-1625)
tests/phpunit/base/frm_factory.php (1)
classes/models/FrmForm.php (2)
FrmForm(6-1407)create(13-58)
classes/helpers/FrmAppHelper.php (1)
classes/models/FrmDb.php (2)
FrmDb(6-867)prepare_array_values(636-640)
classes/controllers/FrmSettingsController.php (3)
classes/controllers/FrmFormsController.php (1)
get_settings_tabs(1510-1627)classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5049)simple_get(807-816)classes/models/FrmDb.php (2)
FrmDb(6-867)get_results(328-330)
classes/models/FrmMigrate.php (2)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5049)plugin_path(58-60)classes/models/FrmDb.php (2)
FrmDb(6-867)get_results(328-330)
classes/models/fields/FrmFieldType.php (1)
classes/models/FrmField.php (2)
FrmField(6-1625)get_option(1479-1481)
🪛 PHPMD (2.15.0)
classes/controllers/FrmFormActionsController.php
117-117: Avoid unused local variables such as '$form'. (undefined)
(UnusedLocalVariable)
269-269: Avoid unused local variables such as '$classes'. (undefined)
(UnusedLocalVariable)
467-467: Avoid unused local variables such as '$form_action'. (undefined)
(UnusedLocalVariable)
468-468: Avoid unused local variables such as '$use_logging'. (undefined)
(UnusedLocalVariable)
470-470: Avoid unused local variables such as '$form'. (undefined)
(UnusedLocalVariable)
490-490: Avoid unused local variables such as '$form'. (undefined)
(UnusedLocalVariable)
classes/controllers/FrmEntriesController.php
949-949: Avoid unused local variables such as '$id'. (undefined)
(UnusedLocalVariable)
stripe/controllers/FrmTransLiteListsController.php
174-174: Avoid unused local variables such as '$errors'. (undefined)
(UnusedLocalVariable)
175-175: Avoid unused local variables such as '$message'. (undefined)
(UnusedLocalVariable)
classes/controllers/FrmDashboardController.php
127-127: Avoid unused local variables such as '$dashboard_view'. (undefined)
(UnusedLocalVariable)
classes/helpers/FrmFieldsHelper.php
690-690: Avoid unused local variables such as '$html_id'. (undefined)
(UnusedLocalVariable)
692-692: Avoid unused local variables such as '$options_count'. (undefined)
(UnusedLocalVariable)
733-733: Avoid unused local variables such as '$field_val'. (undefined)
(UnusedLocalVariable)
734-734: Avoid unused local variables such as '$opt'. (undefined)
(UnusedLocalVariable)
735-735: Avoid unused local variables such as '$checked'. (undefined)
(UnusedLocalVariable)
736-736: Avoid unused local variables such as '$field_name'. (undefined)
(UnusedLocalVariable)
737-737: Avoid unused local variables such as '$html_id'. (undefined)
(UnusedLocalVariable)
2509-2509: Avoid unused local variables such as '$args'. (undefined)
(UnusedLocalVariable)
classes/controllers/FrmSettingsController.php
50-50: Avoid unused local variables such as '$target_path'. (undefined)
(UnusedLocalVariable)
51-51: Avoid unused local variables such as '$sections'. (undefined)
(UnusedLocalVariable)
classes/controllers/FrmFormsController.php
1728-1728: Avoid unused local variables such as '$linked_forms'. (undefined)
(UnusedLocalVariable)
1729-1729: Avoid unused local variables such as '$col'. (undefined)
(UnusedLocalVariable)
3302-3302: Avoid unused local variables such as '$submit'. (undefined)
(UnusedLocalVariable)
3390-3390: Avoid unused local variables such as '$errors'. (undefined)
(UnusedLocalVariable)
3391-3391: Avoid unused local variables such as '$form'. (undefined)
(UnusedLocalVariable)
3392-3392: Avoid unused local variables such as '$message'. (undefined)
(UnusedLocalVariable)
⏰ 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: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP 6.9
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/helpers/FrmAppHelper.php (2)
1368-1402: Bug: deprecated icon class replacement likely doesn’t work as intended.At Lines 1385-1388,
$iconis overwritten beforestr_replace, so the old class may never be replaced in$class.Proposed fix
if ( isset( $deprecated[ $icon ] ) ) { - $icon = $deprecated[ $icon ]; - $class = str_replace( $icon, $deprecated[ $icon ], $class ); + $old_icon = $icon; + $icon = $deprecated[ $icon ]; + $class = str_replace( $old_icon, $icon, $class ); }
2460-2480: Fixrecursive_function_map()— current implementation can break non-string callables.Line 2463 now derives
$functionviaFrmDb::prepare_array_values(...)+explode(...), which will behave badly if$functionis an array/closure callable (and is unnecessarily indirect even for strings). This is a behavior change risk for this public helper.Proposed fix (safe for string + callable)
public static function recursive_function_map( $value, $function ) { if ( is_array( $value ) ) { $original_function = $function; - $function = count( $value ) ? explode( ', ', FrmDb::prepare_array_values( $value, $function ) ) : array( $function ); + $function = array_fill( 0, count( $value ), $function ); if ( ! self::is_assoc( $value ) ) { $value = array_map( array( 'FrmAppHelper', 'recursive_function_map' ), $value, $function ); } else { foreach ( $value as $k => $v ) { if ( ! is_array( $v ) ) { $value[ $k ] = call_user_func( $original_function, $v ); } } } } else { $value = self::maybe_update_value_if_null( $value, $function ); $value = call_user_func( $function, $value ); }//end if return $value; }
🤖 Fix all issues with AI agents
In
@phpcs-sniffs/Formidable/Sniffs/WhiteSpace/ConsecutiveAssignmentSpacingSniff.php:
- Around line 311-328: The private method getVariableSignature(File $phpcsFile,
$varPtr) is defined but never used; update hasDuplicateVariables() to call
getVariableSignature($phpcsFile, $varPtr) when extracting a variable's
name/signature so array element accesses are included (instead of using
$tokens[$varPtr]['content'] or similar direct token reads). Replace the code
path in hasDuplicateVariables() that builds $varName/$name from the raw token
with a call to getVariableSignature and use that signature for duplicate
comparison and mapping to ensure distinct array indices are treated as different
variables.
- Around line 212-231: hasDuplicateVariables currently compares only the base
variable name via $tokens[$varPtr]['content'], causing different keyed array
assignments to be treated as duplicates; change the code in
hasDuplicateVariables (method name) to derive the full variable signature by
calling the existing getVariableSignature(...) helper (instead of using
$tokens[$varPtr]['content']) so signatures like $data['name'] and $data['age']
are distinct while still treating true duplicates (e.g. repeated $styles[]
assignments) as duplicates; ensure you pass the same File $phpcsFile and $varPtr
into getVariableSignature and use its returned signature as the key in
$baseVariables.
🧹 Nitpick comments (1)
phpcs-sniffs/Formidable/Sniffs/WhiteSpace/ConsecutiveAssignmentSpacingSniff.php (1)
403-411: Inconsistent constant naming style.The backslash prefix on constants (
\T_EQUAL,\T_PLUS_EQUAL, etc.) is inconsistent with the rest of the file, which uses unprefixed constant names (e.g.,T_VARIABLEon line 342,T_SEMICOLONon line 437).♻️ Consistency improvement
// Must be followed by an assignment operator. $assignmentOps = array( - \T_EQUAL, - \T_PLUS_EQUAL, - \T_MINUS_EQUAL, - \T_MUL_EQUAL, - \T_DIV_EQUAL, - \T_CONCAT_EQUAL, - \T_COALESCE_EQUAL, + T_EQUAL, + T_PLUS_EQUAL, + T_MINUS_EQUAL, + T_MUL_EQUAL, + T_DIV_EQUAL, + T_CONCAT_EQUAL, + T_COALESCE_EQUAL, );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
classes/controllers/FrmAddonsController.phpclasses/controllers/FrmEmailStylesController.phpclasses/controllers/FrmEntriesController.phpclasses/controllers/FrmFieldsController.phpclasses/controllers/FrmFormsController.phpclasses/controllers/FrmSimpleBlocksController.phpclasses/helpers/FrmAppHelper.phpclasses/helpers/FrmEmailSummaryHelper.phpclasses/helpers/FrmEntriesHelper.phpclasses/helpers/FrmFormTemplatesHelper.phpclasses/helpers/FrmStylesCardHelper.phpclasses/helpers/FrmXMLHelper.phpclasses/models/FrmEntry.phpclasses/models/FrmEntryFormatter.phpclasses/models/FrmEntryMeta.phpclasses/models/FrmFormAction.phpclasses/models/FrmPersonalData.phpclasses/models/FrmStyle.phpphpcs-sniffs/Formidable/Sniffs/WhiteSpace/ConsecutiveAssignmentSpacingSniff.phpstripe/controllers/FrmTransLiteActionsController.phpstripe/helpers/FrmTransLiteListHelper.phptests/phpunit/entries/test_FrmShowEntryShortcode.phptests/phpunit/forms/test_FrmFormsController.phptests/phpunit/forms/test_FrmFormsControllerAjax.phptests/phpunit/misc/test_FrmAppHelper.phptests/phpunit/styles/test_FrmStylesController.php
✅ Files skipped from review due to trivial changes (8)
- classes/controllers/FrmSimpleBlocksController.php
- classes/models/FrmEntryFormatter.php
- classes/models/FrmFormAction.php
- tests/phpunit/forms/test_FrmFormsController.php
- classes/helpers/FrmFormTemplatesHelper.php
- tests/phpunit/misc/test_FrmAppHelper.php
- tests/phpunit/styles/test_FrmStylesController.php
- classes/controllers/FrmFieldsController.php
🚧 Files skipped from review as they are similar to previous changes (8)
- stripe/helpers/FrmTransLiteListHelper.php
- classes/helpers/FrmXMLHelper.php
- classes/helpers/FrmEmailSummaryHelper.php
- classes/models/FrmStyle.php
- tests/phpunit/entries/test_FrmShowEntryShortcode.php
- classes/models/FrmEntryMeta.php
- classes/controllers/FrmAddonsController.php
- classes/controllers/FrmEmailStylesController.php
🧰 Additional context used
🧬 Code graph analysis (6)
classes/helpers/FrmEntriesHelper.php (2)
classes/helpers/FrmShortcodeHelper.php (2)
FrmShortcodeHelper(9-157)get_shortcode_attribute_array(20-32)classes/controllers/FrmEntriesController.php (2)
FrmEntriesController(6-971)show_entry_shortcode(892-938)
classes/models/FrmPersonalData.php (3)
classes/models/FrmDb.php (2)
FrmDb(6-867)get_col(297-299)classes/models/FrmEntry.php (2)
FrmEntry(6-1283)getOne(477-505)classes/models/FrmField.php (1)
getOne(816-843)
classes/helpers/FrmAppHelper.php (1)
classes/models/FrmDb.php (2)
FrmDb(6-867)prepare_array_values(636-640)
tests/phpunit/forms/test_FrmFormsControllerAjax.php (1)
classes/models/FrmForm.php (1)
FrmForm(6-1407)
classes/controllers/FrmEntriesController.php (2)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5048)simple_get(807-816)classes/helpers/FrmListHelper.php (1)
get_pagenum(561-569)
classes/controllers/FrmFormsController.php (2)
classes/helpers/FrmAppHelper.php (3)
FrmAppHelper(6-5048)get_param(737-774)get_settings(280-291)classes/models/FrmForm.php (2)
FrmForm(6-1407)getOne(856-884)
🪛 PHPMD (2.15.0)
phpcs-sniffs/Formidable/Sniffs/WhiteSpace/ConsecutiveAssignmentSpacingSniff.php
311-328: Avoid unused private methods such as 'getVariableSignature'. (undefined)
(UnusedPrivateMethod)
classes/controllers/FrmEntriesController.php
947-947: Avoid unused local variables such as '$id'. (undefined)
(UnusedLocalVariable)
classes/controllers/FrmFormsController.php
1728-1728: Avoid unused local variables such as '$linked_forms'. (undefined)
(UnusedLocalVariable)
1729-1729: Avoid unused local variables such as '$col'. (undefined)
(UnusedLocalVariable)
3301-3301: Avoid unused local variables such as '$submit'. (undefined)
(UnusedLocalVariable)
3389-3389: Avoid unused local variables such as '$errors'. (undefined)
(UnusedLocalVariable)
3390-3390: Avoid unused local variables such as '$form'. (undefined)
(UnusedLocalVariable)
3391-3391: Avoid unused local variables such as '$message'. (undefined)
(UnusedLocalVariable)
⏰ 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: Cypress
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
🔇 Additional comments (28)
tests/phpunit/forms/test_FrmFormsControllerAjax.php (2)
38-39: LGTM!Alignment formatting for consecutive variable assignments is consistent with the PR's sniff for grouping simple assignments.
79-80: LGTM!Alignment formatting for consecutive assignments is consistent with the PR's objective.
classes/helpers/FrmEntriesHelper.php (3)
52-52: LGTM! Formatting alignment applied.The assignment operator alignment improves code readability without changing functionality.
202-204: LGTM! Consecutive assignments properly aligned.The alignment of these consecutive assignments enhances readability while preserving the shortcode processing logic.
532-532: LGTM! Assignment alignment applied correctly.The formatting change aligns the assignment operator with the surrounding code. The security pattern (unsanitized extraction followed by immediate sanitization on line 533) remains intact.
classes/controllers/FrmFormsController.php (1)
255-255: LGTM! Consistent alignment improves readability.The consecutive assignment alignment changes throughout this file improve code consistency and readability. These formatting adjustments align with the new sniff being introduced for grouping simple assignments.
Also applies to: 1446-1448, 1727-1730, 3300-3301, 3389-3391
classes/models/FrmPersonalData.php (3)
149-150: LGTM: Formatting alignment for consecutive assignments.The whitespace adjustment aligns the assignment operators for improved visual grouping, consistent with the new sniff introduced in this PR.
158-159: LGTM: Formatting alignment for consecutive assignments.The whitespace adjustments align the assignment operators, improving code readability and consistency with the new spacing rules.
179-180: LGTM: Formatting alignment for consecutive assignments.The whitespace adjustment on line 179 aligns the assignment operator with line 180, maintaining consistency with the formatting rules enforced by the new sniff.
classes/controllers/FrmEntriesController.php (6)
88-88: LGTM! Alignment formatting applied.This alignment change improves code readability by grouping consecutive assignments consistently.
189-189: LGTM! Alignment formatting applied.This alignment change maintains consistency with the new consecutive assignment spacing standard.
585-585: LGTM! Alignment formatting applied.The assignment alignment improves consistency with the codebase formatting standard.
620-622: LGTM! Alignment formatting applied.These consecutive assignment alignments enhance readability and conform to the new coding standard.
924-924: LGTM! Alignment formatting applied.The assignment alignment maintains consistency with the formatting standard being enforced across the codebase.
946-947: LGTM! Alignment formatting applied.These consecutive assignment alignments improve code consistency. Note: The static analysis warning about
$idbeing unused is a false positive—$idis assigned a value on line 952 when$entryexists and is used in the included template on line 969.classes/helpers/FrmAppHelper.php (8)
827-861: Keep theget_simple_request()refactor purely cosmetic (looks OK).Line 836 is just alignment/assignment formatting; behavior remains the same.
1891-1918:maybe_autocomplete_pages_options()change is formatting-only.Line 1892 is just spacing/alignment; logic is unchanged.
1927-1993:maybe_autocomplete_options()change is formatting-only.Line 1941 is just spacing/alignment; logic is unchanged.
2445-2452:check_selected()changes are formatting-only.Lines 2446-2447 are alignment-only; behavior is unchanged.
3071-3118:truncate()changes are formatting-only.Lines 3089-3090 are just alignment; logic is unchanged.
3170-3192:get_formatted_time()change is formatting-only.Line 3185 is alignment-only; logic is unchanged.
3237-3281:human_time_diff()changes are formatting-only.Lines 3246-3247 are alignment-only; logic is unchanged.
4248-4264:images_dropdown()change is formatting-only.Line 4249 is alignment-only; logic is unchanged.
classes/models/FrmEntry.php (1)
101-101: LGTM! Formatting aligns with the new sniff.The alignment of consecutive assignment operators improves code readability and is consistent with the new
ConsecutiveAssignmentSpacingSniffintroduced in this PR.Also applies to: 280-280, 346-346, 603-603, 620-620, 711-711
stripe/controllers/FrmTransLiteActionsController.php (4)
265-272: LGTM! Good defensive programming with the fallback logic.The assignment alignment on lines 265-266 improves readability and aligns with the PR's objective of grouping simple assignments.
The new fallback logic on lines 269-271 correctly handles cases where the computed trigger event is not in the allowed triggers list. The fallback appropriately maps the 'complete' status to 'payment-success' and treats unknown statuses as 'payment-failed', which is a reasonable safe default.
313-313: LGTM!Alignment formatting consistent with PR objectives.
511-511: LGTM!Alignment formatting consistent with PR objectives.
567-567: LGTM!Alignment formatting consistent with PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
classes/helpers/FrmAppHelper.phpclasses/helpers/FrmEmailSummaryHelper.phpphpcs-sniffs/Formidable/Sniffs/WhiteSpace/ConsecutiveAssignmentSpacingSniff.php
🚧 Files skipped from review as they are similar to previous changes (1)
- classes/helpers/FrmEmailSummaryHelper.php
🧰 Additional context used
🧬 Code graph analysis (1)
classes/helpers/FrmAppHelper.php (1)
classes/models/FrmDb.php (2)
FrmDb(6-867)prepare_array_values(636-640)
🪛 PHPMD (2.15.0)
phpcs-sniffs/Formidable/Sniffs/WhiteSpace/ConsecutiveAssignmentSpacingSniff.php
230-247: Avoid unused private methods such as 'getVariableSignature'. (undefined)
(UnusedPrivateMethod)
⏰ 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 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: Cypress
🔇 Additional comments (16)
classes/helpers/FrmAppHelper.php (9)
836-836: LGTM: Formatting improvement for visual alignment.This whitespace adjustment aligns consecutive variable assignments, improving code readability and consistency with the new coding standard being applied across the codebase.
1376-1376: LGTM: Consistent assignment alignment.The whitespace adjustment aligns this assignment with the surrounding code structure.
1892-1892: LGTM: Visual grouping of assignments.The spacing adjustment groups consecutive variable assignments for improved readability.
1941-1941: LGTM: Alignment formatting.Whitespace adjustment for consistent alignment of consecutive assignments.
2446-2447: LGTM: Aligned consecutive assignments.The spacing creates visual alignment between these two related function calls, improving readability.
2463-2463: LGTM: Extended alignment for consistency.The spacing aligns this assignment with the longer variable name on the preceding line.
3089-3090: LGTM: Clean initialization alignment.These aligned variable initializations create a clear, readable block of related assignments.
3185-3185: LGTM: Aligned assignments.Whitespace adjustment maintains consistent spacing with surrounding assignments.
4250-4250: LGTM: Consistent formatting applied.This completes the consistent alignment pattern applied throughout the file. All changes are purely cosmetic whitespace adjustments that improve visual consistency without affecting functionality.
phpcs-sniffs/Formidable/Sniffs/WhiteSpace/ConsecutiveAssignmentSpacingSniff.php (7)
31-54: LGTM! Clean class structure.The class structure follows PHP_CodeSniffer conventions properly, with appropriate constants and properties for tracking state. The registration for
T_VARIABLEtokens is correct for detecting variable assignments.
64-127: LGTM! Well-structured main processing logic.The
process()method follows a clear and logical flow:
- Deduplicates using a filename:position key
- Validates simple assignments before processing
- Collects consecutive assignment groups
- Applies reasonable constraints (max group size, no duplicates)
- Detects and fixes blank line violations
The fixable error reporting and auto-fix integration is properly implemented.
137-186: LGTM! Robust group collection logic.The method correctly collects consecutive assignments with appropriate break conditions:
- Validates each assignment is simple and single-line
- Breaks on more than 2 blank lines to respect intentional separation
- Properly delegates to helper methods for validation
257-333: LGTM! Comprehensive validation logic.The
isSimpleAssignmentStart()method thoroughly validates assignment statements with proper handling of:
- Statement boundaries (curly braces, semicolons, etc.)
- Array access syntax (
$var[],$var['key'])- Multiple assignment operators including
??=The implementation correctly distinguishes between simple assignments and other variable uses.
343-362: LGTM! Proper nesting handling.The method correctly tracks nesting levels to find the terminating semicolon, which properly handles complex expressions with nested parentheses, brackets, and braces.
389-424: LGTM! Smart comment handling.The method correctly prevents grouping when comments separate assignments (lines 411-416), which respects the developer's intent to document or separate specific assignments. The dual-pass approach (skip comments to find next statement, then check if comments exist) is the right design.
435-476: LGTM! Proper indentation preservation.The
removeBlankLines()method correctly:
- Uses the PHP_CodeSniffer fixer API with changesets
- Preserves indentation by extracting it from the next statement
- Handles both indented and non-indented code (column check on line 443)
- Replaces all intervening whitespace with a single newline and appropriate indentation
…signments Add a new sniff for grouping simple assignments
1 participant
No description provided.