Add new RSpec/ExpectInHook cop (#445)

Author: pocke

CHANGELOG.md

@@ -12,6 +12,7 @@
 * Add `RSpec/InvalidPredicateMatcher` cop. ([@pocke][])
 * Change HookArgument cop to detect when hook has a receiver. ([@pocke][])
 * Add `RSpec/PredicateMatcher` cop. ([@pocke][])
+* Add `RSpec/ExpectInHook` cop. ([@pocke][])
 
 ## 1.15.1 (2017-04-30)
 

config/default.yml

@@ -113,6 +113,11 @@ RSpec/ExpectActual:
   - spec/routing/**/*
   StyleGuide: http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ExpectActual
 
+RSpec/ExpectInHook:
+  Enabled: true
+  Description: Do not use `expect` in hooks such as `before`.
+  StyleGuide: http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ExpectInHook
+
 RSpec/ExpectOutput:
   Description: Checks for opportunities to use `expect { ... }.to output`.
   Enabled: true

lib/rubocop-rspec.rb

@@ -38,6 +38,7 @@
 require 'rubocop/cop/rspec/example_length'
 require 'rubocop/cop/rspec/example_wording'
 require 'rubocop/cop/rspec/expect_actual'
+require 'rubocop/cop/rspec/expect_in_hook'
 require 'rubocop/cop/rspec/expect_output'
 require 'rubocop/cop/rspec/factory_girl/dynamic_attribute_defined_statically'
 require 'rubocop/cop/rspec/file_path'

lib/rubocop/cop/rspec/expect_in_hook.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Do not use `expect` in hooks such as `before`.
+      #
+      # @example
+      #   # bad
+      #   before do
+      #     expect(something).to eq 'foo'
+      #   end
+      #
+      #   # bad
+      #   after do
+      #     expect_any_instance_of(Something).to receive(:foo)
+      #   end
+      #
+      #   # good
+      #   it do
+      #     expect(something).to eq 'foo'
+      #   end
+      class ExpectInHook < Cop
+        MSG = 'Do not use `%<expect>s` in `%<hook>s` hook'.freeze
+        HOOKS = Hooks::ALL.node_pattern_union.freeze
+
+        def_node_matcher :hook, <<-PATTERN
+          (block (send _ $#{HOOKS} ...) _ $!nil)
+        PATTERN
+
+        def_node_search :expect, <<-PATTERN
+          {
+            #{Expectations::ALL.send_pattern}
+            #{Expectations::ALL.block_pattern}
+          }
+        PATTERN
+
+        def on_block(node)
+          hook(node) do |hook_name, body|
+            expect(body) do |expect|
+              method = send_node(expect)
+              add_offense(method, :selector,
+                          message(method, hook_name))
+            end
+          end
+        end
+
+        private
+
+        def message(expect, hook)
+          format(MSG, expect: expect.method_name, hook: hook)
+        end
+
+        def send_node(node)
+          return node if node.send_type?
+          node.children.first
+        end
+      end
+    end
+  end
+end

lib/rubocop/rspec/language.rb

@@ -106,6 +106,10 @@ module Subject
         ALL = SelectorSet.new(%i[subject subject!])
       end
 
+      module Expectations
+        ALL = SelectorSet.new(%i[expect expect_any_instance_of])
+      end
+
       ALL =
         ExampleGroups::ALL +
         SharedGroups::ALL  +

spec/rubocop/cop/rspec/expect_in_hook_spec.rb

@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::RSpec::ExpectInHook do
+  subject(:cop) { described_class.new }
+
+  it 'adds an offense for `expect` in `before` hook' do
+    expect_offense(<<-RUBY)
+      before do
+        expect(something).to eq('foo')
+        ^^^^^^ Do not use `expect` in `before` hook
+        expect_any_instance_of(Something).to receive(:foo)
+        ^^^^^^^^^^^^^^^^^^^^^^ Do not use `expect_any_instance_of` in `before` hook
+      end
+    RUBY
+  end
+
+  it 'adds an offense for `expect` in `after` hook' do
+    expect_offense(<<-RUBY)
+      after do
+        expect(something).to eq('foo')
+        ^^^^^^ Do not use `expect` in `after` hook
+        expect_any_instance_of(Something).to receive(:foo)
+        ^^^^^^^^^^^^^^^^^^^^^^ Do not use `expect_any_instance_of` in `after` hook
+      end
+    RUBY
+  end
+
+  it 'adds an offense for `expect` in `around` hook' do
+    expect_offense(<<-RUBY)
+      around do
+        expect(something).to eq('foo')
+        ^^^^^^ Do not use `expect` in `around` hook
+        expect_any_instance_of(Something).to receive(:foo)
+        ^^^^^^^^^^^^^^^^^^^^^^ Do not use `expect_any_instance_of` in `around` hook
+      end
+    RUBY
+  end
+
+  it 'adds an offense for `expect` with block in `before` hook' do
+    expect_offense(<<-RUBY)
+      before do
+        expect { something }.to eq('foo')
+        ^^^^^^ Do not use `expect` in `before` hook
+      end
+    RUBY
+  end
+
+  it 'accepts an empty `before` hook' do
+    expect_no_offenses(<<-RUBY)
+      before do
+      end
+    RUBY
+  end
+
+  it 'accepts `allow` in `before` hook' do
+    expect_no_offenses(<<-RUBY)
+      before do
+        allow(something).to receive(:foo)
+        allow_any_instance_of(something).to receive(:foo)
+      end
+    RUBY
+  end
+
+  it 'accepts `expect` in `it`' do
+    expect_no_offenses(<<-RUBY)
+      it do
+        expect(something).to eq('foo')
+        expect_any_instance_of(something).to receive(:foo)
+      end
+    RUBY
+  end
+end