Merge pull request #285 from ydah/fix-map-compact-error

Fix an error for `Performance/MapCompact` when using `map(&:do_something).compact.first` and there is a line break after `map.compact` and receiver

CHANGELOG.md

@@ -344,3 +344,4 @@
 [@mfbmina]: https://github.com/mfbmina
 [@mvz]: https://github.com/mvz
 [@leoarnold]: https://github.com/leoarnold
+[@ydah]: https://github.com/ydah

changelog/fix_fix_error_for_performance_mapcorrect_when_line_break.md

@@ -0,0 +1 @@
+* [#285](https://github.com/rubocop/rubocop-performance/pull/285): Fix an error for `Performance/MapCompact` when using `map(&:do_something).compact.first` and there is a line break after `map.compact` and receiver. ([@ydah][])

lib/rubocop/cop/performance/map_compact.rb

@@ -58,17 +58,17 @@ def on_send(node)
 
           add_offense(range) do |corrector|
             corrector.replace(map_node.loc.selector, 'filter_map')
-            remove_compact_method(corrector, node, node.parent)
+            remove_compact_method(corrector, map_node, node, node.parent)
           end
         end
 
         private
 
-        def remove_compact_method(corrector, compact_node, chained_method)
+        def remove_compact_method(corrector, map_node, compact_node, chained_method)
           compact_method_range = compact_node.loc.selector
 
           if compact_node.multiline? && chained_method&.loc.respond_to?(:selector) && chained_method.dot? &&
-             !map_method_and_compact_method_on_same_line?(compact_node) &&
+             !map_method_and_compact_method_on_same_line?(map_node, compact_node) &&
              !invoke_method_after_map_compact_on_same_line?(compact_node, chained_method)
             compact_method_range = compact_method_with_final_newline_range(compact_method_range)
           else
@@ -78,11 +78,7 @@ def remove_compact_method(corrector, compact_node, chained_method)
           corrector.remove(compact_method_range)
         end
 
-        def map_method_and_compact_method_on_same_line?(compact_node)
-          return false unless compact_node.children.first.respond_to?(:send_node)
-
-          map_node = compact_node.children.first.send_node
-
+        def map_method_and_compact_method_on_same_line?(map_node, compact_node)
           compact_node.loc.selector.line == map_node.loc.selector.line
         end
 

spec/rubocop/cop/performance/map_compact_spec.rb

@@ -46,7 +46,81 @@
       RUBY
     end
 
-    it 'registers an offense when using `map.compact.first` with single-line method calls' do
+    it 'registers an offense when using `map(&:do_something).compact.first` with single-line method calls' do
+      expect_offense(<<~RUBY)
+        collection.map(&:do_something).compact.first
+                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection.filter_map(&:do_something).first
+      RUBY
+    end
+
+    it 'registers an offense when using `map(&:do_something).compact.first` with multi-line leading dot method calls' do
+      expect_offense(<<~RUBY)
+        collection
+          .map(&:do_something)
+           ^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead.
+          .compact
+          .first
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection
+          .filter_map(&:do_something)
+          .first
+      RUBY
+    end
+
+    it 'registers an offense when using `map(&:do_something).compact.first` with multi-line trailing' \
+       'dot method calls' do
+      expect_offense(<<~RUBY)
+        collection.
+          map(&:do_something).
+          ^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead.
+          compact.
+          first
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection.
+          filter_map(&:do_something).
+          first
+      RUBY
+    end
+
+    it 'registers an offense when using `map(&:do_something).compact.first` and there is a line break after' \
+       '`map.compact`' do
+      expect_offense(<<~RUBY)
+        collection.map(&:do_something).compact
+                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead.
+          .first
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection.filter_map(&:do_something)
+          .first
+      RUBY
+    end
+
+    it 'registers an offense when using `map(&:do_something).compact.first` and there is a line break after' \
+       '`map.compact` and receiver' do
+      expect_offense(<<~RUBY)
+        collection
+          .map(&:do_something).compact
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead.
+          .first
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection
+          .filter_map(&:do_something)
+          .first
+      RUBY
+    end
+
+    it 'registers an offense when using `map { ... }.compact.first` with single-line method calls' do
       expect_offense(<<~RUBY)
         collection.map { |item| item.do_something }.compact.first
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead.
@@ -57,7 +131,7 @@
       RUBY
     end
 
-    it 'registers an offense when using `map.compact.first` with multi-line leading dot method calls' do
+    it 'registers an offense when using `map { ... }.compact.first` with multi-line leading dot method calls' do
       expect_offense(<<~RUBY)
         collection
           .map { |item| item.do_something }
@@ -73,7 +147,7 @@
       RUBY
     end
 
-    it 'registers an offense when using `map.compact.first` with multi-line trailing dot method calls' do
+    it 'registers an offense when using `map { ... }.compact.first` with multi-line trailing dot method calls' do
       expect_offense(<<~RUBY)
         collection.
           map { |item| item.do_something }.
@@ -89,7 +163,7 @@
       RUBY
     end
 
-    it 'registers an offense when using `map.compact.first` and there is a line break after `map.compact`' do
+    it 'registers an offense when using `map { ... }.compact.first` and there is a line break after `map.compact`' do
       expect_offense(<<~RUBY)
         collection.map { |item| item.do_something }.compact
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead.
@@ -102,7 +176,7 @@
       RUBY
     end
 
-    it 'registers an offense when using `map.compact.first` and there is a line break after `map.compact` ' \
+    it 'registers an offense when using `map { ... }.compact.first` and there is a line break after `map.compact` ' \
        'and receiver' do
       expect_offense(<<~RUBY)
         collection