Skip to content

Commit

Permalink
Merge pull request #76 from koic/fix_false_negative_for_performance_f…
Browse files Browse the repository at this point in the history
…lat_map

[Fix #70] Fix a false negative for `Performance/FlatMap`
  • Loading branch information
koic committed Oct 1, 2019
2 parents b4b6531 + 7f00956 commit 74b3186
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Bug fixes

* [#74](https://github.com/rubocop-hq/rubocop-performance/pull/74): Fix an error for `Performance/RedundantMerge` when `MaxKeyValuePairs` option is set to `null`. ([@koic][])
* [#70](https://github.com/rubocop-hq/rubocop-performance/issues/70): This PR fixes a false negative for `Performance/FlatMap` when using symbol to proc operator argument of `map` method. ([@koic][], [@splattael][])

### Changes

Expand Down Expand Up @@ -59,3 +60,4 @@
[@dduugg]: https://github.com/dduugg
[@tejasbubane]: https://github.com/tejasbubane
[@rrosenblum]: https://github.com/rrosenblum
[@splattael]: https://github.com/splattael
9 changes: 8 additions & 1 deletion lib/rubocop/cop/performance/flat_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,14 @@ class FlatMap < Cop
'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)
Expand Down
30 changes: 30 additions & 0 deletions spec/rubocop/cop/performance/flat_map_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,22 @@
expect(cop.highlights).to eq(["#{method} { |e| [e, e] }.#{flatten}(1)"])
end

it "registers an offense when calling #{method}(&:foo).#{flatten}(1)" do
inspect_source("[1, 2, 3, 4].#{method}(&:foo).#{flatten}(1)")

expect(cop.messages)
.to eq(["Use `flat_map` instead of `#{method}...#{flatten}`."])
expect(cop.highlights).to eq(["#{method}(&:foo).#{flatten}(1)"])
end

it "registers an offense when calling #{method}(&foo).#{flatten}(1)" do
inspect_source("[1, 2, 3, 4].#{method}(&foo).#{flatten}(1)")

expect(cop.messages)
.to eq(["Use `flat_map` instead of `#{method}...#{flatten}`."])
expect(cop.highlights).to eq(["#{method}(&foo).#{flatten}(1)"])
end

it "does not register an offense when calling #{method}...#{flatten} " \
'with a number greater than 1' do
expect_no_offenses("[1, 2, 3, 4].#{method} { |e| [e, e] }.#{flatten}(3)")
Expand All @@ -27,6 +43,20 @@

expect(new_source).to eq('[1, 2].flat_map { |e| [e, e] }')
end

it "corrects #{method}(&:foo).#{flatten} to flat_map" do
source = "[1, 2].#{method}(&:foo).#{flatten}(1)"
new_source = autocorrect_source(source)

expect(new_source).to eq('[1, 2].flat_map(&:foo)')
end

it "corrects #{method}(&foo).#{flatten} to flat_map" do
source = "[1, 2].#{method}(&:foo).#{flatten}(1)"
new_source = autocorrect_source(source)

expect(new_source).to eq('[1, 2].flat_map(&:foo)')
end
end

describe 'configured to only warn when flattening one level' do
Expand Down

0 comments on commit 74b3186

Please sign in to comment.