Add new Performance/CollectionLiteralInLoop cop (#140)

Co-authored-by: Bozhidar Batsov <[email protected]>

.rubocop.yml

@@ -88,6 +88,10 @@ Performance/Caller:
 Performance/ChainArrayAllocation:
   Enabled: false
 
+Performance/CollectionLiteralInLoop:
+  Exclude:
+    - 'spec/**/*.rb'
+
 RSpec/PredicateMatcher:
   EnforcedStyle: explicit
 

CHANGELOG.md

@@ -18,6 +18,7 @@
 
 ### New features
 
+* [#140](https://github.com/rubocop-hq/rubocop-performance/pull/140): Add new `Performance/CollectionLiteralInLoop` cop. ([@fatkodima][])
 * [#137](https://github.com/rubocop-hq/rubocop-performance/pull/137): Add new `Performance/Sum` cop. ([@fatkodima][])
 * [#141](https://github.com/rubocop-hq/rubocop-performance/pull/141): Add new `Performance/RedundantStringChars` cop. ([@fatkodima][])
 * [#127](https://github.com/rubocop-hq/rubocop-performance/pull/127): Add new `Performance/IoReadlines` cop. ([@fatkodima][])

config/default.yml

@@ -49,6 +49,13 @@ Performance/ChainArrayAllocation:
   Enabled: false
   VersionAdded: '0.59'
 
+Performance/CollectionLiteralInLoop:
+  Description: 'Extract Array and Hash literals outside of loops into local variables or constants.'
+  Enabled: true
+  VersionAdded: '1.7'
+  # Min number of elements to consider an offense
+  MinSize: 1
+
 Performance/CompareWithBlock:
   Description: 'Use `sort_by(&:foo)` instead of `sort { |a, b| a.foo <=> b.foo }`.'
   Enabled: true

docs/modules/ROOT/pages/cops.adoc

@@ -9,6 +9,7 @@
 * xref:cops_performance.adoc#performancecasewhensplat[Performance/CaseWhenSplat]
 * xref:cops_performance.adoc#performancecasecmp[Performance/Casecmp]
 * xref:cops_performance.adoc#performancechainarrayallocation[Performance/ChainArrayAllocation]
+* xref:cops_performance.adoc#performancecollectionliteralinloop[Performance/CollectionLiteralInLoop]
 * xref:cops_performance.adoc#performancecomparewithblock[Performance/CompareWithBlock]
 * xref:cops_performance.adoc#performancecount[Performance/Count]
 * xref:cops_performance.adoc#performancedeleteprefix[Performance/DeletePrefix]

docs/modules/ROOT/pages/cops_performance.adoc

@@ -279,6 +279,58 @@ array
 
 * https://twitter.com/schneems/status/1034123879978029057
 
+== Performance/CollectionLiteralInLoop
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Enabled
+| Yes
+| No
+| 1.7
+| -
+|===
+
+This cop identifies places where Array and Hash literals are used
+within loops. It is better to extract them into a local variable or constant
+to avoid unnecessary allocations on each iteration.
+
+You can set the minimum number of elements to consider
+an offense with `MinSize`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+users.select do |user|
+  %i[superadmin admin].include?(user.role)
+end
+
+# good
+admin_roles = %i[superadmin admin]
+users.select do |user|
+  admin_roles.include?(user.role)
+end
+
+# good
+ADMIN_ROLES = %i[superadmin admin]
+...
+users.select do |user|
+  ADMIN_ROLES.include?(user.role)
+end
+----
+
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| MinSize
+| `1`
+| Integer
+|===
+
 == Performance/CompareWithBlock
 
 |===

lib/rubocop/cop/performance/collection_literal_in_loop.rb

@@ -0,0 +1,140 @@
+# frozen_string_literal: true
+
+require 'set'
+
+module RuboCop
+  module Cop
+    module Performance
+      # This cop identifies places where Array and Hash literals are used
+      # within loops. It is better to extract them into a local variable or constant
+      # to avoid unnecessary allocations on each iteration.
+      #
+      # You can set the minimum number of elements to consider
+      # an offense with `MinSize`.
+      #
+      # @example
+      #   # bad
+      #   users.select do |user|
+      #     %i[superadmin admin].include?(user.role)
+      #   end
+      #
+      #   # good
+      #   admin_roles = %i[superadmin admin]
+      #   users.select do |user|
+      #     admin_roles.include?(user.role)
+      #   end
+      #
+      #   # good
+      #   ADMIN_ROLES = %i[superadmin admin]
+      #   ...
+      #   users.select do |user|
+      #     ADMIN_ROLES.include?(user.role)
+      #   end
+      #
+      class CollectionLiteralInLoop < Cop
+        MSG = 'Avoid immutable %<literal_class>s literals in loops. '\
+          'It is better to extract it into a local variable or a constant.'
+
+        POST_CONDITION_LOOP_TYPES = %i[while_post until_post].freeze
+        LOOP_TYPES = (POST_CONDITION_LOOP_TYPES + %i[while until for]).freeze
+
+        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_node_matcher :kernel_loop?, <<~PATTERN
+          (block
+            (send {nil? (const nil? :Kernel)} :loop)
+            ...)
+        PATTERN
+
+        def_node_matcher :enumerable_loop?, <<~PATTERN
+          (block
+            (send $_ #enumerable_method? ...)
+            ...)
+        PATTERN
+
+        def on_send(node)
+          receiver, method, = *node.children
+          return unless check_literal?(receiver, method) && parent_is_loop?(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_loop?(node)
+          node.each_ancestor.any? { |ancestor| loop?(ancestor, node) }
+        end
+
+        def loop?(ancestor, node)
+          keyword_loop?(ancestor.type) ||
+            kernel_loop?(ancestor) ||
+            node_within_enumerable_loop?(node, ancestor)
+        end
+
+        def keyword_loop?(type)
+          LOOP_TYPES.include?(type)
+        end
+
+        def node_within_enumerable_loop?(node, ancestor)
+          enumerable_loop?(ancestor) do |receiver|
+            receiver != node && !receiver.descendants.include?(node)
+          end
+        end
+
+        def literal_class(node)
+          if node.array_type?
+            'Array'
+          elsif node.hash_type?
+            'Hash'
+          end
+        end
+
+        def enumerable_method?(method_name)
+          ENUMERABLE_METHOD_NAMES.include?(method_name)
+        end
+
+        def min_size
+          Integer(cop_config['MinSize'] || 1)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/performance_cops.rb

@@ -9,6 +9,7 @@
 require_relative 'performance/caller'
 require_relative 'performance/case_when_splat'
 require_relative 'performance/casecmp'
+require_relative 'performance/collection_literal_in_loop.rb'
 require_relative 'performance/compare_with_block'
 require_relative 'performance/count'
 require_relative 'performance/delete_prefix'