Add rector rule for SimplifyIfReturnBoolRector

[Crabcyborg]

No description provided.

[coderabbitai]

Warning

Rate limit exceeded

@Crabcyborg has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 21 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 08c466e and 514c5af.

📒 Files selected for processing (2)

• classes/models/FrmEntry.php (1 hunks)
• classes/models/fields/FrmFieldType.php (1 hunks)

Walkthrough

This PR simplifies conditional return logic across 15 files by replacing explicit if-else blocks with single return expressions. The rector.php configuration was updated to remove SimplifyIfReturnBoolRector from the skipped rules, enabling this simplification pattern to be applied throughout the codebase.

Changes

Cohort / File(s)	Summary
Controllers classes/controllers/FrmAddonsController.php, classes/controllers/FrmAppController.php, classes/controllers/FrmDashboardController.php	Simplified conditional return statements to single boolean expressions in can_install_addon_api(), in_our_pages(), display_counter_cta(), and welcome_banner_has_closed().
Helpers classes/helpers/FrmApiHelper.php, classes/helpers/FrmAppHelper.php, classes/helpers/FrmFormMigratorsHelper.php, classes/helpers/FrmFormsHelper.php	Refactored return logic in is_for_user(), validate_url_is_in_s3_bucket(), is_dismissed(), and is_gutenberg_editor() to use direct boolean expressions instead of multi-line conditionals.
Models classes/models/FrmApplicationTemplate.php, classes/models/FrmEmail.php, classes/models/FrmEntry.php, classes/models/FrmStyle.php	Consolidated conditional returns in is_available_for_purchase(), should_send(), is_duplicate_check_needed(), and should_remove_every_brace() to single-line expressions.
Field/View Components classes/models/fields/FrmFieldType.php, classes/views/styles/components/FrmStyleComponent.php	Simplified should_strip_most_html() and hide_component() conditional logic. Note: FrmFieldType.php removes the updated_by privilege check.
Stripe Integration stripe/models/FrmStrpLiteAuth.php	Refactored statement_descriptor_is_valid() and payment_failed() to eliminate intermediate returns and use direct boolean expressions.
Configuration rector.php	Removed import and skip configuration entry for SimplifyIfReturnBoolRector to enable the rule application across the codebase.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

• FrmFieldType.php: Requires careful verification—the removal of the updated_by privilege check may alter control flow behavior when updated_by is privileged but user_id is absent.
• FrmFormMigratorsHelper.php: Multiple consolidated conditions in is_dismissed() need validation for correctness.
• Rector configuration: Verify that removing SimplifyIfReturnBoolRector from skip list doesn't conflict with other active rules or produce unintended changes.

Possibly related PRs

• Remove some rector exceptions #2614: Both PRs modify rector.php to adjust configured Rector rules by removing specific entries.
• Add new rector rule for SimplifyIfElseToTernaryRector #2666: Both PRs update rector.php to remove/swap simplify-if and ternary Rector rules and apply similar conditional-to-expression refactorings in overlapping files.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No description was provided by the author; the PR has no description content to evaluate.	Add a description explaining why this refactoring is being applied, what benefit it provides, and any relevant context about the SimplifyIfReturnBoolRector rule.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: applying the SimplifyIfReturnBoolRector rule, which is evidenced by the removal of the rule skip and the systematic simplification of boolean return logic across 13 files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

classes/helpers/FrmAppHelper.php (1)

4815-4816: Orphaned comment is now misleading.

The comment // The URL isn't to an XML file. was originally associated with the early return for the failure case. Now that the logic is simplified to a single return, this comment is misleading since:

1. The function is generic and handles any expected extension, not just XML
2. The comment doesn't accurately describe what the return statement does

Consider removing the orphaned comment or updating it:

-		// The URL isn't to an XML file.
 		return $expected_extension === $ext;

classes/helpers/FrmFormMigratorsHelper.php (1)

18-18: Consider using strict comparison in in_array.

The in_array call is missing the third parameter for strict comparison. While the current usage with string class names likely works correctly, adding strict comparison is a defensive best practice to avoid potential type coercion issues.

-		return ! empty( $dismissed ) && in_array( $form['class'], $dismissed );
+		return ! empty( $dismissed ) && in_array( $form['class'], $dismissed, true );

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 610dc4f and 08c466e.

📒 Files selected for processing (15)

• classes/controllers/FrmAddonsController.php (1 hunks)
• classes/controllers/FrmAppController.php (1 hunks)
• classes/controllers/FrmDashboardController.php (2 hunks)
• classes/helpers/FrmApiHelper.php (1 hunks)
• classes/helpers/FrmAppHelper.php (1 hunks)
• classes/helpers/FrmFormMigratorsHelper.php (1 hunks)
• classes/helpers/FrmFormsHelper.php (1 hunks)
• classes/models/FrmApplicationTemplate.php (1 hunks)
• classes/models/FrmEmail.php (1 hunks)
• classes/models/FrmEntry.php (1 hunks)
• classes/models/FrmStyle.php (1 hunks)
• classes/models/fields/FrmFieldType.php (1 hunks)
• classes/views/styles/components/FrmStyleComponent.php (1 hunks)
• rector.php (0 hunks)
• stripe/models/FrmStrpLiteAuth.php (2 hunks)

💤 Files with no reviewable changes (1)

• rector.php

🧰 Additional context used

🧬 Code graph analysis (3)

classes/models/FrmApplicationTemplate.php (1)

classes/helpers/FrmFormsHelper.php (2)

• FrmFormsHelper (6-2188)
• plan_is_allowed (1667-1690)

classes/controllers/FrmAppController.php (1)

classes/helpers/FrmAppHelper.php (2)

• FrmAppHelper (6-5050)
• simple_get (807-816)

classes/views/styles/components/FrmStyleComponent.php (1)

classes/helpers/FrmAppHelper.php (2)

• FrmAppHelper (6-5050)
• get_param (737-774)

⏰ 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 7.4 tests in WP trunk
• GitHub Check: PHP 8 tests in WP trunk
• GitHub Check: Cypress
• GitHub Check: Cypress
• GitHub Check: PHP 8 tests in WP trunk
• GitHub Check: PHP 7.4 tests in WP trunk

🔇 Additional comments (14)

classes/models/FrmStyle.php (1)

371-371: LGTM!

The simplification correctly converts the preg_match result to a boolean. Since preg_match returns 1 for a match and 0 for no match, the (bool) cast produces the same result as the previous if/else block.

classes/models/FrmEntry.php (1)

267-267: LGTM!

The negated condition correctly inverts the original early-return logic. When parent_form_id is set and truthy (repeating field entries), the function returns false to skip duplicate checking, which matches the original behavior.

classes/controllers/FrmDashboardController.php (2)

300-300: LGTM!

The boolean expression correctly consolidates the CTA display logic: show CTA when counter is zero or less, except for the entries counter when no form exists.

---

368-368: LGTM!

The simplified expression correctly checks if the banner was closed by the current user. The strict comparison (true parameter) in in_array is preserved, ensuring type-safe matching.

classes/controllers/FrmAddonsController.php (1)

1340-1343: Auth comparison simplification looks correct

The new return ! empty( $auth ) && hash_equals( $auth, $post_auth ); preserves the previous behavior with proper short‑circuiting and keeps hash_equals use intact. No logic or security regression here.

classes/helpers/FrmFormsHelper.php (1)

2109-2122: is_gutenberg_editor boolean return is safely simplified

Returning 'post.php' === $pagenow directly is logically equivalent to the previous conditional and keeps the earlier REST URL guards intact.

classes/controllers/FrmAppController.php (1)

1436-1447: in_our_pages final condition simplification is correct

The combined return using in_array( $pagenow, ... ) && 'frm_application' === FrmAppHelper::simple_get( 'taxonomy' ) is behaviorally equivalent to the prior branching logic and keeps strict comparisons.

classes/models/FrmEmail.php (1)

487-520: has_recipients simplification matches existing behavior

return ! ( empty( $this->to ) && empty( $this->cc ) && empty( $this->bcc ) ); is equivalent to the previous explicit true/false branching and keeps should_send() semantics unchanged.

classes/helpers/FrmApiHelper.php (1)

20-43: is_for_user tail return is correctly collapsed

The final return self::check_free_segments( $who ); preserves the earlier behavior where the method fell through to checking the free segments last; only the return style changed.

classes/models/FrmApplicationTemplate.php (1)

242-267: is_available_for_purchase now returns plan check directly

return FrmFormsHelper::plan_is_allowed( $args ); is a straight simplification of the previous true/false branching and keeps all surrounding validation logic intact.

classes/views/styles/components/FrmStyleComponent.php (1)

230-235: hide_component conditional is safely condensed

The method still short‑circuits when not_show_in is empty, and the final equality check against the sanitized section param is equivalent to the previous if/return pattern.

classes/models/fields/FrmFieldType.php (1)

1779-1786: HTML‑stripping privilege check remains equivalent

The new return ! ( ! empty( $entry->user_id ) && $this->user_id_is_privileged( $entry->user_id ) ); together with the preceding updated_by check keeps the same logic: do not strip for privileged updated_by or user_id, strip otherwise. Only the return style was compacted.

stripe/models/FrmStrpLiteAuth.php (2)

659-659: LGTM!

The simplification correctly converts the explicit conditional return to a direct boolean expression. The (bool) cast appropriately handles preg_match's return values (1, 0, or false).

---

864-864: LGTM!

The simplification correctly converts the explicit conditional return to a direct boolean expression. The compound condition logic is preserved and reads clearly.