Skip to content

Commit

Permalink
Add new RSpec/ChangeByZero cop
Browse files Browse the repository at this point in the history
Resolves: #1100

This cop enforces the use of `not_to change` over `to change.by(0)`

## example

```ruby
# bad
expect { run }.to change(Foo, :bar).by(0)
expect { run }.to change { Foo.bar }.by(0)

# good
expect { run }.not_to change(Foo, :bar)
expect { run }.not_to change { Foo.bar }
```
  • Loading branch information
ydah committed Apr 26, 2022
1 parent 2f8ec8a commit 146a772
Show file tree
Hide file tree
Showing 7 changed files with 204 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Master (Unreleased)

* Add new `RSpec/ChangeByZero` cop. ([@ydah][])

## 2.10.0 (2022-04-19)

* Fix a false positive for `RSpec/EmptyExampleGroup` when expectations in case statement. ([@ydah][])
Expand Down
6 changes: 6 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,12 @@ RSpec/BeforeAfterAll:
StyleGuide: https://rspec.rubystyle.guide/#avoid-hooks-with-context-scope
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/BeforeAfterAll

RSpec/ChangeByZero:
Description: Prefer negated matchers over `to change.by(0)`.
Enabled: pending
VersionAdded: 2.11.0
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ChangeByZero

RSpec/ContextMethod:
Description: "`context` should not be used for specifying methods."
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* xref:cops_rspec.adoc#rspecbeeql[RSpec/BeEql]
* xref:cops_rspec.adoc#rspecbenil[RSpec/BeNil]
* xref:cops_rspec.adoc#rspecbeforeafterall[RSpec/BeforeAfterAll]
* xref:cops_rspec.adoc#rspecchangebyzero[RSpec/ChangeByZero]
* xref:cops_rspec.adoc#rspeccontextmethod[RSpec/ContextMethod]
* xref:cops_rspec.adoc#rspeccontextwording[RSpec/ContextWording]
* xref:cops_rspec.adoc#rspecdescribeclass[RSpec/DescribeClass]
Expand Down
43 changes: 43 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,49 @@ end
* https://rspec.rubystyle.guide/#avoid-hooks-with-context-scope
* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/BeforeAfterAll

== RSpec/ChangeByZero

|===
| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed

| Pending
| Yes
| Yes
| 2.11.0
| -
|===

Prefer negated matchers over `to change.by(0)`.

=== Examples

[source,ruby]
----
# bad
expect { run }.to change(Foo, :bar).by(0)
expect { run }.to change { Foo.bar }.by(0)
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
expect { run }.not_to change(Foo, :bar)
expect { run }.not_to change { Foo.bar }
expect { run }
.to not_change(Foo, :bar)
.and not_change(Foo, :baz)
expect { run }
.to not_change { Foo.bar }
.and not_change { Foo.baz }
----

=== References

* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ChangeByZero

== RSpec/ContextMethod

|===
Expand Down
87 changes: 87 additions & 0 deletions lib/rubocop/cop/rspec/change_by_zero.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# frozen_string_literal: true

module RuboCop
module Cop
module RSpec
# Prefer negated matchers over `to change.by(0)`.
#
# @example
# # bad
# expect { run }.to change(Foo, :bar).by(0)
# expect { run }.to change { Foo.bar }.by(0)
# 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
# expect { run }.not_to change(Foo, :bar)
# expect { run }.not_to change { Foo.bar }
# 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 ' \
'over `change.by(0)`.'

# @!method expect_change_with_arguments(node)
def_node_matcher :expect_change_with_arguments, <<-PATTERN
(send
(send nil? :change ...) :by
(int 0))
PATTERN

# @!method expect_change_with_block(node)
def_node_matcher :expect_change_with_block, <<-PATTERN
(send
(block
(send nil? :change)
(args)
(send (...) $_)) :by
(int 0))
PATTERN

def on_send(node)
expect_change_with_arguments(node) do
check_offence(node)
end

expect_change_with_block(node) do
check_offence(node)
end
end

private

def check_offence(node)
expression = node.loc.expression
if compound_expectations?(node)
add_offense(expression, message: MSG_COMPOUND)
else
add_offense(expression) do |corrector|
autocorrect(corrector, node)
end
end
end

def compound_expectations?(node)
%i[and or].include?(node.parent.method_name)
end

def autocorrect(corrector, node)
corrector.replace(node.parent.loc.selector, 'not_to')
range = node.loc.dot.with(end_pos: node.loc.expression.end_pos)
corrector.remove(range)
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rspec_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
require_relative 'rspec/be_eql'
require_relative 'rspec/be_nil'
require_relative 'rspec/before_after_all'
require_relative 'rspec/change_by_zero'
require_relative 'rspec/context_method'
require_relative 'rspec/context_wording'
require_relative 'rspec/describe_class'
Expand Down
64 changes: 64 additions & 0 deletions spec/rubocop/cop/rspec/change_by_zero_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::RSpec::ChangeByZero do
it 'registers an offense when the argument to `by` is zero' do
expect_offense(<<-RUBY)
it do
expect { foo }.to change(Foo, :bar).by(0)
^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
expect { foo }.to change(::Foo, :bar).by(0)
^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
expect { foo }.to change { Foo.bar }.by(0)
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
expect { foo }.to change(Foo, :bar).by 0
^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
end
RUBY

expect_correction(<<-RUBY)
it do
expect { foo }.not_to change(Foo, :bar)
expect { foo }.not_to change(::Foo, :bar)
expect { foo }.not_to change { Foo.bar }
expect { foo }.not_to change(Foo, :bar)
end
RUBY
end

it 'registers an offense 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)`.
.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
end

it 'does not register an offense when the argument to `by` is not zero' do
expect_no_offenses(<<-RUBY)
it do
expect { foo }.to change(Foo, :bar).by(1)
expect { foo }.to change { Foo.bar }.by(1)
end
RUBY
end
end

0 comments on commit 146a772

Please sign in to comment.