Merge pull request #277 from koic/fix_incorrect_autocorrect_for_perfo…

…rmance_map_compact Fix an incorrect autocorrect for `Performance/MapCompact`
rubocop · Dec 10, 2021 · b9c1f05 · b9c1f05
2 parents 1d7f986 + f45a323
commit b9c1f05
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 4 deletions.
diff --git a/changelog/fix_incorrect_autocorrect_for_performance_map_compact.md b/changelog/fix_incorrect_autocorrect_for_performance_map_compact.md
@@ -0,0 +1 @@
+* [#277](https://github.com/rubocop/rubocop-performance/pull/277): Fix an incorrect autocorrect for `Performance/MapCompact` when using `map.compact.first` and there is a line break after `map.compact` and receiver. ([@koic][])
diff --git a/lib/rubocop/cop/performance/map_compact.rb b/lib/rubocop/cop/performance/map_compact.rb
@@ -58,29 +58,41 @@ def on_send(node)
 
           add_offense(range) do |corrector|
             corrector.replace(map_node.loc.selector, 'filter_map')
-            remove_compact_method(corrector, node)
+            remove_compact_method(corrector, node, node.parent)
           end
         end
 
         private
 
-        def remove_compact_method(corrector, compact_node)
-          chained_method = compact_node.parent
+        def remove_compact_method(corrector, 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) &&
              !invoke_method_after_map_compact_on_same_line?(compact_node, chained_method)
-            compact_method_range = range_by_whole_lines(compact_method_range, include_final_newline: true)
+            compact_method_range = compact_method_with_final_newline_range(compact_method_range)
           else
             corrector.remove(compact_node.loc.dot)
           end
 
           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
+
+          compact_node.loc.selector.line == map_node.loc.selector.line
+        end
+
         def invoke_method_after_map_compact_on_same_line?(compact_node, chained_method)
           compact_node.loc.selector.line == chained_method.loc.selector.line
         end
+
+        def compact_method_with_final_newline_range(compact_method_range)
+          range_by_whole_lines(compact_method_range, include_final_newline: true)
+        end
       end
     end
   end

diff --git a/spec/rubocop/cop/performance/map_compact_spec.rb b/spec/rubocop/cop/performance/map_compact_spec.rb
@@ -102,6 +102,22 @@
       RUBY
     end
 
+    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
+          .map { |item| item.do_something }.compact
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `filter_map` instead.
+          .first
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection
+          .filter_map { |item| item.do_something }
+          .first
+      RUBY
+    end
+
     it 'registers an offense when using `map { ... }.compact`' do
       expect_offense(<<~RUBY)
         collection.map { |item|