diff --git a/changelog/new_add_array_insert_cop.md b/changelog/new_add_array_insert_cop.md new file mode 100644 index 000000000..9684f454a --- /dev/null +++ b/changelog/new_add_array_insert_cop.md @@ -0,0 +1 @@ +* [#396](https://github.com/rubocop/rubocop-performance/issues/396): Use `unshift` instead of `insert(0, ...)` for better performance. ([@pessi-v][]) diff --git a/config/default.yml b/config/default.yml index 42b2fc5b0..41b5fc400 100644 --- a/config/default.yml +++ b/config/default.yml @@ -11,6 +11,19 @@ Performance/AncestorsInclude: Safe: false VersionAdded: '1.7' +Performance/ArrayInsert: + Description: >- + Use `unshift` instead of `insert(0, ...)` for better performance. + The `unshift` method is specifically optimized for prepending elements + and is faster than using `insert` at index 0. + Reference: 'https://github.com/fastruby/fast-ruby#arrayinsert-vs-arrayunshift' + Enabled: 'pending' + VersionAdded: '<>' + Details: | + This cop identifies array prepend operations using the general-purpose + `insert` method at index 0, which can be replaced with the specialized + `unshift` method for a significant performance improvement. + Performance/ArraySemiInfiniteRangeSlice: Description: 'Identifies places where slicing arrays with semi-infinite ranges can be replaced by `Array#take` and `Array#drop`.' # This cop was created due to a mistake in microbenchmark. diff --git a/lib/rubocop/cop/performance/array_insert.rb b/lib/rubocop/cop/performance/array_insert.rb new file mode 100644 index 000000000..fb01494c7 --- /dev/null +++ b/lib/rubocop/cop/performance/array_insert.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Performance + # Identifies places where `array.insert(0, item)` can be replaced with `array.unshift(item)`. + # + # The `unshift` method is specifically optimized for prepending elements to the beginning + # of an array, providing up to 262x better performance than the more general `insert` method + # when used at index 0. + # + # @example + # # bad + # array.insert(0, item) + # array.insert(0, 1, 2, 3) + # array.insert(0, *items) + # + # # good + # array.unshift(item) + # array.unshift(1, 2, 3) + # array.unshift(*items) + # + # # good - insert at other positions is fine + # array.insert(1, item) + # array.insert(-1, item) + # array.insert(index, item) + # + class ArrayInsert < Base + include RangeHelp + extend AutoCorrector + + MSG = 'Use `unshift` instead of `insert(0, ...)` for better performance.' + RESTRICT_ON_SEND = %i[insert].freeze + + # @!method insert_at_zero?(node) + def_node_matcher :insert_at_zero?, <<~PATTERN + (send _ :insert (int 0) ...) + PATTERN + + # rubocop:disable Metrics/AbcSize + def on_send(node) + return unless insert_at_zero?(node) + + add_offense(range_between(node.loc.selector.begin_pos, node.source_range.end_pos)) do |corrector| + # Replace 'insert' with 'unshift' + corrector.replace(node.loc.selector, 'unshift') + + # Remove the first argument (0) and its following comma/space + first_arg = node.first_argument + range_to_remove = if node.arguments.size > 1 + second_arg = node.arguments[1] + # Remove from start of first arg to start of second arg + range_between(first_arg.source_range.begin_pos, second_arg.source_range.begin_pos) + elsif node.loc.begin && node.loc.end + # Edge case: only one argument with parentheses + range_between(node.loc.begin.begin_pos, node.loc.end.end_pos) + else + # No parentheses case + range_between(node.loc.selector.end_pos, first_arg.source_range.end_pos) + end + corrector.remove(range_to_remove) + end + end + # rubocop:enable Metrics/AbcSize + end + end + end +end diff --git a/lib/rubocop/cop/performance_cops.rb b/lib/rubocop/cop/performance_cops.rb index 8a56be5db..1101771b8 100644 --- a/lib/rubocop/cop/performance_cops.rb +++ b/lib/rubocop/cop/performance_cops.rb @@ -4,6 +4,7 @@ require_relative 'mixin/sort_block' require_relative 'performance/ancestors_include' +require_relative 'performance/array_insert' require_relative 'performance/array_semi_infinite_range_slice' require_relative 'performance/big_decimal_with_numeric_argument' require_relative 'performance/bind_call' diff --git a/spec/rubocop/cop/performance/array_insert_spec.rb b/spec/rubocop/cop/performance/array_insert_spec.rb new file mode 100644 index 000000000..9acd52bfa --- /dev/null +++ b/spec/rubocop/cop/performance/array_insert_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Performance::ArrayInsert, :config do + it 'registers an offense when using `insert(0, item)`' do + expect_offense(<<~RUBY) + array.insert(0, item) + ^^^^^^^^^^^^^^^ Use `unshift` instead of `insert(0, ...)` for better performance. + RUBY + + expect_correction(<<~RUBY) + array.unshift(item) + RUBY + end + + it 'registers an offense when using `insert(0, multiple, items)`' do + expect_offense(<<~RUBY) + array.insert(0, 1, 2, 3) + ^^^^^^^^^^^^^^^^^^ Use `unshift` instead of `insert(0, ...)` for better performance. + RUBY + + expect_correction(<<~RUBY) + array.unshift(1, 2, 3) + RUBY + end + + it 'registers an offense when using `insert(0, *items)` with splat' do + expect_offense(<<~RUBY) + array.insert(0, *items) + ^^^^^^^^^^^^^^^^^ Use `unshift` instead of `insert(0, ...)` for better performance. + RUBY + + expect_correction(<<~RUBY) + array.unshift(*items) + RUBY + end + + it 'registers an offense with complex arguments' do + expect_offense(<<~RUBY) + array.insert(0, method_call, hash[:key], *rest) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `unshift` instead of `insert(0, ...)` for better performance. + RUBY + + expect_correction(<<~RUBY) + array.unshift(method_call, hash[:key], *rest) + RUBY + end + + it 'does not register an offense when using insert with non-zero index' do + expect_no_offenses(<<~RUBY) + array.insert(1, item) + array.insert(-1, item) + array.insert(index, item) + RUBY + end + + it 'does not register an offense when using insert with variable index' do + expect_no_offenses(<<~RUBY) + position = 0 + array.insert(position, item) + RUBY + end + + it 'does not register an offense when using unshift' do + expect_no_offenses(<<~RUBY) + array.unshift(item) + array.unshift(1, 2, 3) + array.unshift(*items) + RUBY + end + + it 'handles insert in method chains correctly' do + expect_offense(<<~RUBY) + [1, 2, 3].insert(0, 0).reverse + ^^^^^^^^^^^^ Use `unshift` instead of `insert(0, ...)` for better performance. + RUBY + + expect_correction(<<~RUBY) + [1, 2, 3].unshift(0).reverse + RUBY + end + + it 'handles parentheses-less method calls' do + expect_offense(<<~RUBY) + array.insert 0, item + ^^^^^^^^^^^^^^ Use `unshift` instead of `insert(0, ...)` for better performance. + RUBY + + expect_correction(<<~RUBY) + array.unshift item + RUBY + end + + context 'when insert is called with only index 0 (edge case)' do + it 'registers an offense and corrects properly' do + expect_offense(<<~RUBY) + array.insert(0) + ^^^^^^^^^ Use `unshift` instead of `insert(0, ...)` for better performance. + RUBY + + expect_correction(<<~RUBY) + array.unshift + RUBY + end + end + + context 'when insert is called with only index 0 and no parentheses (edge case)' do + it 'registers an offense and corrects properly' do + expect_offense(<<~RUBY) + array.insert 0 + ^^^^^^^^ Use `unshift` instead of `insert(0, ...)` for better performance. + RUBY + + expect_correction(<<~RUBY) + array.unshift + RUBY + end + end +end