Skip to content

Commit

Permalink
Add ImplicitBlockExpectation cop
Browse files Browse the repository at this point in the history
  • Loading branch information
pirj committed Jul 30, 2019
1 parent 6420b04 commit 7f135df
Show file tree
Hide file tree
Showing 7 changed files with 234 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Master (Unreleased)

* Fix `RSpec/DescribedClass`'s error when a local variable is part of the namespace. ([@pirj][])
* Add `RSpec/ImplicitBlockExpectation` cop. ([@pirj][])

## 1.34.0 (2019-07-23)

Expand Down
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,11 @@ RSpec/HooksBeforeExamples:
Description: Checks for before/around/after hooks that come after an example.
StyleGuide: http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/HooksBeforeExamples

RSpec/ImplicitBlockExpectation:
Description: Check that implicit block expectation syntax is not used.
Enabled: true
StyleGuide: http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ImplicitBlockExpectation

RSpec/ImplicitExpect:
Description: Check that a consistent implicit expectation style is used.
Enabled: true
Expand Down
64 changes: 64 additions & 0 deletions lib/rubocop/cop/rspec/implicit_block_expectation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# frozen_string_literal: true

module RuboCop
module Cop
module RSpec
# Check that implicit block expectation syntax is not used.
#
# Prefer using explicit block expectations.
#
# @example
# # bad
# subject { -> { do_something } }
# it { is_expected.to change(something).to(new_value) }
#
# # good
# it 'changes something to a new value' do
# expect { do_something }.to change(something).to(new_value)
# end
class ImplicitBlockExpectation < Cop
MSG = 'Avoid implicit block expectations.'

def_node_matcher :lambda?, <<-PATTERN
{
(send (const nil? :Proc) :new)
(send nil? :proc)
(send nil? :lambda)
}
PATTERN

def_node_matcher :lambda_subject?, '(block #lambda? ...)'

def_node_matcher :implicit_expect, <<-PATTERN
$(send nil? {:is_expected :should :should_not} ...)
PATTERN

def on_send(node)
implicit_expect(node) do |implicit_expect|
subject = nearest_subject(implicit_expect)
add_offense(implicit_expect) if lambda_subject?(subject&.body)
end
end

private

def nearest_subject(node)
node
.each_ancestor(:block)
.lazy
.select { |block_node| multi_statement_example_group?(block_node) }
.map { |block_node| find_subject(block_node) }
.find(&:itself)
end

def multi_statement_example_group?(node)
example_group_with_body?(node) && node.body.begin_type?
end

def find_subject(block_node)
block_node.body.child_nodes.find { |send_node| subject?(send_node) }
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 @@ -41,6 +41,7 @@
require_relative 'rspec/focus'
require_relative 'rspec/hook_argument'
require_relative 'rspec/hooks_before_examples'
require_relative 'rspec/implicit_block_expectation'
require_relative 'rspec/implicit_expect'
require_relative 'rspec/implicit_subject'
require_relative 'rspec/instance_spy'
Expand Down
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
* [RSpec/Focus](cops_rspec.md#rspecfocus)
* [RSpec/HookArgument](cops_rspec.md#rspechookargument)
* [RSpec/HooksBeforeExamples](cops_rspec.md#rspechooksbeforeexamples)
* [RSpec/ImplicitBlockExpectation](cops_rspec.md#rspecimplicitblockexpectation)
* [RSpec/ImplicitExpect](cops_rspec.md#rspecimplicitexpect)
* [RSpec/ImplicitSubject](cops_rspec.md#rspecimplicitsubject)
* [RSpec/InstanceSpy](cops_rspec.md#rspecinstancespy)
Expand Down
27 changes: 27 additions & 0 deletions manual/cops_rspec.md
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,33 @@ end

* [http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/HooksBeforeExamples](http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/HooksBeforeExamples)

## RSpec/ImplicitBlockExpectation

Enabled by default | Supports autocorrection
--- | ---
Enabled | No

Check that implicit block expectation syntax is not used.

Prefer using explicit block expectations.

### Examples

```ruby
# bad
subject { -> { do_something } }
it { is_expected.to change(something).to(new_value) }

# good
it 'changes something to a new value' do
expect { do_something }.to change(something).to(new_value)
end
```

### References

* [http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ImplicitBlockExpectation](http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ImplicitBlockExpectation)

## RSpec/ImplicitExpect

Enabled by default | Supports autocorrection
Expand Down
135 changes: 135 additions & 0 deletions spec/rubocop/cop/rspec/implicit_block_expectation_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::RSpec::ImplicitBlockExpectation do
subject(:cop) { described_class.new }

it 'flags lambda in subject' do
expect_offense(<<-RUBY)
describe do
subject { -> { boom } }
it { is_expected.to change { something }.to(new_value) }
^^^^^^^^^^^ Avoid implicit block expectations.
end
RUBY
end

it 'ignores non-lambda subject' do
expect_no_offenses(<<-RUBY)
describe do
subject { 'normal' }
it { is_expected.to eq(something) }
end
RUBY
end

it 'flags lambda in subject!' do
expect_offense(<<-RUBY)
describe do
subject! { -> { boom } }
it { is_expected.to change { something }.to(new_value) }
^^^^^^^^^^^ Avoid implicit block expectations.
end
RUBY
end

it 'flags literal lambda' do
expect_offense(<<-RUBY)
describe do
subject! { lambda { boom } }
it { is_expected.to change { something }.to(new_value) }
^^^^^^^^^^^ Avoid implicit block expectations.
end
RUBY
end

it 'flags proc' do
expect_offense(<<-RUBY)
describe do
subject! { proc { boom } }
it { is_expected.to change { something }.to(new_value) }
^^^^^^^^^^^ Avoid implicit block expectations.
end
RUBY
end

it 'flags Proc.new' do
expect_offense(<<-RUBY)
describe do
subject! { Proc.new { boom } }
it { is_expected.to change { something }.to(new_value) }
^^^^^^^^^^^ Avoid implicit block expectations.
end
RUBY
end

it 'flags named subject' do
expect_offense(<<-RUBY)
describe do
subject(:name) { -> { boom } }
it { is_expected.to change { something }.to(new_value) }
^^^^^^^^^^^ Avoid implicit block expectations.
end
RUBY
end

it 'flags when subject is defined in the outer example group' do
expect_offense(<<-RUBY)
describe do
subject { -> { boom } }
context do
it { is_expected.to change { something }.to(new_value) }
^^^^^^^^^^^ Avoid implicit block expectations.
end
end
RUBY
end

it 'ignores normal local subject' do
expect_no_offenses(<<-RUBY)
describe do
subject { -> { boom } }
context do
subject { 'normal' }
it { is_expected.to eq(something) }
end
end
RUBY
end

it 'ignores named subject with deeply nested lambda' do
expect_no_offenses(<<-RUBY)
describe do
subject { {hash: -> { boom }} }
it { is_expected.to be(something) }
end
RUBY
end

it 'flags with `should` as implicit subject' do
expect_offense(<<-RUBY)
describe do
subject { -> { boom } }
it { should change { something }.to(new_value) }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid implicit block expectations.
end
RUBY
end

it 'flags with `should_not` as implicit subject' do
expect_offense(<<-RUBY)
describe do
subject { -> { boom } }
it { should_not change { something }.to(new_value) }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid implicit block expectations.
end
RUBY
end

it 'ignores when there is no subject defined' do
expect_no_offenses(<<-RUBY)
shared_examples 'subject is defined somewhere else' do
it { is_expected.to change { something }.to(new_value) }
end
RUBY
end
end

0 comments on commit 7f135df

Please sign in to comment.