Merge pull request #789 from pirj/add-implicit-block-expectation-cop

Add ImplicitBlockExpectation cop

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][])

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

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

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'

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)

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

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