Skip to content

Commit

Permalink
Merge pull request #1630 from ydah/ydah/support-change-by-zero
Browse files Browse the repository at this point in the history
Add support `a_block_changing` and `changing` for `RSpec/ChangeByZero`
  • Loading branch information
pirj authored Apr 25, 2023
2 parents 7226265 + 6b1ea4c commit 53db62b
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 21 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 @@
- Fix a false positive in `RSpec/IndexedLet` with suffixes after index-like numbers. ([@pirj])
- Fix an error for `RSpec/Rails/HaveHttpStatus` with comparison with strings containing non-numeric characters. ([@ydah])
- Fix an error for `RSpec/MatchArray` when `match_array` with no argument. ([@ydah])
- Add support `a_block_changing` and `changing` for `RSpec/ChangeByZero`. ([@ydah])

## 2.20.0 (2023-04-18)

Expand Down
50 changes: 30 additions & 20 deletions lib/rubocop/cop/rspec/change_by_zero.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,64 +59,78 @@ module RSpec
#
class ChangeByZero < Base
extend AutoCorrector
MSG = 'Prefer `not_to change` over `to change.by(0)`.'
MSG = 'Prefer `not_to change` over `to %<method>s.by(0)`.'
MSG_COMPOUND = 'Prefer %<preferred>s with compound expectations ' \
'over `change.by(0)`.'
RESTRICT_ON_SEND = %i[change].freeze
'over `%<method>s.by(0)`.'
CHANGE_METHODS = Set[:change, :a_block_changing, :changing].freeze
RESTRICT_ON_SEND = CHANGE_METHODS.freeze

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

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

# @!method change_nodes(node)
def_node_search :change_nodes, <<-PATTERN
$(send nil? :change ...)
$(send nil? CHANGE_METHODS ...)
PATTERN

def on_send(node)
expect_change_with_arguments(node.parent) do
check_offense(node.parent)
expect_change_with_arguments(node.parent) do |change|
register_offense(node.parent, change)
end

expect_change_with_block(node.parent.parent) do
check_offense(node.parent.parent)
expect_change_with_block(node.parent.parent) do |change|
register_offense(node.parent.parent, change)
end
end

private

def check_offense(node)
expression = node.source_range
# rubocop:disable Metrics/MethodLength
def register_offense(node, change_node)
if compound_expectations?(node)
add_offense(expression, message: message_compound) do |corrector|
add_offense(node.source_range,
message: message_compound(change_node)) do |corrector|
autocorrect_compound(corrector, node)
end
else
add_offense(expression) do |corrector|
autocorrect(corrector, node)
add_offense(node.source_range,
message: message(change_node)) do |corrector|
autocorrect(corrector, node, change_node)
end
end
end
# rubocop:enable Metrics/MethodLength

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

def autocorrect(corrector, node)
def message(change_node)
format(MSG, method: change_node.method_name)
end

def message_compound(change_node)
format(MSG_COMPOUND, preferred: preferred_method,
method: change_node.method_name)
end

def autocorrect(corrector, node, change_node)
corrector.replace(node.parent.loc.selector, 'not_to')
corrector.replace(change_node.loc.selector, 'change')
range = node.loc.dot.with(end_pos: node.source_range.end_pos)
corrector.remove(range)
end
Expand All @@ -135,10 +149,6 @@ 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
Expand Down
34 changes: 33 additions & 1 deletion spec/rubocop/cop/rspec/change_by_zero_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::RSpec::ChangeByZero, :config do
it 'registers an offense when the argument to `by` is zero' do
it 'registers an offense when using `change` and argument to `by` is zero' do
expect_offense(<<-RUBY)
it do
expect { foo }.to change(Foo, :bar).by(0)
Expand All @@ -25,6 +25,38 @@
RUBY
end

it 'registers an offense when using `a_block_changing` ' \
'and argument to `by` is zero' do
expect_offense(<<-RUBY)
it do
expect { foo }.to a_block_changing(Foo, :bar).by(0)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to a_block_changing.by(0)`.
end
RUBY

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

it 'registers an offense when using `changing` ' \
'and argument to `by` is zero' do
expect_offense(<<-RUBY)
it do
expect { foo }.to changing(Foo, :bar).by(0)
^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to changing.by(0)`.
end
RUBY

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

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
Expand Down

0 comments on commit 53db62b

Please # to comment.