Add new Performance/CollectionLiteralInMethod cop

changelog/new_collection_literal_in_method.md

@@ -0,0 +1 @@
+* [#303](https://github.com/rubocop/rubocop-performance/pull/303): Add new Performance/CollectionLiteralInMethod cop. ([@exoego][])

config/default.yml

@@ -74,6 +74,12 @@ Performance/CollectionLiteralInLoop:
   # Min number of elements to consider an offense
   MinSize: 1
 
+Performance/CollectionLiteralInMethod:
+  Description: 'Extract Array and Hash literals outside of methods into class variables or constants.'
+  Enabled: 'pending'
+  VersionAdded: '1.15'
+  MinSize: 1
+
 Performance/CompareWithBlock:
   Description: 'Use `sort_by(&:foo)` instead of `sort { |a, b| a.foo <=> b.foo }`.'
   Enabled: true

lib/rubocop/cop/performance/collection_literal_in_method.rb

@@ -0,0 +1,111 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Performance
+      # Identifies places where Array and Hash literals are used within method.
+      # It is better to extract them into a constant to avoid unnecessary allocations
+      # on each iteration.
+      #
+      # You can set the minimum number of elements to consider
+      # an offense with `MinSize`.
+      #
+      # @example
+      #   # bad
+      #   def fallback_data(config)
+      #     {
+      #       foo: 'bar',
+      #       sit: 'amet',
+      #     }.merge(config)
+      #   end
+      #
+      #   # good
+      #   FALLBACK_DATA = {
+      #     foo: 'bar',
+      #     sit: 'amet',
+      #   }.freeze
+      #
+      #   def fallback_data(config)
+      #     FALLBACK_DATA.merge(config)
+      #   end
+      #
+      class CollectionLiteralInMethod < Base
+        MSG = 'Avoid immutable %<literal_class>s literals in method definition. ' \
+              'It is better to extract it into a constant.'
+
+        ENUMERABLE_METHOD_NAMES = (Enumerable.instance_methods + [:each]).to_set.freeze
+        NONMUTATING_ARRAY_METHODS = %i[& * + - <=> == [] all? any? assoc at
+                                       bsearch bsearch_index collect combination
+                                       compact count cycle deconstruct difference dig
+                                       drop drop_while each each_index empty? eql?
+                                       fetch filter find_index first flatten hash
+                                       include? index inspect intersection join
+                                       last length map max min minmax none? one? pack
+                                       permutation product rassoc reject
+                                       repeated_combination repeated_permutation reverse
+                                       reverse_each rindex rotate sample select shuffle
+                                       size slice sort sum take take_while
+                                       to_a to_ary to_h to_s transpose union uniq
+                                       values_at zip |].freeze
+
+        ARRAY_METHODS = (ENUMERABLE_METHOD_NAMES | NONMUTATING_ARRAY_METHODS).to_set.freeze
+
+        NONMUTATING_HASH_METHODS = %i[< <= == > >= [] any? assoc compact dig
+                                      each each_key each_pair each_value empty?
+                                      eql? fetch fetch_values filter flatten has_key?
+                                      has_value? hash include? inspect invert key key?
+                                      keys? length member? merge rassoc rehash reject
+                                      select size slice to_a to_h to_hash to_proc to_s
+                                      transform_keys transform_values value? values values_at].freeze
+
+        HASH_METHODS = (ENUMERABLE_METHOD_NAMES | NONMUTATING_HASH_METHODS).to_set.freeze
+
+        def on_send(node)
+          receiver, method, = *node.children
+          return unless check_literal?(receiver, method) && parent_is_method?(receiver)
+
+          message = format(MSG, literal_class: literal_class(receiver))
+          add_offense(receiver, message: message)
+        end
+
+        private
+
+        def check_literal?(node, method)
+          !node.nil? &&
+            nonmutable_method_of_array_or_hash?(node, method) &&
+            node.children.size >= min_size &&
+            node.recursive_basic_literal?
+        end
+
+        def nonmutable_method_of_array_or_hash?(node, method)
+          (node.array_type? && ARRAY_METHODS.include?(method)) ||
+            (node.hash_type? && HASH_METHODS.include?(method))
+        end
+
+        def parent_is_method?(node)
+          node.each_ancestor.any? { |ancestor| method?(ancestor) || singleton_method?(ancestor) }
+        end
+
+        def method?(ancestor)
+          ancestor.def_type?
+        end
+
+        def singleton_method?(ancestor)
+          ancestor.defs_type?
+        end
+
+        def literal_class(node)
+          if node.array_type?
+            'Array'
+          elsif node.hash_type?
+            'Hash'
+          end
+        end
+
+        def min_size
+          Integer(cop_config['MinSize'] || 1)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/performance/compare_with_block.rb

@@ -3,6 +3,8 @@
 module RuboCop
   module Cop
     module Performance
+      KEY_TYPES = %i[sym str int].freeze
+
       # Identifies places where `sort { |a, b| a.foo <=> b.foo }`
       # can be replaced by `sort_by(&:foo)`.
       # This cop also checks `max` and `min` methods.
@@ -73,7 +75,7 @@ def slow_compare?(method, args_a, args_b)
             return false unless args_a.size == 1
 
             key = args_a.first
-            return false unless %i[sym str int].include?(key.type)
+            return false unless KEY_TYPES.include?(key.type)
           else
             return false unless args_a.empty?
           end

lib/rubocop/cop/performance/regexp_match.rb

@@ -80,7 +80,8 @@ class RegexpMatch < Base
 
         # Constants are included in this list because it is unlikely that
         # someone will store `nil` as a constant and then use it for comparison
-        TYPES_IMPLEMENTING_MATCH = %i[const regexp str sym].freeze
+        TYPES_IMPLEMENTING_MATCH = %i[const regexp str sym].to_set.freeze
+        GVAR_SYMS = %i[$~ $MATCH $PREMATCH $POSTMATCH $LAST_PAREN_MATCH $LAST_MATCH_INFO].to_set.freeze
         MSG = 'Use `match?` instead of `%<current>s` when `MatchData` is not used.'
 
         def_node_matcher :match_method?, <<~PATTERN
@@ -240,7 +241,7 @@ def scope_root(node)
         end
 
         def match_gvar?(sym)
-          %i[$~ $MATCH $PREMATCH $POSTMATCH $LAST_PAREN_MATCH $LAST_MATCH_INFO].include?(sym)
+          GVAR_SYMS.include?(sym)
         end
 
         def correct_operator(corrector, recv, arg, oper = nil)

lib/rubocop/cop/performance_cops.rb

@@ -12,6 +12,7 @@
 require_relative 'performance/case_when_splat'
 require_relative 'performance/casecmp'
 require_relative 'performance/collection_literal_in_loop'
+require_relative 'performance/collection_literal_in_method'
 require_relative 'performance/compare_with_block'
 require_relative 'performance/concurrent_monotonic_time'
 require_relative 'performance/constant_regexp'

spec/rubocop/cop/performance/collection_literal_in_method_spec.rb

@@ -0,0 +1,181 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Performance::CollectionLiteralInMethod, :config do
+  let(:cop_config) do
+    { 'MinSize' => 1 }
+  end
+
+  context 'when inside `def` method definition' do
+    it 'registers an offense when using Array literal' do
+      expect_offense(<<~RUBY)
+        def foo(e)
+          [1, 2, 3].include?(e)
+          ^^^^^^^^^ Avoid immutable Array literals in method definition. It is better to extract it into a constant.
+        end
+      RUBY
+    end
+
+    it 'registers an offense when using Hash literal' do
+      expect_offense(<<~RUBY)
+        def method
+          { foo: :bar }.key?(:foo)
+          ^^^^^^^^^^^^^ Avoid immutable Hash literals in method definition. It is better to extract it into a constant.
+        end
+      RUBY
+    end
+  end
+
+  context 'when inside singleton method definition' do
+    it 'registers an offense when using Array literal' do
+      expect_offense(<<~RUBY)
+        def bar.foo(e)
+          [1, 2, 3].include?(e)
+          ^^^^^^^^^ Avoid immutable Array literals in method definition. It is better to extract it into a constant.
+        end
+      RUBY
+    end
+
+    it 'registers an offense when using Hash literal' do
+      expect_offense(<<~RUBY)
+        def Bar.method
+          { foo: :bar }.key?(:foo)
+          ^^^^^^^^^^^^^ Avoid immutable Hash literals in method definition. It is better to extract it into a constant.
+        end
+      RUBY
+    end
+  end
+
+  context 'when inside loop inside `def` method definition' do
+    it 'registers an offense when using Array literal' do
+      expect_offense(<<~RUBY)
+        def foo(e)
+          while i < 100
+            [1, 2, 3].include?(e)
+            ^^^^^^^^^ Avoid immutable Array literals in method definition. It is better to extract it into a constant.
+          end
+        end
+      RUBY
+    end
+
+    it 'registers an offense when using Hash literal' do
+      expect_offense(<<~RUBY)
+        def method
+          while i < 100
+            { foo: :bar }.key?(:foo)
+            ^^^^^^^^^^^^^ Avoid immutable Hash literals in method definition. It is better to extract it into a constant.
+          end
+        end
+      RUBY
+    end
+  end
+
+  context 'when not inside any type of method definition' do
+    it 'does not register an offense when using Array literal' do
+      expect_no_offenses(<<~RUBY)
+        module Foo
+          [1, 2, 3].include?(e)
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when using Hash literal' do
+      expect_no_offenses(<<~RUBY)
+        class Foo
+          { foo: :bar }.key?(:foo)
+        end
+      RUBY
+    end
+  end
+
+  context 'when literal contains element of non basic type' do
+    it 'does not register an offense when using Array literal' do
+      expect_no_offenses(<<~RUBY)
+        def method(e)
+          [1, 2, variable].include?(e)
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when using Hash literal' do
+      expect_no_offenses(<<~RUBY)
+        def method(e)
+          { foo: { bar: variable } }.key?(:foo)
+        end
+      RUBY
+    end
+  end
+
+  context 'when destructive method is called' do
+    it 'does not register an offense when using Array literal' do
+      expect_no_offenses(<<~RUBY)
+        def method
+          [1, nil, 3].compact!
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when using Hash literal' do
+      expect_no_offenses(<<~RUBY)
+        def method
+          { foo: :bar, baz: nil }.select! { |_k, v| !v.nil? }
+        end
+      RUBY
+    end
+  end
+
+  context 'when none method is called' do
+    it 'does not register an offense when using Array literal' do
+      expect_no_offenses(<<~RUBY)
+        def method
+          array = [1, nil, 3]
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when using Hash literal' do
+      expect_no_offenses(<<~RUBY)
+        def method()
+          hash = { foo: :bar, baz: nil }
+        end
+      RUBY
+    end
+  end
+
+  it 'does not register an offense when there are no literals in a def' do
+    expect_no_offenses(<<~RUBY)
+      def foo(x)
+        puts x
+      end
+    RUBY
+  end
+
+  it 'does not register an offense when nondestructive method is called on nonliteral' do
+    expect_no_offenses(<<~RUBY)
+      def bar(array)
+        array.all? { |x| x > 100 }
+      end
+    RUBY
+  end
+
+  context 'with MinSize of 2' do
+    let(:cop_config) do
+      { 'MinSize' => 2 }
+    end
+
+    it 'does not register an offense when using Array literal' do
+      expect_no_offenses(<<~RUBY)
+        def foo(e)
+          [1].include?(e)
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when using Hash literal' do
+      expect_no_offenses(<<~RUBY)
+        def foo(bar)
+          { foo: :bar }.key?(bar)
+        end
+      RUBY
+    end
+  end
+end