Skip to content

Commit

Permalink
Add new Style/RedundantFetchBlock cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima authored and bbatsov committed Jun 21, 2020
1 parent 42a03c3 commit 7b0e011
Show file tree
Hide file tree
Showing 7 changed files with 316 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -4,6 +4,7 @@

### New features

* [#8147](https://github.com/rubocop-hq/rubocop/pull/8147): Add new `Style/RedundantFetchBlock` cop. ([@fatkodima][])
* [#8111](https://github.com/rubocop-hq/rubocop/pull/8111): Add auto-correct for `Style/StructInheritance`. ([@tejasbubane][])
* [#8113](https://github.com/rubocop-hq/rubocop/pull/8113): Let `expect_offense` templates add variable-length whitespace with `_{foo}`. ([@eugeneius][])
* [#8148](https://github.com/rubocop-hq/rubocop/pull/8148): Support autocorrection for `Style/MultilineTernaryOperator`. ([@koic][])
Expand Down Expand Up @@ -4604,3 +4605,4 @@
[@andrykonchin]: https://github.com/andrykonchin
[@avrusanov]: https://github.com/avrusanov
[@mauro-oto]: https://github.com/mauro-oto
[@fatkodima]: https://github.com/fatkodima
13 changes: 13 additions & 0 deletions config/default.yml
Expand Up @@ -3610,6 +3610,19 @@ Style/RedundantException:
VersionAdded: '0.14'
VersionChanged: '0.29'

Style/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: 'pending'
Safe: 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.
SafeForConstants: false
VersionAdded: '0.86'

Style/RedundantFreeze:
Description: "Checks usages of Object#freeze on immutable objects."
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -435,6 +435,7 @@ In the following section you find all available cops:
* xref:cops_style.adoc#styleredundantcondition[Style/RedundantCondition]
* xref:cops_style.adoc#styleredundantconditional[Style/RedundantConditional]
* xref:cops_style.adoc#styleredundantexception[Style/RedundantException]
* xref:cops_style.adoc#styleredundantfetchblock[Style/RedundantFetchBlock]
* xref:cops_style.adoc#styleredundantfreeze[Style/RedundantFreeze]
* xref:cops_style.adoc#styleredundantinterpolation[Style/RedundantInterpolation]
* xref:cops_style.adoc#styleredundantparentheses[Style/RedundantParentheses]
Expand Down
64 changes: 64 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -7004,6 +7004,70 @@ raise 'message'

* https://rubystyle.guide#no-explicit-runtimeerror

== Style/RedundantFetchBlock

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| No
| Yes (Unsafe)
| 0.86
| -
|===

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

==== SafeForConstants: false (default)

[source,ruby]
----
# bad
hash.fetch(:key) { 5 }
hash.fetch(:key) { true }
hash.fetch(:key) { nil }
array.fetch(5) { :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')
----

==== SafeForConstants: true

[source,ruby]
----
# bad
ENV.fetch(:key) { VALUE }
# good
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

== Style/RedundantFreeze

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -426,6 +426,7 @@
require_relative 'rubocop/cop/style/line_end_concatenation'
require_relative 'rubocop/cop/style/method_call_without_args_parentheses'
require_relative 'rubocop/cop/style/method_call_with_args_parentheses'
require_relative 'rubocop/cop/style/redundant_fetch_block'
require_relative 'rubocop/cop/style/method_call_with_args_parentheses/omit_parentheses'
require_relative 'rubocop/cop/style/method_call_with_args_parentheses/require_parentheses'
require_relative 'rubocop/cop/style/method_called_on_do_end_block'
Expand Down
103 changes: 103 additions & 0 deletions lib/rubocop/cop/style/redundant_fetch_block.rb
@@ -0,0 +1,103 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# 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 SafeForConstants: false (default)
# # bad
# hash.fetch(:key) { 5 }
# hash.fetch(:key) { true }
# hash.fetch(:key) { nil }
# array.fetch(5) { :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')
#
# @example SafeForConstants: true
# # bad
# ENV.fetch(:key) { VALUE }
#
# # good
# 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
132 changes: 132 additions & 0 deletions spec/rubocop/cop/style/redundant_fetch_block_spec.rb
@@ -0,0 +1,132 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::RedundantFetchBlock do
subject(:cop) { described_class.new(config) }

context 'with SafeForConstants: true' do
let(:config) do
RuboCop::Config.new(
'Style/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(
'Style/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

0 comments on commit 7b0e011

Please sign in to comment.