diff --git a/CHANGELOG.md b/CHANGELOG.md index a6423144f..1a6b9ca9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Master (Unreleased) +* Add `RSpec/ImplicitBlockExpectation` cop. ([@pirj][]) + ## 1.34.1 (2019-07-31) * Fix `RSpec/DescribedClass`'s error when a local variable is part of the namespace. ([@pirj][]) diff --git a/config/default.yml b/config/default.yml index 616af5376..2be99f104 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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 diff --git a/lib/rubocop/cop/rspec/implicit_block_expectation.rb b/lib/rubocop/cop/rspec/implicit_block_expectation.rb new file mode 100644 index 000000000..d77680029 --- /dev/null +++ b/lib/rubocop/cop/rspec/implicit_block_expectation.rb @@ -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 diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index 9cb51da31..eaed7fa9e 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -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' diff --git a/manual/cops.md b/manual/cops.md index 4cbf6f566..32e5a2d70 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -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) diff --git a/manual/cops_rspec.md b/manual/cops_rspec.md index c90fb48d2..cc2479049 100644 --- a/manual/cops_rspec.md +++ b/manual/cops_rspec.md @@ -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 diff --git a/spec/rubocop/cop/rspec/implicit_block_expectation_spec.rb b/spec/rubocop/cop/rspec/implicit_block_expectation_spec.rb new file mode 100644 index 000000000..10a1262ad --- /dev/null +++ b/spec/rubocop/cop/rspec/implicit_block_expectation_spec.rb @@ -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