Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Performance/FlatMap does not work with block_pass #70

Closed
splattael opened this issue Jul 12, 2019 · 0 comments · Fixed by #76
Closed

Performance/FlatMap does not work with block_pass #70

splattael opened this issue Jul 12, 2019 · 0 comments · Fixed by #76
Labels
bug Something isn't working

Comments

@splattael
Copy link

Expected behavior

Given a test file test.rb:

# frozen_string_literal: true

# 1.
p [1, 2, 3].map { |x| x.digits }.flatten(1)

# 2.
p [1, 2, 3].map(&:digits).flatten(1)

# 3.
digits = proc { |x| x.digits }
p [1, 2, 3].map(&digits).flatten(1)

I would expect RuboCop to suggest flat_map for 1., 2., and 3.

Output:

Inspecting 1 file
C

Offenses:

test.rb:3:13: C: Performance/FlatMap: Use flat_map instead of map...flatten.
p [1, 2, 3].map { |x| x.digits }.flatten(1)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test.rb:3:17: C: Style/SymbolProc: Pass &:digits as an argument to map instead of a block.
p [1, 2, 3].map { |x| x.digits }.flatten(1)
                ^^^^^^^^^^^^^^^^
test.rb:4:13: C: Performance/FlatMap: Use flat_map instead of map...flatten.
p [1, 2, 3].map(&:digits).flatten(1)
            ^^^^^^^^^^^^^^^^^^^^^^^^
test.rb:7:13: C: Performance/FlatMap: Use flat_map instead of map...flatten.
p [1, 2, 3].map(&digits).flatten(1)
            ^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 4 offenses detected

Actual behavior

RuboCop only suggests flat_map for 1.

Output:

Inspecting 1 file
C

Offenses:

test.rb:3:13: C: Performance/FlatMap: Use flat_map instead of map...flatten.
p [1, 2, 3].map { |x| x.digits }.flatten(1)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test.rb:3:17: C: Style/SymbolProc: Pass &:digits as an argument to map instead of a block.
p [1, 2, 3].map { |x| x.digits }.flatten(1)
                ^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected

Steps to reproduce the problem

Run rubocop -C false test.rb

RuboCop version

rubocop -V
0.72.0 (using Parser 2.6.3.0, running on ruby 2.6.3 x86_64-linux)

Suggested solultion

This patch below works as expected.

diff --git a/lib/rubocop/cop/performance/flat_map.rb b/lib/rubocop/cop/performance/flat_map.rb
index c90932f71..b5b91876a 100644
--- a/lib/rubocop/cop/performance/flat_map.rb
+++ b/lib/rubocop/cop/performance/flat_map.rb
@@ -23,7 +23,14 @@ module RuboCop
                                   'multiple levels.'

         def_node_matcher :flat_map_candidate?, <<-PATTERN
-          (send (block $(send _ ${:collect :map}) ...) ${:flatten :flatten!} $...)
+          (send
+            {
+              (block $(send _ ${:collect :map}) ...)
+              $(send _ ${:collect :map} (block_pass _))
+            }
+            ${:flatten :flatten!}
+            $...
+          )
         PATTERN

         def on_send(node)
@koic koic added the bug Something isn't working label Jul 12, 2019
koic added a commit to koic/rubocop-performance that referenced this issue Sep 8, 2019
Fixes rubocop#70.

This PR fixes a false negative for `Performance/FlatMap`
when using symbol to proc operator argument of `map` method.
@koic koic closed this as completed in #76 Oct 1, 2019
koic added a commit that referenced this issue Oct 1, 2019
…lat_map

[Fix #70] Fix a false negative for `Performance/FlatMap`
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants