Skip to content

Remove some rector exceptions#2614

Merged
Crabcyborg merged 1 commit into
masterfrom
remove_some_rector_exceptions
Nov 28, 2025
Merged

Remove some rector exceptions#2614
Crabcyborg merged 1 commit into
masterfrom
remove_some_rector_exceptions

Conversation

@Crabcyborg
Copy link
Copy Markdown
Contributor

No description provided.

@Crabcyborg Crabcyborg added this to the 6.26 milestone Nov 28, 2025
@Crabcyborg Crabcyborg merged commit fac102c into master Nov 28, 2025
35 of 36 checks passed
@Crabcyborg Crabcyborg deleted the remove_some_rector_exceptions branch November 28, 2025 14:59
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 28, 2025

Walkthrough

Modified rector.php configuration to remove unused Rector rule entries from the skip list, remove explicit registration of ReduceAlwaysFalseIfOrRector, and add inline documentation comments explaining rationale for retained/newly included rules.

Changes

Cohort / File(s) Summary
Rector Configuration
rector.php
Removed ReduceAlwaysFalseIfOrRector from imports and active ruleset. Cleaned up skip list by removing three non-existing rules: RemoveNonExistingVarAnnotationRector, RemoveUnusedNonEmptyArrayBeforeForeachRector, and UnnecessaryTernaryExpressionRector. Added inline comments for JoinStringConcatRector (tab character preference), RemoveUselessParamTagRector (param tag retention preference), and SimplifyEmptyArrayCheckRector (empty array check expression preference).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Verify that removed rules from skip list correspond to actual non-existing/unused Rector rules in the installed version
  • Confirm that the commented rules (JoinStringConcatRector, RemoveUselessParamTagRector, SimplifyEmptyArrayCheckRector) are properly enabled in the ruleset
  • Ensure ReduceAlwaysFalseIfOrRector removal doesn't unexpectedly enable unwanted rule transformations

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether a description relates to the changeset. Please add a pull request description explaining the motivation and impact of removing these Rector exceptions and configuration changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Remove some rector exceptions' accurately describes the main change: removing certain Rector rule exceptions and skip list entries from the configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch remove_some_rector_exceptions

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

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 26.17%. Comparing base (624b140) to head (e0a38d6).
⚠️ Report is 45 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2614      +/-   ##
============================================
- Coverage     26.24%   26.17%   -0.08%     
+ Complexity     8836     8831       -5     
============================================
  Files           144      144              
  Lines         29803    29798       -5     
============================================
- Hits           7822     7799      -23     
- Misses        21981    21999      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
rector.php (1)

94-104: Good rationale comments on skipped Rector rules; tiny wording tweak optional

The added comments around JoinStringConcatRector, RemoveUselessParamTagRector, and SimplifyEmptyArrayCheckRector make the intent of keeping these rules in the skip list much clearer for future maintainers. This is a nice improvement in config documentation.

If you want everything to read consistently, you could optionally mirror the style of the first two comments for SimplifyEmptyArrayCheckRector, e.g. explicitly say that you prefer the is_array() && empty() style instead of just describing what the rule does. Not required, just a clarity nit.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 313ca77 and e0a38d6.

📒 Files selected for processing (1)
  • rector.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). (6)
  • GitHub Check: PHP 7.4 tests in WP trunk
  • GitHub Check: PHP 8 tests in WP trunk
  • GitHub Check: Cypress
  • GitHub Check: PHP 7.4 tests in WP trunk
  • GitHub Check: PHP 8 tests in WP trunk
  • GitHub Check: Cypress

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant