From 7f00956dc57f6d74bb68720a918950fd7a5bce26 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 8 Sep 2019 22:42:31 +0900 Subject: [PATCH] [Fix #70] Fix a false negative for `Performance/FlatMap` Fixes #70. This PR fixes a false negative for `Performance/FlatMap` when using symbol to proc operator argument of `map` method. --- CHANGELOG.md | 2 ++ lib/rubocop/cop/performance/flat_map.rb | 9 +++++- spec/rubocop/cop/performance/flat_map_spec.rb | 30 +++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23c465fbdc..66964d6084 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -59,3 +60,4 @@ [@dduugg]: https://github.com/dduugg [@tejasbubane]: https://github.com/tejasbubane [@rrosenblum]: https://github.com/rrosenblum +[@splattael]: https://github.com/splattael diff --git a/lib/rubocop/cop/performance/flat_map.rb b/lib/rubocop/cop/performance/flat_map.rb index c90932f71d..b5b91876a7 100644 --- a/lib/rubocop/cop/performance/flat_map.rb +++ b/lib/rubocop/cop/performance/flat_map.rb @@ -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) diff --git a/spec/rubocop/cop/performance/flat_map_spec.rb b/spec/rubocop/cop/performance/flat_map_spec.rb index 235c80f9e3..e9242beefe 100644 --- a/spec/rubocop/cop/performance/flat_map_spec.rb +++ b/spec/rubocop/cop/performance/flat_map_spec.rb @@ -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)") @@ -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