Further improve whitespace consistency after end curly braces#2770
Conversation
WalkthroughThis PR consists primarily of formatting and whitespace adjustments across multiple files, with several localized logic enhancements including stylesheet URL storage, JSON encoding in XML migration methods, field metadata population in duplicate detection, and expanded array-access handling in PHP code sniffer rules. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 (3)
classes/controllers/FrmStylesController.php (1)
1305-1318: Avoid potential undefined-index notices when mappingalt_bg_color→bg_color_active.If a style doesn’t have
bg_color_active(older defaults, partial post_content, etc.),$style->post_content[ $setting ]will throw notices. Safer to fall back to the existing$val(or$style->post_content[ $name ]) when missing.Proposed tweak
if ( 'border_width' === $name ) { $setting = 'field_border_width'; } elseif ( 'alt_bg_color' === $name ) { $setting = 'bg_color_active'; } - $default_styles[ $name ] = $style->post_content[ $setting ]; + $default_styles[ $name ] = $style->post_content[ $setting ] ?? $style->post_content[ $name ] ?? $val; unset( $name, $val );phpcs-sniffs/Formidable/Sniffs/WhiteSpace/BlankLineAfterClosingBraceSniff.php (1)
60-98: Handle chained array access (e.g.,$a[0][1] = ...) before assignment.Current logic skips only one
[...]segment; a second bracket will cause a false negative.Proposed fix (loop through consecutive bracketed segments)
- $checkToken = $nextToken; - - // If followed by array access, skip to after the closing bracket. - if ( $tokens[ $nextToken ]['code'] === T_OPEN_SQUARE_BRACKET ) { - // Find the matching closing bracket. - if ( isset( $tokens[ $nextToken ]['bracket_closer'] ) ) { - $closeBracket = $tokens[ $nextToken ]['bracket_closer']; - $checkToken = $phpcsFile->findNext( T_WHITESPACE, $closeBracket + 1, null, true ); - - if ( false === $checkToken ) { - return; - } - } else { - return; - } - } + $checkToken = $nextToken; + + // If followed by array access, skip to after the closing bracket(s). + while ( isset( $tokens[ $checkToken ] ) && $tokens[ $checkToken ]['code'] === T_OPEN_SQUARE_BRACKET ) { + if ( ! isset( $tokens[ $checkToken ]['bracket_closer'] ) ) { + return; + } + $closeBracket = $tokens[ $checkToken ]['bracket_closer']; + $checkToken = $phpcsFile->findNext( T_WHITESPACE, $closeBracket + 1, null, true ); + if ( false === $checkToken ) { + return; + } + }classes/helpers/FrmXMLHelper.php (1)
1953-2025: UseFrmAppHelper::prepare_and_encode()for consistency and add null guard forswitch_action_field_ids()return value.When
$switchis true andswitch_action_field_ids()returnsnull(when$frm_duplicate_idsis empty),json_encode(null)becomes the literal string"null"in the database. Additionally, this code should useFrmAppHelper::prepare_and_encode()instead of rawjson_encode()to match the codebase standard applied everywhere else (FrmOnSubmitHelper, migrate_email_settings_to_action, FrmDb::save_settings, tests). The prepare_and_encode method handles critical slash escaping for escape sequences like\r,\n,\u,\t.Recommended fix
- $new_action['post_content'] = json_encode( $new_action['post_content'] ); + if ( null === $new_action['post_content'] ) { + $new_action['post_content'] = array(); + } + $new_action['post_content'] = FrmAppHelper::prepare_and_encode( $new_action['post_content'] );
🧹 Nitpick comments (1)
classes/helpers/FrmXMLHelper.php (1)
2256-2282: Joiningemail_tointo a string is fine, but consider trimming/filtering empties.
preg_splitcan yield empty items; imploding them can produce", , "sequences.Optional hardening
- $atts['email_to'] = implode( ', ', $atts['email_to'] ); + $atts['email_to'] = implode( + ', ', + array_values( + array_filter( + array_map( 'trim', $atts['email_to'] ), + 'strlen' + ) + ) + );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
classes/controllers/FrmEmailStylesController.phpclasses/controllers/FrmEntriesController.phpclasses/controllers/FrmFormTemplatesController.phpclasses/controllers/FrmFormsController.phpclasses/controllers/FrmStylesController.phpclasses/helpers/FrmFieldsHelper.phpclasses/helpers/FrmStylesCardHelper.phpclasses/helpers/FrmXMLHelper.phpclasses/models/FrmApplicationTemplate.phpclasses/models/FrmEntry.phpclasses/models/FrmEntryMeta.phpclasses/models/FrmField.phpclasses/models/FrmForm.phpclasses/models/FrmInbox.phpclasses/models/fields/FrmFieldCaptcha.phpphpcs-sniffs/Formidable/Sniffs/WhiteSpace/BlankLineAfterClosingBraceSniff.phpstripe/models/FrmStrpLiteAuth.phptests/phpunit/emails/test_FrmEmail.php
⏰ 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). (9)
- 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 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
🔇 Additional comments (20)
classes/models/FrmInbox.php (1)
462-462: LGTM! Formatting improvement enhances readability.The blank line added after the
parse_strblock provides clear visual separation between parsing the query string and modifying it, making the code flow more evident.tests/phpunit/emails/test_FrmEmail.php (1)
373-373: LGTM! Consistent formatting improvement in test code.The blank line clearly separates the sitename normalization logic from its usage, improving test readability.
classes/controllers/FrmFormTemplatesController.php (1)
569-569: LGTM! Improves visual structure of category organization.The blank line provides clear separation between the conditional category addition and the final 'all-items' category definition, making the special categories section more scannable.
classes/helpers/FrmStylesCardHelper.php (1)
276-276: LGTM! Enhances logical grouping in style parameter generation.The blank line effectively separates the background color determination logic from the subsequent style rules application, improving code structure and readability.
stripe/models/FrmStrpLiteAuth.php (1)
376-376: LGTM! Clarifies control flow in intent processing.The blank line provides clear visual separation after the setup intent check and before payment intent processing, making the logic flow easier to follow.
classes/controllers/FrmEntriesController.php (1)
255-263: Whitespace-only change looks good. This improves readability and matches the PR objective.classes/models/fields/FrmFieldCaptcha.php (1)
321-330: Whitespace-only change looks good. No logic impact.classes/controllers/FrmFormsController.php (2)
1224-1236: Whitespace-only change looks good.
1609-1616: Whitespace-only change looks good.classes/helpers/FrmFieldsHelper.php (1)
1812-1824: Whitespace-only change looks good.classes/controllers/FrmEmailStylesController.php (1)
303-312: Whitespace-only change looks good.classes/models/FrmEntry.php (1)
106-121: Functional change in a “whitespace” PR: please double-check duplicate-detection equivalence. The new$field_metas[ $meta->field_id ] = $meta->meta_valuemapping is sensible, but it relies onFrmEntryMeta::get_entry_meta_info()returning values in the same (serialized vs non-serialized) form you later compare againstarray_map( 'maybe_serialize', ...).classes/models/FrmField.php (1)
694-703: Whitespace-only change in serialization block looks good.classes/models/FrmApplicationTemplate.php (1)
192-205: Formatting-only spacing improvements are fine; no behavior change spotted.Also applies to: 210-226
classes/models/FrmEntryMeta.php (2)
361-370: No-op spacing change in meta_field_query is fine.
552-560: No-op spacing change in get_ids_query is fine.classes/controllers/FrmStylesController.php (1)
240-250: Explicitly keying the stylesheet URL by theformidablehandle is the right direction.classes/models/FrmForm.php (1)
356-367: Unconditionally populating$field_arrayimproves reliability and avoids extra lookups.classes/helpers/FrmXMLHelper.php (2)
776-799: Whitespace-only adjustment in placeholder migration block is fine.
2082-2137: This concern is unfounded.FrmDb::save_json_postdoes not encodepost_content—it passes the settings array directly towp_insert_post, which saves the string value as-is.The
prepare_and_encodefunction correctly returns a JSON-encoded string, which is the expected format forpost_contentwhen saving viasave_json_post. There is no double encoding, and the contract is being followed correctly throughout the codebase.Likely an incorrect or invalid review comment.
…onsistency Further improve whitespace consistency after end curly braces
1 participant
No description provided.