Add new Performance/RedundantFetchBlock cop

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### New features
 
+* [#126](https://github.com/rubocop-hq/rubocop-performance/pull/126): Add new `Performance/RedundantFetchBlock` cop. ([@fatkodima][])
 * [#132](https://github.com/rubocop-hq/rubocop-performance/issues/132): Add new `Performance/RedundantSortBlock` cop. ([@fatkodima][])
 * [#125](https://github.com/rubocop-hq/rubocop-performance/pull/125): Support `Array()` and `Hash()` methods for `Performance/Size` cop. ([@fatkodima][])
 * [#124](https://github.com/rubocop-hq/rubocop-performance/pull/124): Add new `Performance/Squeeze` cop. ([@fatkodima][])

config/default.yml

@@ -162,6 +162,18 @@ Performance/RedundantBlockCall:
   Enabled: true
   VersionAdded: '0.36'
 
+Performance/RedundantFetchBlock:
+  Description: >-
+                  Use `fetch(key, value)` instead of `fetch(key) { value }`
+                  when value has Numeric, Rational, Complex, Symbol or String type, `false`, `true`, `nil` or is a constant.
+  Reference: 'https://github.com/JuanitoFatas/fast-ruby#hashfetch-with-argument-vs-hashfetch--block-code'
+  Enabled: true
+  VersionAdded: '1.7'
+  SafeForConstants: false
+  # If enabled, this cop will autocorrect usages of
+  # `fetch` being called with block returning a constant.
+  # This can be dangerous since constants will not be defined at that moment.
+
 Performance/RedundantMatch:
   Description: >-
                   Use `=~` instead of `String#match` or `Regexp#match` in a context where the

docs/modules/ROOT/pages/cops.adoc

@@ -22,6 +22,7 @@
 * xref:cops_performance.adoc#performanceopenstruct[Performance/OpenStruct]
 * xref:cops_performance.adoc#performancerangeinclude[Performance/RangeInclude]
 * xref:cops_performance.adoc#performanceredundantblockcall[Performance/RedundantBlockCall]
+* xref:cops_performance.adoc#performanceredundantfetchblock[Performance/RedundantFetchBlock]
 * xref:cops_performance.adoc#performanceredundantmatch[Performance/RedundantMatch]
 * xref:cops_performance.adoc#performanceredundantmerge[Performance/RedundantMerge]
 * xref:cops_performance.adoc#performanceredundantsortblock[Performance/RedundantSortBlock]

docs/modules/ROOT/pages/cops_performance.adoc

@@ -948,6 +948,59 @@ end
 
 * https://github.com/JuanitoFatas/fast-ruby#proccall-and-block-arguments-vs-yieldcode
 
+== Performance/RedundantFetchBlock
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Enabled
+| Yes
+| Yes
+| 1.7
+| -
+|===
+
+This cop identifies places where `fetch(key) { value }`
+can be replaced by `fetch(key, value)`.
+
+In such cases `fetch(key, value)` method is faster
+than `fetch(key) { value }`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+hash.fetch(:key) { 5 }
+hash.fetch(:key) { true }
+hash.fetch(:key) { nil }
+array.fetch(5) { :value }
+ENV.fetch(:key) { VALUE }
+ENV.fetch(:key) { 'value' }
+
+# good
+hash.fetch(:key, 5)
+hash.fetch(:key, true)
+hash.fetch(:key, nil)
+array.fetch(5, :value)
+ENV.fetch(:key, VALUE)
+ENV.fetch(:key, 'value')
+----
+
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| SafeForConstants
+| `false`
+| Boolean
+|===
+
+=== References
+
+* https://github.com/JuanitoFatas/fast-ruby#hashfetch-with-argument-vs-hashfetch--block-code
+
 == Performance/RedundantMatch
 
 |===

lib/rubocop/cop/performance/redundant_fetch_block.rb

@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Performance
+      # This cop identifies places where `fetch(key) { value }`
+      # can be replaced by `fetch(key, value)`.
+      #
+      # In such cases `fetch(key, value)` method is faster
+      # than `fetch(key) { value }`.
+      #
+      # @example
+      #   # bad
+      #   hash.fetch(:key) { 5 }
+      #   hash.fetch(:key) { true }
+      #   hash.fetch(:key) { nil }
+      #   array.fetch(5) { :value }
+      #   ENV.fetch(:key) { VALUE }
+      #   ENV.fetch(:key) { 'value' }
+      #
+      #   # good
+      #   hash.fetch(:key, 5)
+      #   hash.fetch(:key, true)
+      #   hash.fetch(:key, nil)
+      #   array.fetch(5, :value)
+      #   ENV.fetch(:key, VALUE)
+      #   ENV.fetch(:key, 'value')
+      #
+      class RedundantFetchBlock < Cop
+        include FrozenStringLiteral
+        include RangeHelp
+
+        MSG = 'Use `%<good>s` instead of `%<bad>s`.'
+
+        def_node_matcher :redundant_fetch_block_candidate?, <<~PATTERN
+          (block
+            $(send _ :fetch _)
+            (args)
+            ${#basic_literal? const_type?})
+        PATTERN
+
+        def on_block(node)
+          redundant_fetch_block_candidate?(node) do |send, body|
+            return if body.const_type? && !check_for_constant?
+            return if body.str_type? && !check_for_string?
+
+            range = fetch_range(send, node)
+            good = build_good_method(send, body)
+            bad = build_bad_method(send, body)
+
+            add_offense(
+              node,
+              location: range,
+              message: format(MSG, good: good, bad: bad)
+            )
+          end
+        end
+
+        def autocorrect(node)
+          redundant_fetch_block_candidate?(node) do |send, body|
+            lambda do |corrector|
+              receiver, _, key = send.children
+              corrector.replace(node, "#{receiver.source}.fetch(#{key.source}, #{body.source})")
+            end
+          end
+        end
+
+        private
+
+        def basic_literal?(node)
+          node.basic_literal?
+        end
+
+        def fetch_range(send, node)
+          range_between(send.loc.selector.begin_pos, node.loc.end.end_pos)
+        end
+
+        def build_good_method(send, body)
+          key = send.children[2].source
+          "fetch(#{key}, #{body.source})"
+        end
+
+        def build_bad_method(send, body)
+          key = send.children[2].source
+          "fetch(#{key}) { #{body.source} }"
+        end
+
+        def check_for_constant?
+          cop_config['SafeForConstants']
+        end
+
+        def check_for_string?
+          frozen_string_literals_enabled?
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/performance_cops.rb

@@ -22,6 +22,7 @@
 require_relative 'performance/open_struct'
 require_relative 'performance/range_include'
 require_relative 'performance/redundant_block_call'
+require_relative 'performance/redundant_fetch_block'
 require_relative 'performance/redundant_match'
 require_relative 'performance/redundant_merge'
 require_relative 'performance/redundant_sort_block'

spec/rubocop/cop/performance/redundant_fetch_block_spec.rb

@@ -0,0 +1,132 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Performance::RedundantFetchBlock do
+  subject(:cop) { described_class.new(config) }
+
+  context 'with SafeForConstants: true' do
+    let(:config) do
+      RuboCop::Config.new(
+        'Performance/RedundantFetchBlock' => {
+          'SafeForConstants' => true
+        }
+      )
+    end
+
+    it 'registers an offense and corrects when using `#fetch` with Integer in the block' do
+      expect_offense(<<~RUBY)
+        hash.fetch(:key) { 5 }
+             ^^^^^^^^^^^^^^^^^ Use `fetch(:key, 5)` instead of `fetch(:key) { 5 }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        hash.fetch(:key, 5)
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `#fetch` with Float in the block' do
+      expect_offense(<<~RUBY)
+        hash.fetch(:key) { 2.5 }
+             ^^^^^^^^^^^^^^^^^^^ Use `fetch(:key, 2.5)` instead of `fetch(:key) { 2.5 }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        hash.fetch(:key, 2.5)
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `#fetch` with Symbol in the block' do
+      expect_offense(<<~RUBY)
+        hash.fetch(:key) { :value }
+             ^^^^^^^^^^^^^^^^^^^^^^ Use `fetch(:key, :value)` instead of `fetch(:key) { :value }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        hash.fetch(:key, :value)
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `#fetch` with Rational in the block' do
+      expect_offense(<<~RUBY)
+        hash.fetch(:key) { 2.0r }
+             ^^^^^^^^^^^^^^^^^^^^ Use `fetch(:key, 2.0r)` instead of `fetch(:key) { 2.0r }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        hash.fetch(:key, 2.0r)
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `#fetch` with Complex in the block' do
+      expect_offense(<<~RUBY)
+        hash.fetch(:key) { 1i }
+             ^^^^^^^^^^^^^^^^^^ Use `fetch(:key, 1i)` instead of `fetch(:key) { 1i }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        hash.fetch(:key, 1i)
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `#fetch` with constant in the block' do
+      expect_offense(<<~RUBY)
+        hash.fetch(:key) { CONSTANT }
+             ^^^^^^^^^^^^^^^^^^^^^^^^ Use `fetch(:key, CONSTANT)` instead of `fetch(:key) { CONSTANT }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        hash.fetch(:key, CONSTANT)
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `#fetch` with String in the block and strings are frozen' do
+      expect_offense(<<~RUBY)
+        # frozen_string_literal: true
+        hash.fetch(:key) { 'value' }
+             ^^^^^^^^^^^^^^^^^^^^^^^ Use `fetch(:key, 'value')` instead of `fetch(:key) { 'value' }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        # frozen_string_literal: true
+        hash.fetch(:key, 'value')
+      RUBY
+    end
+
+    it 'does not register an offense when using `#fetch` with String in the block and strings are not frozen' do
+      expect_no_offenses(<<~RUBY)
+        hash.fetch(:key) { 'value' }
+      RUBY
+    end
+
+    it 'does not register an offense when using `#fetch` with argument fallback' do
+      expect_no_offenses(<<~RUBY)
+        hash.fetch(:key, :value)
+      RUBY
+    end
+
+    it 'does not register an offense when using `#fetch` with interpolated Symbol in the block' do
+      inspect_source('hash.fetch(:key) { :"value_#{value}" }')
+      expect(cop.offenses.size).to eq(0)
+    end
+
+    it 'does not register an offense when using `#fetch` with an argument in the block' do
+      inspect_source('hash.fetch(:key) { |k| "missing-#{k}" }')
+      expect(cop.offenses.size).to eq(0)
+    end
+  end
+
+  context 'with SafeForConstants: false' do
+    let(:config) do
+      RuboCop::Config.new(
+        'Performance/RedundantFetchBlock' => {
+          'SafeForConstants' => false
+        }
+      )
+    end
+
+    it 'does not register an offense when using `#fetch` with constant in the block' do
+      expect_no_offenses(<<~RUBY)
+        hash.fetch(:key) { CONSTANT }
+      RUBY
+    end
+  end
+end