Skip to content

Commit

Permalink
[Fix rubocop#3428] Allow Style/PreferredHashMethods to be configured …
Browse files Browse the repository at this point in the history
…with short (default) or verbose method naming
  • Loading branch information
abrom authored and Neodelf committed Oct 15, 2016
1 parent 9d43a59 commit ec32149
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 43 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down Expand Up @@ -2404,3 +2405,4 @@
[@scottmatthewman]: https://github.com/scottmatthewman
[@tcdowney]: https://github.com/tcdowney
[@logicminds]: https://github.com/logicminds
[@abrom]: https://github.com/abrom
6 changes: 6 additions & 0 deletions config/default.yml
Expand Up @@ -820,6 +820,12 @@ Style/PredicateName:
Exclude:
- 'spec/**/*'

Style/PreferredHashMethods:
EnforcedStyle: short
SupportedStyles:
- short
- verbose

Style/RaiseArgs:
EnforcedStyle: exploded
SupportedStyles:
Expand Down
10 changes: 5 additions & 5 deletions config/enabled.yml
Expand Up @@ -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
Expand Down Expand Up @@ -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'
Expand Down
49 changes: 44 additions & 5 deletions lib/rubocop/cop/style/preferred_hash_methods.rb
Expand Up @@ -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,
Expand All @@ -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
Expand Down
112 changes: 79 additions & 33 deletions spec/rubocop/cop/style/preferred_hash_methods_spec.rb
Expand Up @@ -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

0 comments on commit ec32149

Please sign in to comment.