From 6647b14f7327b6a73b5ee4e9b557ca8f0777173e Mon Sep 17 00:00:00 2001
From: Masataka Pocke Kuwabara <kuwabara@pocke.me>
Date: Thu, 17 Aug 2017 23:42:29 +0900
Subject: [PATCH] Add new `RSpec/ExpectInHook` cop

---
 CHANGELOG.md                                  |  1 +
 config/default.yml                            |  5 ++
 lib/rubocop-rspec.rb                          |  1 +
 lib/rubocop/cop/rspec/expect_in_hook.rb       | 57 +++++++++++++++++
 spec/rubocop/cop/rspec/expect_in_hook_spec.rb | 63 +++++++++++++++++++
 5 files changed, 127 insertions(+)
 create mode 100644 lib/rubocop/cop/rspec/expect_in_hook.rb
 create mode 100644 spec/rubocop/cop/rspec/expect_in_hook_spec.rb

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0a5bcb076..60ef890de 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@
 * Add `RSpec/VoidExpect` 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)
 
diff --git a/config/default.yml b/config/default.yml
index d9c2d2ac3..f6139a783 100644
--- a/config/default.yml
+++ b/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
diff --git a/lib/rubocop-rspec.rb b/lib/rubocop-rspec.rb
index a70e095ca..88b815c53 100644
--- a/lib/rubocop-rspec.rb
+++ b/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'
diff --git a/lib/rubocop/cop/rspec/expect_in_hook.rb b/lib/rubocop/cop/rspec/expect_in_hook.rb
new file mode 100644
index 000000000..3389780b4
--- /dev/null
+++ b/lib/rubocop/cop/rspec/expect_in_hook.rb
@@ -0,0 +1,57 @@
+# 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
+      #
+      #   # good
+      #   it do
+      #     expect_any_instance_of(Something).to receive(:foo)
+      #   end
+      class ExpectInHook < Cop
+        MSG = 'Do not use `%<expect>s` in `%<hook>s` hook'.freeze
+        HOOKS = Hooks::ALL.node_pattern_union.freeze
+
+        def on_block(node)
+          hook(node) do |hook_name, body|
+            expect(body) do |expect|
+              add_offense(expect, :selector,
+                          message(expect.method_name, hook_name))
+            end
+          end
+        end
+
+        private
+
+        def message(expect, hook)
+          format(MSG, expect: expect, hook: hook)
+        end
+
+        def_node_matcher :hook, <<-PATTERN
+          (block (send _ $#{HOOKS} ...) _ $!nil)
+        PATTERN
+
+        def_node_search :expect, <<-PATTERN
+          (send nil {:expect :expect_any_instance_of} ...)
+        PATTERN
+      end
+    end
+  end
+end
diff --git a/spec/rubocop/cop/rspec/expect_in_hook_spec.rb b/spec/rubocop/cop/rspec/expect_in_hook_spec.rb
new file mode 100644
index 000000000..cc1e1b301
--- /dev/null
+++ b/spec/rubocop/cop/rspec/expect_in_hook_spec.rb
@@ -0,0 +1,63 @@
+# 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 '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