Merge pull request #2583 from DNNX/times-map

New Performance/TimesMap cop

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### New features
 
+* [#2583](https://github.com/bbatsov/rubocop/pull/2583): New cop `Performance/TimesMap` checks for `x.times.map{}` and suggests replacing them with `Array.new(x){}`. ([@DNNX][])
 * [#2529](https://github.com/bbatsov/rubocop/pull/2529): Add EnforcedStyle config parameter to IndentArray. ([@jawshooah][])
 * [#2479](https://github.com/bbatsov/rubocop/pull/2479): Add option `AllowHeredoc` to `Metrics/LineLength`. ([@fphilipe][])
 * [#2416](https://github.com/bbatsov/rubocop/pull/2416): New cop `Style/ConditionalAssignment` checks for assignment of the same variable in all branches of conditionals and replaces them with a single assignment to the return of the conditional. ([@rrosenblum][])
@@ -1862,3 +1863,4 @@
 [@weh]: https://github.com/weh
 [@bfontaine]: https://github.com/bfontaine
 [@jawshooah]: https://github.com/jawshooah
+[@DNNX]: https://github.com/DNNX

config/enabled.yml

@@ -1242,6 +1242,10 @@ Performance/StringReplacement:
   Reference: 'https://github.com/JuanitoFatas/fast-ruby#stringgsub-vs-stringtr-code'
   Enabled: true
 
+Performance/TimesMap:
+  Description: 'Checks for .times.map calls.'
+  Enabled: true
+
 ##################### Rails ##################################
 
 Rails/ActionFilter:

lib/rubocop.rb

@@ -156,6 +156,7 @@
 require 'rubocop/cop/performance/size'
 require 'rubocop/cop/performance/start_with'
 require 'rubocop/cop/performance/string_replacement'
+require 'rubocop/cop/performance/times_map'
 
 require 'rubocop/cop/style/access_modifier_indentation'
 require 'rubocop/cop/style/accessor_method_name'

lib/rubocop/cop/performance/times_map.rb

@@ -0,0 +1,48 @@
+# encoding: utf-8
+
+module RuboCop
+  module Cop
+    module Performance
+      # This cop checks for .times.map calls.
+      # In most cases such calls can be replaced
+      # with an explicit array creation.
+      #
+      # @example
+      #
+      #   @bad
+      #   9.times.map do |i|
+      #     i.to_s
+      #   end
+      #
+      #   @good
+      #   Array.new(9) do |i|
+      #     i.to_s
+      #   end
+      class TimesMap < Cop
+        MSG = 'Use `Array.new` with a block instead of `.times.%s`.'
+
+        def on_send(node)
+          check(node)
+        end
+
+        def on_block(node)
+          check(node)
+        end
+
+        private
+
+        def check(node)
+          map_or_collect = times_map_call(node)
+          if map_or_collect
+            add_offense(node, :expression, format(MSG, map_or_collect))
+          end
+        end
+
+        def_node_matcher :times_map_call, <<-END
+          {(block (send (send _ :times) ${:map :collect}) ...)
+           (send (send _ :times) ${:map :collect} (block_pass ...))}
+        END
+      end
+    end
+  end
+end

spec/rubocop/cop/performance/times_map_spec.rb

@@ -0,0 +1,79 @@
+# encoding: utf-8
+
+require 'spec_helper'
+
+describe RuboCop::Cop::Performance::TimesMap do
+  subject(:cop) { described_class.new }
+
+  before do
+    inspect_source(cop, source)
+  end
+
+  context '.times.map' do
+    context 'with a block' do
+      let(:source) { '4.times.map { |i| i.to_s }' }
+
+      it 'registers an offense' do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.first.message).to eq(
+          'Use `Array.new` with a block instead of `.times.map`.'
+        )
+        expect(cop.highlights).to eq(['4.times.map { |i| i.to_s }'])
+      end
+    end
+
+    context 'with an explicitly passed block' do
+      let(:source) { '4.times.map(&method(:foo))' }
+
+      it 'registers an offense' do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.first.message).to eq(
+          'Use `Array.new` with a block instead of `.times.map`.'
+        )
+        expect(cop.highlights).to eq(['4.times.map(&method(:foo))'])
+      end
+    end
+
+    context 'without a block' do
+      let(:source) { '4.times.map' }
+
+      it "doesn't register an offense" do
+        expect(cop.offenses).to be_empty
+      end
+    end
+  end
+
+  context '.times.collect' do
+    context 'with a block' do
+      let(:source) { '4.times.collect { |i| i.to_s }' }
+
+      it 'registers an offense' do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.first.message).to eq(
+          'Use `Array.new` with a block instead of `.times.collect`.'
+        )
+        expect(cop.highlights).to eq(['4.times.collect { |i| i.to_s }'])
+      end
+    end
+
+    context 'with an explicitly passed block' do
+      let(:source) { '4.times.collect(&method(:foo))' }
+
+      it 'registers an offense' do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.first.message).to eq(
+          'Use `Array.new` with a block instead of `.times.collect`.'
+        )
+        expect(cop.highlights).to eq(['4.times.collect(&method(:foo))'])
+      end
+    end
+
+    context 'without a block' do
+      let(:source) { '4.times.collect' }
+
+      it "doesn't register an offense" do
+        expect(cop.offenses).to be_empty
+      end
+    end
+  end
+end

spec/rubocop/formatter/progress_formatter_spec.rb

@@ -97,7 +97,7 @@ def offense_with_severity(severity)
       context 'when any offenses are detected' do
         before do
           source_buffer = Parser::Source::Buffer.new('test', 1)
-          source = 9.times.map do |index|
+          source = Array.new(9) do |index|
             "This is line #{index + 1}."
           end
           source_buffer.source = source.join("\n")