search

driftingly / rector-laravel

Rector upgrades rules for Laravel
http://getrector.org
MIT License
500 stars 49 forks source link

Allow configuration with `EloquentOrderByToLatestOrOldestRector` rule #147

Closed johnbacon closed 9 months ago

johnbacon commented 11 months ago

By default, EloquentOrderByToLatestOrOldestRector modifies all orderBy and orderByDesc methods to use latest() or oldest() instead.

Our team feels we'd like to do this only regarding timestamps, as oldest('name') or similar could be confusing.

As such, some configuration allowing for explicit column names you'd like to change to oldest() or latest() is desired.

This suggestion was also briefly mentioned in the relevant PR:

These changes apply to all columns in our project, not just date/datetime columns. Is that intended behavior? If so, I'm wondering if rector-laravel is open to configuration, so we can specify which columns should and shouldn't have these rules applied.

peterfox commented 11 months ago

@johnbacon what's your thoughts, a list of columns or simply a regex to match? At current the rule will refactor if a variable is used. If this was enabled. It would only refactor with string literals.

johnbacon commented 10 months ago

@peterfox The implementation allows for wildcards and a list of columns, but do you feel regex would provide more benefit on top of that?

The fact that static analysis can't determine the name of a column within a variable was a bummer to learn. I "got around" that by allowing you to specify the name of a variable, e.g. '$columnName'. But if you always want to refactor all variables in an orderBy(), I'm not sure what would be best. Maybe an additional option you config? Maybe defining '$*' or similar would work?

peterfox commented 10 months ago

Examining variable names is a bit risky. I'd likely stick to string literals. Regex, to me, makes the most sense rather than listing all possibilities someone might have. Wildcard could work, but sometimes it's more effort when Regex is more flexible, and the example of ends with _at would be easy enough.

johnbacon commented 10 months ago

Is that something you could possibly lend a hand on? I'm less comfortable with regex and even more so in the Laravel/Rector ecosystem, unfortunately.

peterfox commented 10 months ago

@johnbacon Sure, I'll see if I can find time to work on it.