Skip to content

Commit 32f83e2

Browse files
committed
Fix RSpec/ChangeByZero with compound operators
Previously, it didn't recognize compound expressions using `&` or `|`. This led it to correct part of the expressions as regular code, but the generated code was malformed. Now it recognizes compound expressions using the operators. Also, its correction was wrong if `change` `by(0)` was used with `to_not` or `not_to` already. It no longer makes that correction.
1 parent c8a11ce commit 32f83e2

File tree

3 files changed

+46
-1
lines changed

3 files changed

+46
-1
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
* Fix `RSpec/FilePath` cop missing mismatched expanded namespace. ([@sl4vr][])
66
* Add new `AllowConsecutiveOneLiners` (default true) option for `Rspec/EmptyLineAfterHook` cop. ([@ngouy][])
77
* Add autocorrect support for `RSpec/EmptyExampleGroup`. ([@r7kamura][])
8+
* Fix `RSpec/ChangeByZero` with compound expressions using `&` or `|` operators. ([@BrianHawley][])
9+
* Fix `RSpec/ChangeByZero` bad correction when already `not_to` or `to_not`. ([@BrianHawley][])
810

911
## 2.12.1 (2022-07-03)
1012

@@ -718,3 +720,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
718720
[@edgibbs]: https://github.com/edgibbs
719721
[@Drowze]: https://github.com/Drowze
720722
[@akiomik]: https://github.com/akiomik
723+
[@BrianHawley]: https://github.com/BrianHawley

lib/rubocop/cop/rspec/change_by_zero.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,15 @@ def check_offense(node)
7373
add_offense(expression, message: MSG_COMPOUND)
7474
else
7575
add_offense(expression) do |corrector|
76+
next unless node.parent.method?(:to)
77+
7678
autocorrect(corrector, node)
7779
end
7880
end
7981
end
8082

8183
def compound_expectations?(node)
82-
%i[and or].include?(node.parent.method_name)
84+
%i[and or & |].include?(node.parent.method_name)
8385
end
8486

8587
def autocorrect(corrector, node)

spec/rubocop/cop/rspec/change_by_zero_spec.rb

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,24 @@
2525
RUBY
2626
end
2727

28+
it 'registers an offense when the argument to `by` is zero, ' \
29+
'but does not correct if it is `not_to` `change` already' do
30+
expect_offense(<<-RUBY)
31+
it do
32+
expect { foo }.not_to change(Foo, :bar).by(0)
33+
^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
34+
expect { foo }.not_to change(::Foo, :bar).by(0)
35+
^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
36+
expect { foo }.not_to change { Foo.bar }.by(0)
37+
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
38+
expect { foo }.not_to change(Foo, :bar).by 0
39+
^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
40+
end
41+
RUBY
42+
43+
expect_no_corrections
44+
end
45+
2846
it 'registers an offense when the argument to `by` is zero ' \
2947
'with compound expectations' do
3048
expect_offense(<<-RUBY)
@@ -39,6 +57,16 @@
3957
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
4058
.and change { Foo.baz }.by(0)
4159
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
60+
expect { foo }
61+
.to change(Foo, :bar).by(0) &
62+
^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
63+
change(Foo, :baz).by(0)
64+
^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
65+
expect { foo }
66+
.to change { Foo.bar }.by(0) &
67+
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
68+
change { Foo.baz }.by(0)
69+
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
4270
expect { foo }
4371
.to change(Foo, :bar).by(0)
4472
^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
@@ -49,8 +77,20 @@
4977
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
5078
.or change { Foo.baz }.by(0)
5179
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
80+
expect { foo }
81+
.to change(Foo, :bar).by(0) |
82+
^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
83+
change(Foo, :baz).by(0)
84+
^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
85+
expect { foo }
86+
.to change { Foo.bar }.by(0) |
87+
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
88+
change { Foo.baz }.by(0)
89+
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
5290
end
5391
RUBY
92+
93+
expect_no_corrections
5494
end
5595

5696
it 'does not register an offense when the argument to `by` is not zero' do

0 commit comments

Comments
 (0)