From 1abf72b9201662e37ba45f73dcb13e446fce9926 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 1 Jan 2020 03:25:30 +0900 Subject: [PATCH] [Fix #150] Add `EnforcedStyle: refute` for `Rails/RefuteMethods` Fixes #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 --- CHANGELOG.md | 1 + config/default.yml | 4 + lib/rubocop/cop/rails/refute_methods.rb | 78 ++++++--- manual/cops_rails.md | 16 ++ spec/rubocop/cop/rails/refute_methods_spec.rb | 151 +++++++++++++----- 5 files changed, 183 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80d975bffa..ff902a3f5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/config/default.yml b/config/default.yml index 7dead435a4..ec25965e3e 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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/**/*' diff --git a/lib/rubocop/cop/rails/refute_methods.rb b/lib/rubocop/cop/rails/refute_methods.rb index 3211463b5d..70bfe97a79 100644 --- a/lib/rubocop/cop/rails/refute_methods.rb +++ b/lib/rubocop/cop/rails/refute_methods.rb @@ -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] @@ -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 `%s` over `%s`.' + include ConfigurableEnforcedStyle + + MSG = 'Prefer `%s` over `%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) @@ -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 diff --git a/manual/cops_rails.md b/manual/cops_rails.md index dfa7a62796..5137fcf373 100644 --- a/manual/cops_rails.md +++ b/manual/cops_rails.md @@ -1865,6 +1865,8 @@ Use `assert_not` methods instead of `refute` methods. ### Examples +#### EnforcedStyle: assert_not (default) + ```ruby # bad refute false @@ -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 diff --git a/spec/rubocop/cop/rails/refute_methods_spec.rb b/spec/rubocop/cop/rails/refute_methods_spec.rb index ad8da756d1..37832a683d 100644 --- a/spec/rubocop/cop/rails/refute_methods_spec.rb +++ b/spec/rubocop/cop/rails/refute_methods_spec.rb @@ -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