From 8a2164dad6d2f130b84340cad254f3d95e1e80c2 Mon Sep 17 00:00:00 2001 From: Alexander Momchilov Date: Thu, 11 Jan 2024 20:32:55 -0500 Subject: [PATCH] [Fix #431] prefer `<<` with `push` --- .../cop/performance/array_push_single.rb | 57 +++++++++++++++++++ lib/rubocop/cop/performance_cops.rb | 1 + .../cop/performance/array_push_single_spec.rb | 56 ++++++++++++++++++ 3 files changed, 114 insertions(+) create mode 100755 lib/rubocop/cop/performance/array_push_single.rb create mode 100644 spec/rubocop/cop/performance/array_push_single_spec.rb diff --git a/lib/rubocop/cop/performance/array_push_single.rb b/lib/rubocop/cop/performance/array_push_single.rb new file mode 100755 index 0000000000..e072064d27 --- /dev/null +++ b/lib/rubocop/cop/performance/array_push_single.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Performance + # Identifies places where pushing a single element to an array + # can be replaced by `Array#<<`. + # + # @safety + # This cop is unsafe because not all objects that respond to `#push` also respond to `#<<` + # + # @example + # # bad + # array.push(1) + # + # # good + # array << 1 + # + # # good + # array.push(1, 2, 3) # `<<` only works for one element + # + class ArrayPushSingle < Base + extend AutoCorrector + + MSG = 'Use `<<` instead of `%s`.' + + PUSH_METHODS = Set[:push, :append].freeze + RESTRICT_ON_SEND = PUSH_METHODS + + def_node_matcher :push_call?, <<~PATTERN + (call $_receiver $%PUSH_METHODS $!(splat _)) + PATTERN + + def on_send(node) + raise + push_call?(node) do |receiver, method_name, element| + message = format(MSG, current: method_name) + + add_offense(node, message: message) do |corrector| + corrector.replace(node, "#{receiver.source} << #{element.source}") + end + end + end + + def on_csend(node) + push_call?(node) do |receiver, method_name, element| + message = format(MSG, current: method_name) + + add_offense(node, message: message) do |corrector| + corrector.replace(node, "#{receiver.source}&.<< #{element.source}") + end + end + end + end + end + end +end diff --git a/lib/rubocop/cop/performance_cops.rb b/lib/rubocop/cop/performance_cops.rb index 2a18f26120..b21673df09 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_push_single' 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_push_single_spec.rb b/spec/rubocop/cop/performance/array_push_single_spec.rb new file mode 100644 index 0000000000..38380ba139 --- /dev/null +++ b/spec/rubocop/cop/performance/array_push_single_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Performance::ArrayPushSingle, :config do + it 'registers an offense and corrects when using `push` with a single element' do + expect_offense(<<~RUBY) + array.push(element) + ^^^^^^^^^^^^^^^^^^^ Use `<<` instead of `push`. + RUBY + + expect_correction(<<~RUBY) + array << element + RUBY + end + + it 'registers an offense and corrects when using `push` with a single element and safe navigation operator' do + expect_offense(<<~RUBY) + array&.push(element) + ^^^^^^^^^^^^^^^^^^^^ Use `<<` instead of `push`. + RUBY + + # gross. TODO: make a configuration option? + expect_correction(<<~RUBY) + array&.<< element + RUBY + end + + it 'does not register an offense when using `push` with multiple elements' do + expect_no_offenses(<<~RUBY) + array.push(1, 2, 3) + RUBY + end + + it 'does not register an offense when using `push` with splatted elements' do + expect_no_offenses(<<~RUBY) + array.push(*elements) + RUBY + end + + describe "replacing `push` with `<<`" do + subject(:array) { [1, 2, 3] } + + it "returns the same result" do + expect([1, 2, 3] << 4).to eq([1, 2, 3].push(4)) + end + + it "it has the same side-effect" do + a = [1, 2, 3] + a << 4 + + b = [1, 2, 3] + b.push(4) + + expect(a).to eq(b) + end + end +end