Skip to content

Commit eb68f96

Browse files
authored
Merge pull request #488 from Earlopain/collection-literal-ruby34
Change `Performance/CollectionLiteralInLoop` to not register offenses for `Array#include?` that are optimized directly in Ruby.
2 parents 5fa5f1a + d196b53 commit eb68f96

File tree

3 files changed

+103
-4
lines changed

3 files changed

+103
-4
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#482](https://github.com/rubocop/rubocop-performance/issues/482): Change `Performance/CollectionLiteralInLoop` to not register offenses for `Array#include?` that are optimized directly in Ruby. ([@earlopain][])

lib/rubocop/cop/performance/collection_literal_in_loop.rb

+35-4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ module Performance
1212
# You can set the minimum number of elements to consider
1313
# an offense with `MinSize`.
1414
#
15+
# NOTE: Since Ruby 3.4, certain simple arguments to `Array#include?` are
16+
# optimized directly in Ruby. This avoids allocations without changing the
17+
# code, as such no offense will be registered in those cases. Currently that
18+
# includes: strings, `self`, local variables, instance variables, and method
19+
# calls without arguments. Additionally, any number of methods can be chained:
20+
# `[1, 2, 3].include?(@foo)` and `[1, 2, 3].include?(@foo.bar.baz)` both avoid
21+
# the array allocation.
22+
#
1523
# @example
1624
# # bad
1725
# users.select do |user|
@@ -55,6 +63,8 @@ class CollectionLiteralInLoop < Base
5563

5664
ARRAY_METHODS = (ENUMERABLE_METHOD_NAMES | NONMUTATING_ARRAY_METHODS).to_set.freeze
5765

66+
ARRAY_INCLUDE_OPTIMIZED_TYPES = %i[str self lvar ivar send].freeze
67+
5868
NONMUTATING_HASH_METHODS = %i[< <= == > >= [] any? assoc compact dig
5969
each each_key each_pair each_value empty?
6070
eql? fetch fetch_values filter flatten has_key?
@@ -80,21 +90,42 @@ class CollectionLiteralInLoop < Base
8090
PATTERN
8191

8292
def on_send(node)
83-
receiver, method, = *node.children
84-
return unless check_literal?(receiver, method) && parent_is_loop?(receiver)
93+
receiver, method, *arguments = *node.children
94+
return unless check_literal?(receiver, method, arguments) && parent_is_loop?(receiver)
8595

8696
message = format(MSG, literal_class: literal_class(receiver))
8797
add_offense(receiver, message: message)
8898
end
8999

90100
private
91101

92-
def check_literal?(node, method)
102+
def check_literal?(node, method, arguments)
93103
!node.nil? &&
94104
nonmutable_method_of_array_or_hash?(node, method) &&
95105
node.children.size >= min_size &&
96-
node.recursive_basic_literal?
106+
node.recursive_basic_literal? &&
107+
!optimized_array_include?(node, method, arguments)
108+
end
109+
110+
# Since Ruby 3.4, simple arguments to Array#include? are optimized.
111+
# See https://github.com/ruby/ruby/pull/12123 for more details.
112+
# rubocop:disable Metrics/CyclomaticComplexity
113+
def optimized_array_include?(node, method, arguments)
114+
return false unless target_ruby_version >= 3.4 && node.array_type? && method == :include?
115+
# Disallow include?(1, 2)
116+
return false if arguments.count != 1
117+
118+
arg = arguments.first
119+
# Allow `include?(foo.bar.baz.bat)`
120+
while arg.send_type?
121+
return false if arg.arguments.any? # Disallow include?(foo(bar))
122+
break unless arg.receiver
123+
124+
arg = arg.receiver
125+
end
126+
ARRAY_INCLUDE_OPTIMIZED_TYPES.include?(arg.type)
97127
end
128+
# rubocop:enable Metrics/CyclomaticComplexity
98129

99130
def nonmutable_method_of_array_or_hash?(node, method)
100131
(node.array_type? && ARRAY_METHODS.include?(method)) ||

spec/rubocop/cop/performance/collection_literal_in_loop_spec.rb

+67
Original file line numberDiff line numberDiff line change
@@ -269,4 +269,71 @@
269269
RUBY
270270
end
271271
end
272+
273+
context 'when Ruby >= 3.4', :ruby34 do
274+
it 'registers an offense for `include?` on a Hash literal' do
275+
expect_offense(<<~RUBY)
276+
each do
277+
{ foo: :bar }.include?(:foo)
278+
^^^^^^^^^^^^^ Avoid immutable Hash literals in loops. It is better to extract it into a local variable or a constant.
279+
end
280+
RUBY
281+
end
282+
283+
it 'registers an offense for other array methods' do
284+
expect_offense(<<~RUBY)
285+
each do
286+
[1, 2, 3].index(foo)
287+
^^^^^^^^^ Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant.
288+
end
289+
RUBY
290+
end
291+
292+
context 'when using an Array literal and calling `include?`' do
293+
[
294+
'"string"',
295+
'self',
296+
'local_variable',
297+
'method_call',
298+
'@instance_variable'
299+
].each do |argument|
300+
it "registers no offense when the argument is #{argument}" do
301+
expect_no_offenses(<<~RUBY)
302+
#{'local_variable = 123' if argument == 'local_variable'}
303+
array.all? do |e|
304+
[1, 2, 3].include?(#{argument})
305+
end
306+
RUBY
307+
end
308+
309+
it "registers no offense when the argument is #{argument} with method chain" do
310+
expect_no_offenses(<<~RUBY)
311+
#{'local_variable = 123' if argument == 'local_variable'}
312+
array.all? do |e|
313+
[1, 2, 3].include?(#{argument}.call)
314+
end
315+
RUBY
316+
end
317+
318+
it "registers no offense when the argument is #{argument} with double method chain" do
319+
expect_no_offenses(<<~RUBY)
320+
#{'local_variable = 123' if argument == 'local_variable'}
321+
array.all? do |e|
322+
[1, 2, 3].include?(#{argument}.call.call)
323+
end
324+
RUBY
325+
end
326+
327+
it "registers an offense when the argument is #{argument} with method chain and arguments" do
328+
expect_offense(<<~RUBY)
329+
#{'local_variable = 123' if argument == 'local_variable'}
330+
array.all? do |e|
331+
[1, 2, 3].include?(#{argument}.call(true))
332+
^^^^^^^^^ Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant.
333+
end
334+
RUBY
335+
end
336+
end
337+
end
338+
end
272339
end

0 commit comments

Comments
 (0)