Skip to content

Conversation

@calebdw
Copy link
Contributor

@calebdw calebdw commented Aug 20, 2025

Hello!

The primary goal is to move all the orphaned rules (rules that aren't in any sets) into sets or at least document them in the README. There are some rules that are configurable or are too opinionated/dangerous to be included in a set by default so I just documented these in the README.

Note

Laravel Lumen has been archive for over a year and hasn't been updated in over 2 years. The Lumen rules should probably be removed at some point, but that would technically be a BC.

Thanks!

This adds all the rules that require configuration to the README.
This adds the more opinionated rules to the README that (in my opinion)
should not be included in any sets by default, and could be more dangerous
to use.
@calebdw calebdw force-pushed the calebdw/push-mrqposwssyls branch from 4a2a359 to 193ad1e Compare August 20, 2025 04:28
@peterfox peterfox self-requested a review September 22, 2025 09:28
@peterfox peterfox merged commit 97ece0a into driftingly:main Sep 22, 2025
5 checks passed
@calebdw calebdw deleted the calebdw/push-mrqposwssyls branch September 22, 2025 12:36
@GeniJaho
Copy link
Collaborator

GeniJaho commented Sep 28, 2025

We've added configurable rules to the Code Quality set, which are not risky and have sane defaults, but they might cause us some issues down the road.

There is no way to 'reconfigure' these rules, and use the Code Quality set at the same time. The set's defaults take precedence over the user's configurations.

For that reason I think we should remove these rules from the Code Quality set:

  • ApplyDefaultInsteadOfNullCoalesceRector
  • OptionalToNullsafeOperatorRector
  • EloquentOrderByToLatestOrOldestRector

What do you guys think?

@calebdw
Copy link
Contributor Author

calebdw commented Sep 30, 2025

It is possible to add configuration to a rule, you just can't remove what's already been configured. In this case, I think all of these rules are fine:

  • OptionalToNullsafeOperatorRector: this rule allows a user to configure an exclude_method list, we don't exclude anything by default so the user is still free to exclude things in their own configs without any issues
  • ApplyDefaultInsteadOfNullCoalesceRector: this allows the user to configure other custom helpers, I see no reason why these defaults are not okay from a Laravel code quality perspective:
    image
  • EloquentOrderByToLatestOrOldestRector: here the user is free to provide additional patterns, but both *_at and *_on are Laravel conventions for datetime/date columns, and nothing really should end in *_date that's not a date

I don't see any big cause for concern in any of these default configurations, if anything does come up then we can address at that point in time

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants