diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c5d3e8ac..25d0760d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * Fix a false positive for `RSpec/Capybara/SpecificMatcher`. ([@ydah][]) * Fix a false negative for `RSpec/Capybara/SpecificMatcher` for `have_field`. ([@ydah][]) * Fix a false positive for `RSpec/Capybara/SpecificMatcher` when may not have a `href` by `have_link`. ([@ydah][]) +* Add `NegatedMatcher` configuration option to `RSpec/ChangeByZero`. ([@ydah][]) ## 2.12.1 (2022-07-03) diff --git a/config/default.yml b/config/default.yml index 926673a08..5b1e3ef44 100644 --- a/config/default.yml +++ b/config/default.yml @@ -192,8 +192,10 @@ RSpec/BeforeAfterAll: RSpec/ChangeByZero: Description: Prefer negated matchers over `to change.by(0)`. Enabled: pending - VersionAdded: 2.11.0 + VersionAdded: '2.11' + VersionChanged: '2.13' Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ChangeByZero + NegatedMatcher: ~ RSpec/ContextMethod: Description: "`context` should not be used for specifying methods." diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index 6f461b0ba..22f5a5ee8 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -391,16 +391,25 @@ end | Pending | Yes | Yes -| 2.11.0 -| - +| 2.11 +| 2.13 |=== Prefer negated matchers over `to change.by(0)`. -This cop does not support autocorrection in some cases. +In the case of composite expectations, cop suggest using the +negation matchers of `RSpec::Matchers#change`. + +By default the cop does not support autocorrect of +compound expectations, but if you set the +negated matcher for `change`, e.g. `not_change` with +the `NegatedMatcher` option, the cop will perform the autocorrection. +`NegatedMatcher` option, will be autocorrect. === Examples +==== NegatedMatcher: ~ (default) + [source,ruby] ---- # bad @@ -429,6 +438,38 @@ expect { run } .and not_change { Foo.baz } ---- +==== NegatedMatcher: not_change + +[source,ruby] +---- +# bad (support autocorrection to good case) +expect { run } + .to change(Foo, :bar).by(0) + .and change(Foo, :baz).by(0) +expect { run } + .to change { Foo.bar }.by(0) + .and change { Foo.baz }.by(0) + +# good +define_negated_matcher :not_change, :change +expect { run } + .to not_change(Foo, :bar) + .and not_change(Foo, :baz) +expect { run } + .to not_change { Foo.bar } + .and not_change { Foo.baz } +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| NegatedMatcher +| `` +| +|=== + === References * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ChangeByZero diff --git a/lib/rubocop/cop/rspec/change_by_zero.rb b/lib/rubocop/cop/rspec/change_by_zero.rb index 243c37b87..c08c840f3 100644 --- a/lib/rubocop/cop/rspec/change_by_zero.rb +++ b/lib/rubocop/cop/rspec/change_by_zero.rb @@ -5,9 +5,16 @@ module Cop module RSpec # Prefer negated matchers over `to change.by(0)`. # - # This cop does not support autocorrection in some cases. + # In the case of composite expectations, cop suggest using the + # negation matchers of `RSpec::Matchers#change`. # - # @example + # By default the cop does not support autocorrect of + # compound expectations, but if you set the + # negated matcher for `change`, e.g. `not_change` with + # the `NegatedMatcher` option, the cop will perform the autocorrection. + # `NegatedMatcher` option, will be autocorrect. + # + # @example NegatedMatcher: ~ (default) # # bad # expect { run }.to change(Foo, :bar).by(0) # expect { run }.to change { Foo.bar }.by(0) @@ -33,10 +40,28 @@ module RSpec # .to not_change { Foo.bar } # .and not_change { Foo.baz } # + # @example NegatedMatcher: not_change + # # bad (support autocorrection to good case) + # expect { run } + # .to change(Foo, :bar).by(0) + # .and change(Foo, :baz).by(0) + # expect { run } + # .to change { Foo.bar }.by(0) + # .and change { Foo.baz }.by(0) + # + # # good + # define_negated_matcher :not_change, :change + # expect { run } + # .to not_change(Foo, :bar) + # .and not_change(Foo, :baz) + # expect { run } + # .to not_change { Foo.bar } + # .and not_change { Foo.baz } + # class ChangeByZero < Base extend AutoCorrector MSG = 'Prefer `not_to change` over `to change.by(0)`.' - MSG_COMPOUND = 'Prefer negated matchers with compound expectations ' \ + MSG_COMPOUND = 'Prefer %s with compound expectations ' \ 'over `change.by(0)`.' RESTRICT_ON_SEND = %i[change].freeze @@ -57,6 +82,11 @@ class ChangeByZero < Base (int 0)) PATTERN + # @!method change_nodes(node) + def_node_search :change_nodes, <<-PATTERN + $(send nil? :change ...) + PATTERN + def on_send(node) expect_change_with_arguments(node.parent) do check_offense(node.parent) @@ -72,7 +102,9 @@ def on_send(node) def check_offense(node) expression = node.loc.expression if compound_expectations?(node) - add_offense(expression, message: MSG_COMPOUND) + add_offense(expression, message: message_compound) do |corrector| + autocorrect_compound(corrector, node) + end else add_offense(expression) do |corrector| autocorrect(corrector, node) @@ -89,6 +121,28 @@ def autocorrect(corrector, node) range = node.loc.dot.with(end_pos: node.loc.expression.end_pos) corrector.remove(range) end + + def autocorrect_compound(corrector, node) + return unless negated_matcher + + change_nodes(node) do |change_node| + corrector.replace(change_node.loc.selector, negated_matcher) + range = node.loc.dot.with(end_pos: node.loc.expression.end_pos) + corrector.remove(range) + end + end + + def negated_matcher + cop_config['NegatedMatcher'] + end + + def message_compound + format(MSG_COMPOUND, preferred: preferred_method) + end + + def preferred_method + negated_matcher ? "`#{negated_matcher}`" : 'negated matchers' + end end end end diff --git a/spec/rubocop/cop/rspec/change_by_zero_spec.rb b/spec/rubocop/cop/rspec/change_by_zero_spec.rb index 26397e4fc..7a46d4c8e 100644 --- a/spec/rubocop/cop/rspec/change_by_zero_spec.rb +++ b/spec/rubocop/cop/rspec/change_by_zero_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::RSpec::ChangeByZero do +RSpec.describe RuboCop::Cop::RSpec::ChangeByZero, :config do it 'registers an offense when the argument to `by` is zero' do expect_offense(<<-RUBY) it do @@ -25,54 +25,207 @@ RUBY end - it 'registers an offense when the argument to `by` is zero ' \ - 'with compound expectations' do - expect_offense(<<-RUBY) + context 'when `NegatedMatcher` is not defined (default)' do + it 'registers an offense when the argument to `by` is zero ' \ + 'with compound expectations by `and`' do + expect_offense(<<-RUBY) + it do + expect { foo }.to change(Foo, :bar).by(0).and change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change { Foo.bar }.by(0).and change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + end + RUBY + + expect_no_corrections + end + + it 'registers an offense when the argument to `by` is zero ' \ + 'with compound expectations by `&`' do + expect_offense(<<-RUBY) + it do + expect { foo }.to change(Foo, :bar).by(0) & change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change { Foo.bar }.by(0) & change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + end + RUBY + + expect_no_corrections + end + + it 'registers an offense when the argument to `by` is zero ' \ + 'with compound expectations by `or`' do + expect_offense(<<-RUBY) + it do + expect { foo }.to change(Foo, :bar).by(0).or change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change { Foo.bar }.by(0).or change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + end + RUBY + + expect_no_corrections + end + + it 'registers an offense when the argument to `by` is zero ' \ + 'with compound expectations by `|`' do + expect_offense(<<-RUBY) + it do + expect { foo }.to change(Foo, :bar).by(0) | change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change { Foo.bar }.by(0) | change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + end + RUBY + + expect_no_corrections + end + + context 'when with a line break' do + it 'registers an offense when the argument to `by` is zero ' \ + 'with compound expectations by `and`' do + expect_offense(<<-RUBY) + it do + expect { foo } + .to change(Foo, :bar).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + .and change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo } + .to change { Foo.bar }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + .and change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + end + RUBY + + expect_no_corrections + end + + it 'registers an offense when the argument to `by` is zero ' \ + 'with compound expectations by `&`' do + expect_offense(<<-RUBY) + it do + expect { foo } + .to change(Foo, :bar).by(0) & + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo } + .to change { Foo.bar }.by(0) & + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + end + RUBY + + expect_no_corrections + end + + it 'registers an offense when the argument to `by` is zero ' \ + 'with compound expectations by `or`' do + expect_offense(<<-RUBY) + it do + expect { foo } + .to change(Foo, :bar).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + .or change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo } + .to change { Foo.bar }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + .or change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + end + RUBY + + expect_no_corrections + end + + it 'registers an offense when the argument to `by` is zero ' \ + 'with compound expectations by `|`' do + expect_offense(<<-RUBY) + it do + expect { foo } + .to change(Foo, :bar).by(0) | + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo } + .to change { Foo.bar }.by(0) | + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + end + RUBY + + expect_no_corrections + end + end + end + + context "with `NegatedMatcher: 'not_change'`" do + let(:cop_config) { { 'NegatedMatcher' => 'not_change' } } + + it 'registers an offense and autocorrect when ' \ + 'the argument to `by` is zero with compound expectations' do + expect_offense(<<-RUBY) it do - expect { foo } - .to change(Foo, :bar).by(0) - ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - .and change(Foo, :baz).by(0) - ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - expect { foo } - .to change { Foo.bar }.by(0) - ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - .and change { Foo.baz }.by(0) - ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - expect { foo } - .to change(Foo, :bar).by(0) & - ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - change(Foo, :baz).by(0) - ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - expect { foo } - .to change { Foo.bar }.by(0) & - ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - change { Foo.baz }.by(0) - ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - expect { foo } - .to change(Foo, :bar).by(0) - ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - .or change(Foo, :baz).by(0) - ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - expect { foo } - .to change { Foo.bar }.by(0) - ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - .or change { Foo.baz }.by(0) - ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - expect { foo } - .to change(Foo, :bar).by(0) | - ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - change(Foo, :baz).by(0) - ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - expect { foo } - .to change { Foo.bar }.by(0) | - ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. - change { Foo.baz }.by(0) - ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change(Foo, :bar).by(0).and change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. + expect { foo }.to change { Foo.bar }.by(0).and change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. end - RUBY + RUBY + + expect_correction(<<-RUBY) + it do + expect { foo }.to not_change(Foo, :bar).and not_change(Foo, :baz) + expect { foo }.to not_change { Foo.bar }.and not_change { Foo.baz } + end + RUBY + end + + it 'registers an offense and autocorrect when ' \ + 'the argument to `by` is zero with compound expectations ' \ + 'with line break' do + expect_offense(<<-RUBY) + it do + expect { foo } + .to change(Foo, :bar).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. + .and change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. + expect { foo } + .to change { Foo.bar }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. + .and change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. + end + RUBY - expect_no_corrections + expect_correction(<<-RUBY) + it do + expect { foo } + .to not_change(Foo, :bar) + .and not_change(Foo, :baz) + expect { foo } + .to not_change { Foo.bar } + .and not_change { Foo.baz } + end + RUBY + end end it 'does not register an offense when the argument to `by` is not zero' do