Skip to content

Commit

Permalink
[Fix rubocop#178] Add new Performance/FilterMap cop
Browse files Browse the repository at this point in the history
Fixes rubocop#178.

This PR adds new `Performance/FilterMap` cop.

In Ruby 2.7, `Enumerable#filter_map` has been added.

This cop identifies places where `select.map` can be replaced by `filter_map`.
And it allows `map { ... }.compact` that is not compatible with `filter_map`.

```ruby
[true, false, nil].compact              #=> [true, false]
[true, false, nil].filter_map(&:itself) #=> [true]
```

The following is good and bad cases.

```ruby
# bad
ary.select(&:foo).map(&:bar)
ary.filter(&:foo).map(&:bar)

# good
ary.filter_map { |o| o.bar if o.foo }
ary.map(&:foo).compact # Consider whether you can safely replace it with `filter_map`.
```
  • Loading branch information
koic committed Feb 9, 2021
1 parent 4fe6edd commit 11d3c61
Show file tree
Hide file tree
Showing 7 changed files with 226 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### New features

* [#190](https://github.com/rubocop-hq/rubocop-performance/pull/190): Add new `Performance/RedundantSplitRegexpArgument` cop. ([@mfbmina][])
* [#178](https://github.com/rubocop-hq/rubocop-performance/issues/178): Add new `Performance/FilterMap` cop. ([@koic][])

### Bug fixes

Expand Down
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ Performance/EndWith:
VersionAdded: '0.36'
VersionChanged: '1.6'

Performance/FilterMap:
Description: 'Use `Enumerable#filter_map`.'
Enabled: pending
VersionAdded: '1.10'

Performance/FixedSize:
Description: 'Do not compute the size of statically sized objects except in constants.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Performance cops optimization analysis for your projects.
* xref:cops_performance.adoc#performancedetect[Performance/Detect]
* xref:cops_performance.adoc#performancedoublestartendwith[Performance/DoubleStartEndWith]
* xref:cops_performance.adoc#performanceendwith[Performance/EndWith]
* xref:cops_performance.adoc#performancefiltermap[Performance/FilterMap]
* xref:cops_performance.adoc#performancefixedsize[Performance/FixedSize]
* xref:cops_performance.adoc#performanceflatmap[Performance/FlatMap]
* xref:cops_performance.adoc#performanceinefficienthashsearch[Performance/InefficientHashSearch]
Expand Down
39 changes: 39 additions & 0 deletions docs/modules/ROOT/pages/cops_performance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,45 @@ for receiver is multiline string.

* https://github.com/JuanitoFatas/fast-ruby#stringmatch-vs-stringstart_withstringend_with-code-start-code-end

== Performance/FilterMap

NOTE: Required Ruby version: 2.7

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| No
| 1.10
| -
|===

In Ruby 2.7, `Enumerable#filter_map` has been added.

This cop identifies places where `select.map` can be replaced by `filter_map`.

And it allows `map { ... }.compact` that is not compatible with `filter_map`.

[source,ruby]
----
[true, false, nil].compact #=> [true, false]
[true, false, nil].filter_map(&:itself) #=> [true]
----

=== Examples

[source,ruby]
----
# bad
ary.select(&:foo).map(&:bar)
ary.filter(&:foo).map(&:bar)
# good
ary.filter_map { |o| o.bar if o.foo }
ary.map(&:foo).compact # Consider whether you can safely replace it with `filter_map`.
----

== Performance/FixedSize

|===
Expand Down
69 changes: 69 additions & 0 deletions lib/rubocop/cop/performance/filter_map.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Performance
# In Ruby 2.7, `Enumerable#filter_map` has been added.
#
# This cop identifies places where `select.map` can be replaced by `filter_map`.
#
# And it allows `map { ... }.compact` that is not compatible with `filter_map`.
#
# [source,ruby]
# ----
# [true, false, nil].compact #=> [true, false]
# [true, false, nil].filter_map(&:itself) #=> [true]
# ----
#
# @example
# # bad
# ary.select(&:foo).map(&:bar)
# ary.filter(&:foo).map(&:bar)
#
# # good
# ary.filter_map { |o| o.bar if o.foo }
# ary.map(&:foo).compact # Consider whether you can safely replace it with `filter_map`.
#
class FilterMap < Base
include RangeHelp
extend TargetRubyVersion

minimum_target_ruby_version 2.7

MSG = 'Use `filter_map` instead of `%<method_name>s.map`.'
RESTRICT_ON_SEND = %i[select filter].freeze

def_node_matcher :bad_method?, <<~PATTERN
(send nil? :bad_method ...)
PATTERN

def on_send(node)
return if (first_argument = node.first_argument) && !first_argument.block_pass_type?
return unless (send_node = map_method_candidate(node))
return unless send_node.method?(:map)

map_method = send_node.parent&.block_type? ? send_node.parent : send_node

range = offense_range(node, map_method)
add_offense(range, message: format(MSG, method_name: node.method_name))
end

private

def map_method_candidate(node)
return unless (parent = node.parent)

if parent.block_type? && parent.parent&.send_type?
parent.parent
elsif parent.send_type?
parent
end
end

def offense_range(node, map_method)
range_between(node.loc.selector.begin_pos, map_method.loc.expression.end_pos)
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/performance_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
require_relative 'performance/detect'
require_relative 'performance/double_start_end_with'
require_relative 'performance/end_with'
require_relative 'performance/filter_map'
require_relative 'performance/fixed_size'
require_relative 'performance/flat_map'
require_relative 'performance/inefficient_hash_search'
Expand Down
110 changes: 110 additions & 0 deletions spec/rubocop/cop/performance/filter_map_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Performance::FilterMap, :config do
context 'TargetRubyVersion >= 2.7', :ruby27 do
it 'registers an offense when using `select(&:...).map(&:...)`' do
expect_offense(<<~RUBY)
ary.select(&:present?).map(&:to_i)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead of `select.map`.
RUBY
end

it 'registers an offense when using `filter(&:...).map(&:...)`' do
expect_offense(<<~RUBY)
ary.filter(&:present?).map(&:to_i)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead of `filter.map`.
RUBY
end

it 'registers an offense when using `select { ... }.map { ... }`' do
expect_offense(<<~RUBY)
ary.select { |o| o.present? }.map { |o| o.to_i }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead of `select.map`.
RUBY
end

it 'registers an offense when using `select { ... }.map(&:...)`' do
expect_offense(<<~RUBY)
ary.select { |o| o.present? }.map(&:to_i)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead of `select.map`.
RUBY
end

it 'registers an offense when using `select(&:...).map { ... }`' do
expect_offense(<<~RUBY)
ary.select(&:present?).map { |o| o.to_i }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead of `select.map`.
RUBY
end

it 'registers an offense when using `select(&:...).map(&:...)` in method chain' do
expect_offense(<<~RUBY)
ary.do_something.select(&:present?).map(&:to_i).max
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead of `select.map`.
RUBY
end

it 'registers an offense when using `select(&:...).map(&:...)` without receiver' do
expect_offense(<<~RUBY)
select(&:present?).map(&:to_i)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead of `select.map`.
RUBY
end

it 'does not register an offense when using `filter_map`' do
expect_no_offenses(<<~RUBY)
ary.filter_map { |o| o.to_i if o.present? }
RUBY
end

it 'does not register an offense when using `select`' do
expect_no_offenses(<<~RUBY)
ary.select(&:present?)
RUBY
end

it 'does not register an offense when using `select(key: value).map`' do
expect_no_offenses(<<~RUBY)
ary.do_something.select(key: value).map(&:to_i)
RUBY
end

it 'does not register an offense when using `select` with assignment' do
expect_no_offenses(<<~RUBY)
ret = select
RUBY
end

it 'does not register an offense when using `select(&:...).stranger.map(&:...)`' do
expect_no_offenses(<<~RUBY)
ary.do_something.select(&:present?).stranger.map(&:to_i).max
RUBY
end

it 'does not register an offense when using `select { ... }.stranger.map { ... }`' do
expect_no_offenses(<<~RUBY)
ary.select { |o| o.present? }.stranger.map { |o| o.to_i }
RUBY
end

#
# `compact` is not compatible with `filter_map`.
#
# [true, false, nil].compact #=> [true, false]
# [true, false, nil].filter_map(&:itself) #=> [true]
#
it 'does not register an offense when using `map(&:...).compact`' do
expect_no_offenses(<<~RUBY)
ary.map(&:to_i).compact
RUBY
end
end

context 'TargetRubyVersion <= 2.6', :ruby26 do
it 'does not register an offense when using `select.map`' do
expect_no_offenses(<<~RUBY)
ary.select(&:present?).map(&:to_i)
RUBY
end
end
end

0 comments on commit 11d3c61

Please sign in to comment.