From 7b0e011a037f1b6d874714add41ec53ee3abb49e Mon Sep 17 00:00:00 2001 From: fatkodima Date: Fri, 12 Jun 2020 21:51:00 +0300 Subject: [PATCH] Add new `Style/RedundantFetchBlock` cop --- CHANGELOG.md | 2 + config/default.yml | 13 ++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_style.adoc | 64 +++++++++ lib/rubocop.rb | 1 + .../cop/style/redundant_fetch_block.rb | 103 ++++++++++++++ .../cop/style/redundant_fetch_block_spec.rb | 132 ++++++++++++++++++ 7 files changed, 316 insertions(+) create mode 100644 lib/rubocop/cop/style/redundant_fetch_block.rb create mode 100644 spec/rubocop/cop/style/redundant_fetch_block_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index dcb8e3949b4..f7d5e5beec6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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][]) @@ -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 diff --git a/config/default.yml b/config/default.yml index aa56de3a8f5..dbb537f1144 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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 diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 003dfc3574f..a72ca8f155e 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -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] diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index d5203caa2c6..b7b912312e9 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -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 |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index f67ffe9a7bd..37ab45d5900 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -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' diff --git a/lib/rubocop/cop/style/redundant_fetch_block.rb b/lib/rubocop/cop/style/redundant_fetch_block.rb new file mode 100644 index 00000000000..7140c740c59 --- /dev/null +++ b/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 `%s` instead of `%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 diff --git a/spec/rubocop/cop/style/redundant_fetch_block_spec.rb b/spec/rubocop/cop/style/redundant_fetch_block_spec.rb new file mode 100644 index 00000000000..744e24ed7ef --- /dev/null +++ b/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