Skip to content
New issue

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

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

Already on GitHub? Sign in 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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants