diff --git a/CHANGELOG.md b/CHANGELOG.md index bc01c3e4cf97..a5f364a4eb83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * [#3510](https://github.com/bbatsov/rubocop/issues/3510): Add a configuration option, `ConvertCodeThatCanStartToReturnNil`, to `Style/SafeNavigation` to check for code that could start returning `nil` if safe navigation is used. ([@rrosenblum][]) * Add a new `AllCops/StyleGuideBaseURL` setting that allows the use of relative paths and/or fragments within each cop's `StyleGuide` setting, to make forking of custom style guides easier. ([@scottmatthewman][]) * [#3566](https://github.com/bbatsov/rubocop/issues/3566): Add new `Metric/BlockLength` cop to ensure blocks don't get too long. ([@savef][]) +* [#3428](https://github.com/bbatsov/rubocop/issues/3428): Add support for configuring `Style/PreferredHashMethods` with either `short` or `verbose` style method names. ([@abrom][]) ### Bug fixes @@ -2404,3 +2405,4 @@ [@scottmatthewman]: https://github.com/scottmatthewman [@tcdowney]: https://github.com/tcdowney [@logicminds]: https://github.com/logicminds +[@abrom]: https://github.com/abrom diff --git a/config/default.yml b/config/default.yml index df144726585d..d78943aba091 100644 --- a/config/default.yml +++ b/config/default.yml @@ -820,6 +820,12 @@ Style/PredicateName: Exclude: - 'spec/**/*' +Style/PreferredHashMethods: + EnforcedStyle: short + SupportedStyles: + - short + - verbose + Style/RaiseArgs: EnforcedStyle: exploded SupportedStyles: diff --git a/config/enabled.yml b/config/enabled.yml index bc900f8855db..6e85f0886eaa 100644 --- a/config/enabled.yml +++ b/config/enabled.yml @@ -171,11 +171,6 @@ Style/DefWithParentheses: StyleGuide: '#method-parens' Enabled: true -Style/PreferredHashMethods: - Description: 'Checks use of `has_key?` and `has_value?` Hash methods.' - StyleGuide: '#hash-key' - Enabled: true - Style/Documentation: Description: 'Document classes and non-namespace modules.' Enabled: true @@ -616,6 +611,11 @@ Style/PredicateName: StyleGuide: '#bool-methods-qmark' Enabled: true +Style/PreferredHashMethods: + Description: 'Checks use of `has_key?` and `has_value?` Hash methods.' + StyleGuide: '#hash-key' + Enabled: true + Style/Proc: Description: 'Use proc instead of Proc.new.' StyleGuide: '#proc' diff --git a/lib/rubocop/cop/style/preferred_hash_methods.rb b/lib/rubocop/cop/style/preferred_hash_methods.rb index 597d00685b30..630b6f572a63 100644 --- a/lib/rubocop/cop/style/preferred_hash_methods.rb +++ b/lib/rubocop/cop/style/preferred_hash_methods.rb @@ -3,17 +3,48 @@ module RuboCop module Cop module Style - # This cop checks for uses of methods Hash#has_key? and Hash#has_value? - # Prefer to use Hash#key? and Hash#value? instead + # This cop (by default) checks for uses of methods Hash#has_key? and + # Hash#has_value? where it enforces Hash#key? and Hash#value? + # It is configurable to enforce the inverse, using `verbose` method + # names also. + # + # @example + # + # # EnforcedStyle: short (default) + # + # # bad + # Hash#has_key? + # Hash#has_value? + # + # # good + # Hash#key? + # Hash#value? + # + # @example + # + # # EnforcedStyle: verbose + # + # # bad + # Hash#key? + # Hash#value? + # + # # good + # Hash#has_key? + # Hash#has_value? class PreferredHashMethods < Cop + include ConfigurableEnforcedStyle + MSG = 'Use `Hash#%s` instead of `Hash#%s`.'.freeze - PREFERRED_METHODS = [:has_key?, :has_value?].freeze + OFFENDING_SELECTORS = { + short: [:has_key?, :has_value?], + verbose: [:key?, :value?] + }.freeze def on_send(node) _receiver, method_name, *args = *node return unless args.size == 1 && - PREFERRED_METHODS.include?(method_name) + offending_selector?(method_name) add_offense(node, :selector, format(MSG, @@ -31,7 +62,15 @@ def autocorrect(node) private def proper_method_name(method_name) - method_name.to_s.sub(/has_/, '') + if style == :verbose + "has_#{method_name}" + else + method_name.to_s.sub(/has_/, '') + end + end + + def offending_selector?(method_name) + OFFENDING_SELECTORS[style].include?(method_name) end end end diff --git a/spec/rubocop/cop/style/preferred_hash_methods_spec.rb b/spec/rubocop/cop/style/preferred_hash_methods_spec.rb index 3cd60a927a1f..7e2abdf1c7fa 100644 --- a/spec/rubocop/cop/style/preferred_hash_methods_spec.rb +++ b/spec/rubocop/cop/style/preferred_hash_methods_spec.rb @@ -2,44 +2,90 @@ require 'spec_helper' -describe RuboCop::Cop::Style::PreferredHashMethods do - subject(:cop) { described_class.new } - - it 'registers an offense for has_key? with one arg' do - inspect_source(cop, - 'o.has_key?(o)') - expect(cop.offenses.size).to eq(1) - expect(cop.messages) - .to eq(['Use `Hash#key?` instead of `Hash#has_key?`.']) - end +describe RuboCop::Cop::Style::PreferredHashMethods, :config do + subject(:cop) { described_class.new(config) } - it 'accepts has_key? with no args' do - inspect_source(cop, - 'o.has_key?') - expect(cop.offenses).to be_empty - end + context 'with enforced `short` style' do + let(:cop_config) { { 'EnforcedStyle' => 'short' } } - it 'registers an offense for has_value? with one arg' do - inspect_source(cop, - 'o.has_value?(o)') - expect(cop.offenses.size).to eq(1) - expect(cop.messages) - .to eq(['Use `Hash#value?` instead of `Hash#has_value?`.']) - end + it 'registers an offense for has_key? with one arg' do + inspect_source(cop, + 'o.has_key?(o)') + expect(cop.offenses.size).to eq(1) + expect(cop.messages) + .to eq(['Use `Hash#key?` instead of `Hash#has_key?`.']) + end - it 'accepts has_value? with no args' do - inspect_source(cop, - 'o.has_value?') - expect(cop.offenses).to be_empty - end + it 'accepts has_key? with no args' do + inspect_source(cop, + 'o.has_key?') + expect(cop.offenses).to be_empty + end + + it 'registers an offense for has_value? with one arg' do + inspect_source(cop, + 'o.has_value?(o)') + expect(cop.offenses.size).to eq(1) + expect(cop.messages) + .to eq(['Use `Hash#value?` instead of `Hash#has_value?`.']) + end + + it 'accepts has_value? with no args' do + inspect_source(cop, + 'o.has_value?') + expect(cop.offenses).to be_empty + end + + it 'auto-corrects has_key? with key?' do + new_source = autocorrect_source(cop, 'hash.has_key?(:test)') + expect(new_source).to eq('hash.key?(:test)') + end - it 'auto-corrects has_key? with key?' do - new_source = autocorrect_source(cop, 'hash.has_key?(:test)') - expect(new_source).to eq('hash.key?(:test)') + it 'auto-corrects has_value? with value?' do + new_source = autocorrect_source(cop, 'hash.has_value?(value)') + expect(new_source).to eq('hash.value?(value)') + end end - it 'auto-corrects has_value? with value?' do - new_source = autocorrect_source(cop, 'hash.has_value?(value)') - expect(new_source).to eq('hash.value?(value)') + context 'with enforced `verbose` style' do + let(:cop_config) { { 'EnforcedStyle' => 'verbose' } } + + it 'registers an offense for key? with one arg' do + inspect_source(cop, + 'o.key?(o)') + expect(cop.offenses.size).to eq(1) + expect(cop.messages) + .to eq(['Use `Hash#has_key?` instead of `Hash#key?`.']) + end + + it 'accepts key? with no args' do + inspect_source(cop, + 'o.key?') + expect(cop.offenses).to be_empty + end + + it 'registers an offense for value? with one arg' do + inspect_source(cop, + 'o.value?(o)') + expect(cop.offenses.size).to eq(1) + expect(cop.messages) + .to eq(['Use `Hash#has_value?` instead of `Hash#value?`.']) + end + + it 'accepts value? with no args' do + inspect_source(cop, + 'o.value?') + expect(cop.offenses).to be_empty + end + + it 'auto-corrects key? with has_key?' do + new_source = autocorrect_source(cop, 'hash.key?(:test)') + expect(new_source).to eq('hash.has_key?(:test)') + end + + it 'auto-corrects value? with has_value?' do + new_source = autocorrect_source(cop, 'hash.value?(value)') + expect(new_source).to eq('hash.has_value?(value)') + end end end