rubocop / rubocop-performance

An extension of RuboCop focused on code performance checks.
https://docs.rubocop.org/rubocop-performance
MIT License
670 stars 79 forks source link

Cop idea: replace `list = list.select {...}` with `list.select! {...}` #432

Open vlad-pisanov opened 8 months ago

vlad-pisanov commented 8 months ago

When the result of select, reject, uniq, sort, sort_by, map, flatten, and reverse is assigned back to the variable it operates on, prefer the mutable version of the method, i.e.

# bad
list = list.select(&:odd?)
list = list.reject { |x| x > 42 }
list = list.uniq
list = list.sort
list = list.sort_by(&:name)

# good
list.select!(&:odd?)
list.reject! { |x| x > 42 }
list.uniq!
list.sort!
list.sort_by!(&:name)

This could also be a performance cop since mutating in-place can have performance advantages.

koic commented 8 months ago

Yeah, this is likely similar to the Performance/ChainArrayAllocation cop, so I will transfer it to rubocop-performance.

amomchilov commented 8 months ago

This could substantially reduce allocations/copies, but it's quite a can of worms.

The transformation is equivalent only if list isn't shared. This is hard enough in a statically-typed language with good lifetime analysis, but even harder in a dynamic language like Ruby.