Skip to content

Commit

Permalink
[Fix rubocop#150] Add EnforcedStyle: refute for Rails/RefuteMethods
Browse files Browse the repository at this point in the history
Fixes rubocop#150.

This PR adds `EnforcedStyle: refute` for `Rails/RefuteMethods`.

`EnforcedStyle: refute` is an alternative style to
`EnforcedStyle: assert_not`.

```ruby
# @example EnforcedStyle: refute
#   # bad
#   assert_not false
#   assert_not_empty [1, 2, 3]
#   assert_not_equal true, false
#
#   # good
#   refute false
#   refute_empty [1, 2, 3]
#   refute_equal true, false
```

The default style keeps `EnforcedStyle: assert_not` because
following the Rails coding conventions conventions.

> Use `assert_not` methods instead of refute.

https://guides.rubyonrails.org/contributing_to_ruby_on_rails.html#follow-the-coding-conventions
  • Loading branch information
koic committed Feb 20, 2020
1 parent 652bb56 commit 1abf72b
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 67 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### New features

* [#197](https://github.com/rubocop-hq/rubocop-rails/pull/197): Add `Rails/UniqueValidationWithoutIndex` cop. ([@pocke][])
* [#150](https://github.com/rubocop-hq/rubocop-rails/issues/150): Add `EnforcedStyle: refute` for `Rails/RefuteMethods` cop. ([@koic][])

### Bug fixes

Expand Down
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,10 @@ Rails/RefuteMethods:
Description: 'Use `assert_not` methods instead of `refute` methods.'
Enabled: true
VersionAdded: '0.56'
EnforcedStyle: assert_not
SupportedStyles:
- assert_not
- refute
Include:
- '**/test/**/*'

Expand Down
78 changes: 52 additions & 26 deletions lib/rubocop/cop/rails/refute_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Rails
#
# Use `assert_not` methods instead of `refute` methods.
#
# @example
# @example EnforcedStyle: assert_not (default)
# # bad
# refute false
# refute_empty [1, 2, 3]
Expand All @@ -17,29 +17,43 @@ module Rails
# assert_not_empty [1, 2, 3]
# assert_not_equal true, false
#
# @example EnforcedStyle: refute
# # bad
# assert_not false
# assert_not_empty [1, 2, 3]
# assert_not_equal true, false
#
# # good
# refute false
# refute_empty [1, 2, 3]
# refute_equal true, false
#
class RefuteMethods < Cop
MSG = 'Prefer `%<assert_method>s` over `%<refute_method>s`.'
include ConfigurableEnforcedStyle

MSG = 'Prefer `%<good_method>s` over `%<bad_method>s`.'

CORRECTIONS = {
refute: 'assert_not',
refute_empty: 'assert_not_empty',
refute_equal: 'assert_not_equal',
refute_in_delta: 'assert_not_in_delta',
refute_in_epsilon: 'assert_not_in_epsilon',
refute_includes: 'assert_not_includes',
refute_instance_of: 'assert_not_instance_of',
refute_kind_of: 'assert_not_kind_of',
refute_nil: 'assert_not_nil',
refute_operator: 'assert_not_operator',
refute_predicate: 'assert_not_predicate',
refute_respond_to: 'assert_not_respond_to',
refute_same: 'assert_not_same',
refute_match: 'assert_no_match'
refute: :assert_not,
refute_empty: :assert_not_empty,
refute_equal: :assert_not_equal,
refute_in_delta: :assert_not_in_delta,
refute_in_epsilon: :assert_not_in_epsilon,
refute_includes: :assert_not_includes,
refute_instance_of: :assert_not_instance_of,
refute_kind_of: :assert_not_kind_of,
refute_nil: :assert_not_nil,
refute_operator: :assert_not_operator,
refute_predicate: :assert_not_predicate,
refute_respond_to: :assert_not_respond_to,
refute_same: :assert_not_same,
refute_match: :assert_no_match
}.freeze

OFFENSIVE_METHODS = CORRECTIONS.keys.freeze
REFUTE_METHODS = CORRECTIONS.keys.freeze
ASSERT_NOT_METHODS = CORRECTIONS.values.freeze

def_node_matcher :offensive?, '(send nil? #refute_method? ...)'
def_node_matcher :offensive?, '(send nil? #bad_method? ...)'

def on_send(node)
return unless offensive?(node)
Expand All @@ -49,27 +63,39 @@ def on_send(node)
end

def autocorrect(node)
bad_method = node.method_name
good_method = convert_good_method(bad_method)

lambda do |corrector|
corrector.replace(
node.loc.selector,
CORRECTIONS[node.method_name]
)
corrector.replace(node.loc.selector, good_method.to_s)
end
end

private

def refute_method?(method_name)
OFFENSIVE_METHODS.include?(method_name)
def bad_method?(method_name)
if style == :assert_not
REFUTE_METHODS.include?(method_name)
else
ASSERT_NOT_METHODS.include?(method_name)
end
end

def offense_message(method_name)
format(
MSG,
refute_method: method_name,
assert_method: CORRECTIONS[method_name]
bad_method: method_name,
good_method: convert_good_method(method_name)
)
end

def convert_good_method(bad_method)
if style == :assert_not
CORRECTIONS.fetch(bad_method)
else
CORRECTIONS.invert.fetch(bad_method)
end
end
end
end
end
Expand Down
16 changes: 16 additions & 0 deletions manual/cops_rails.md
Original file line number Diff line number Diff line change
Expand Up @@ -1865,6 +1865,8 @@ Use `assert_not` methods instead of `refute` methods.

### Examples

#### EnforcedStyle: assert_not (default)

```ruby
# bad
refute false
Expand All @@ -1876,11 +1878,25 @@ assert_not false
assert_not_empty [1, 2, 3]
assert_not_equal true, false
```
#### EnforcedStyle: refute

```ruby
# bad
assert_not false
assert_not_empty [1, 2, 3]
assert_not_equal true, false

# good
refute false
refute_empty [1, 2, 3]
refute_equal true, false
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
EnforcedStyle | `assert_not` | `assert_not`, `refute`
Include | `**/test/**/*` | Array

## Rails/RelativeDateConstant
Expand Down
151 changes: 110 additions & 41 deletions spec/rubocop/cop/rails/refute_methods_spec.rb
Original file line number Diff line number Diff line change
@@ -1,53 +1,122 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::RefuteMethods do
subject(:cop) { described_class.new }

it 'registers an offense and correct using `refute` with a single argument' do
expect_offense(<<~RUBY)
refute foo
^^^^^^ Prefer `assert_not` over `refute`.
RUBY

expect_correction(<<~RUBY)
assert_not foo
RUBY
end
RSpec.describe RuboCop::Cop::Rails::RefuteMethods, :config do
subject(:cop) { described_class.new(config) }

it 'registers an offense and corrects using `refute` ' \
'with multiple arguments' do
expect_offense(<<~RUBY)
refute foo, bar, baz
^^^^^^ Prefer `assert_not` over `refute`.
RUBY
let(:config) do
RuboCop::Config.new('Rails/RefuteMethods' => cop_config)
end

expect_correction(<<~RUBY)
assert_not foo, bar, baz
RUBY
let(:cop_config) do
{
'EnforcedStyle' => enforced_style,
'SupportedStyles' => %w[assert_not refute]
}
end

it 'registers an offense when using `refute_empty`' do
expect_offense(<<~RUBY)
refute_empty foo
^^^^^^^^^^^^ Prefer `assert_not_empty` over `refute_empty`.
RUBY
context 'when EnforcedStyle is `assert_not`' do
let(:enforced_style) { 'assert_not' }

expect_correction(<<~RUBY)
assert_not_empty foo
RUBY
end
it 'registers an offense and correct using `refute` ' \
'with a single argument' do
expect_offense(<<~RUBY)
refute foo
^^^^^^ Prefer `assert_not` over `refute`.
RUBY

expect_correction(<<~RUBY)
assert_not foo
RUBY
end

it 'registers an offense and corrects using `refute` ' \
'with multiple arguments' do
expect_offense(<<~RUBY)
refute foo, bar, baz
^^^^^^ Prefer `assert_not` over `refute`.
RUBY

expect_correction(<<~RUBY)
assert_not foo, bar, baz
RUBY
end

it 'registers an offense when using `refute_empty`' do
expect_offense(<<~RUBY)
refute_empty foo
^^^^^^^^^^^^ Prefer `assert_not_empty` over `refute_empty`.
RUBY

expect_correction(<<~RUBY)
assert_not_empty foo
RUBY
end

it 'does not registers an offense when using `assert_not` ' \
'with a single argument' do
expect_no_offenses(<<~RUBY)
assert_not foo
RUBY
end

it 'does not registers an offense when using `assert_not` ' \
'with a single argument' do
expect_no_offenses(<<~RUBY)
assert_not foo
RUBY
it 'does not registers an offense when using `assert_not` ' \
'with a multiple arguments' do
expect_no_offenses(<<~RUBY)
assert_not foo, bar, baz
RUBY
end
end

it 'does not registers an offense when using `assert_not` ' \
'with a multiple arguments' do
expect_no_offenses(<<~RUBY)
assert_not foo, bar, baz
RUBY
context 'when EnforcedStyle is `refute`' do
let(:enforced_style) { 'refute' }

it 'registers an offense and correct using `assert_not` ' \
'with a single argument' do
expect_offense(<<~RUBY)
assert_not foo
^^^^^^^^^^ Prefer `refute` over `assert_not`.
RUBY

expect_correction(<<~RUBY)
refute foo
RUBY
end

it 'registers an offense and corrects using `assert_not` ' \
'with multiple arguments' do
expect_offense(<<~RUBY)
assert_not foo, bar, baz
^^^^^^^^^^ Prefer `refute` over `assert_not`.
RUBY

expect_correction(<<~RUBY)
refute foo, bar, baz
RUBY
end

it 'registers an offense when using `assert_not_empty`' do
expect_offense(<<~RUBY)
assert_not_empty foo
^^^^^^^^^^^^^^^^ Prefer `refute_empty` over `assert_not_empty`.
RUBY

expect_correction(<<~RUBY)
refute_empty foo
RUBY
end

it 'does not registers an offense when using `refute` ' \
'with a single argument' do
expect_no_offenses(<<~RUBY)
refute foo
RUBY
end

it 'does not registers an offense when using `refute` ' \
'with a multiple arguments' do
expect_no_offenses(<<~RUBY)
refute foo, bar, baz
RUBY
end
end
end

0 comments on commit 1abf72b

Please sign in to comment.