Organize the rector config#2718
Conversation
|
Warning Rate limit exceeded@Crabcyborg has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 4 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 (1)
WalkthroughReplaced multiple loose equality checks with strict comparisons across several PHP files and reorganized the project's Rector configuration, updating enabled/disabled rules, the withSkip list, and the prepared sets (notably toggling typeDeclarations). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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
🧹 Nitpick comments (1)
rector.php (1)
72-73: Unused import:RenameFunctionRector.This import is not referenced in the
withSkiparray or elsewhere in the configuration. If not needed, remove it to keep imports clean.🔎 Proposed fix
-// Renaming -use Rector\Renaming\Rector\FuncCall\RenameFunctionRector; -
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
classes/helpers/FrmAppHelper.phpclasses/helpers/FrmFormsHelper.phpclasses/models/FrmEntry.phpclasses/models/FrmStyle.phpclasses/views/frm-form-actions/_action_inside.phprector.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). (1)
- GitHub Check: Cypress
🔇 Additional comments (7)
classes/models/FrmEntry.php (1)
660-666: Strict comparison oncount($where)is safe and consistent
count($where) === 1is type‑correct here (count returns int and$whereis confirmed array) and doesn’t change behavior from== 1, while aligning with the new strict‑equality rule. No further changes needed.classes/helpers/FrmFormsHelper.php (1)
1868-1885: Consistent strict equality on unsafe param countUsing
$count === 1(with$countfromcount()) is type‑correct and brings this branch in line with the existing$count === 0check, without changing behavior.classes/views/frm-form-actions/_action_inside.php (1)
33-43: Single‑event check now uses strict comparison
count( $action_control->action_options['event'] ) === 1is appropriate here (int on both sides) and doesn’t alter behavior, while matching the project’s move toward strict equality.classes/models/FrmStyle.php (1)
820-836: Even/odd quote check tightened with strict equality
$is_balanced = $char_count % 2 === 0;keeps the same parity logic as before while using a strict numeric comparison. No issues spotted.classes/helpers/FrmAppHelper.php (1)
3070-3085: Strict comparisons added intruncate,json_to_array, andmaybe_json_decodelook correct
truncate:$lengthis cast to int, so$length === 0is an appropriate strict check and equivalent to the old== 0.json_to_array:$pand$lastare numeric indexes, so$p === $lastis a safe refinement with no behavioral change.maybe_json_decode:json_last_error() === JSON_ERROR_NONEis the idiomatic, type‑correct check for a successful decode.All three updates align with the stricter‑equality Rector rule without altering runtime behavior.
Also applies to: 3460-3475, 3695-3707
rector.php (2)
95-128: Configuration organization is clear and well-documented.The inline comments explaining each boolean parameter improve maintainability. Disabling
typeDeclarations(line 103) aligns with the PR objective that the codebase isn't ready for that type approach yet.
149-229: Well-organized skip list with helpful rationale comments.The category-based organization mirrors the import structure, making it easy to cross-reference. The inline comments explaining specific exclusions (e.g.,
JoinStringConcatRectorchanging\tto actual tabs, WP standard not encouraging Elvis operator) provide valuable context for future maintainers.
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)
rector.php (1)
1-213: PR description inconsistency: "Split into multiple files" vs. single organized file.The PR description states: "Split the Rector configuration into multiple files grouped by category," but the changes show a single
rector.phpfile with internal organization using section comments. The actual work accomplished is valuable—improving maintainability through categorization—but it's not a split into multiple files.Please clarify:
- Was the intention to split into multiple files in a future PR?
- Should the PR description be updated to reflect "Organized the Rector configuration with category groupings"?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rector.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). (3)
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
🔇 Additional comments (4)
rector.php (4)
93-93: LGTM: typeDeclarations set to false as described.This change aligns with the PR description: "Removed the prepared
typeDeclarationset because the codebase is not using that type approach yet."
139-213: Excellent organization of skip rules by category.The categorization of skip rules matches the import organization and significantly improves maintainability. The inline explanatory comments (e.g., lines 152-153, 157-158, 190-191, 202-203) provide valuable context for why specific rules are skipped.
4-72: The import organization improves readability; no action needed.The changes are a straightforward refactoring of the rector.php configuration with category comments grouping related imports. The structure is appropriate and maintainable. No missing rule configuration or inconsistency exists.
99-99: Clarify the intentionalnamingflag change in the PR description.The commit message "Try naming too" documents this change, confirming it's intentional. However, since the PR description doesn't mention this flag modification, consider explicitly documenting why the
namingRector rule was disabled as part of the PR summary for future reviewers.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @rector.php:
- Line 11: The import ExplicitReturnNullRector is unused in this configuration;
either remove the unused use statement for ExplicitReturnNullRector or add
ExplicitReturnNullRector::class to the withSkip array so the rule is explicitly
skipped; locate the use
Rector\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector; statement and
either delete it or add ExplicitReturnNullRector::class into the existing
withSkip([...]) configuration entry.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rector.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). (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 (2)
rector.php (2)
5-71: Clean organization with helpful category groupings.The import statements are well-organized by category with clear section comments. This improves maintainability and makes it easier to locate and manage specific rules.
139-214: Well-documented skip list with explanatory comments.The inline comments explaining why specific rules are skipped (e.g., WP standard compatibility, readability preferences) are valuable for maintainability.
Organize the rector config
1 participant
typeDeclarationprepared set because we aren't using types this way yet.namingfor now as I don't intend to change the name of anything at the moment.UseIdenticalOverEqualWithSameTypeRectorrule.