From 30358b1d8ff3f7d922a1c99085e97df5794dc1ff Mon Sep 17 00:00:00 2001
From: Masataka Kuwabara
Date: Mon, 21 Aug 2017 05:13:14 +0900
Subject: [PATCH] Add new `RSpec/ExpectInHook` cop (#445)
---
CHANGELOG.md | 1 +
config/default.yml | 5 ++
lib/rubocop-rspec.rb | 1 +
lib/rubocop/cop/rspec/expect_in_hook.rb | 61 ++++++++++++++++
lib/rubocop/rspec/language.rb | 4 ++
spec/rubocop/cop/rspec/expect_in_hook_spec.rb | 72 +++++++++++++++++++
6 files changed, 144 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 a1a1ce178..7e0d29aeb 100644
--- a/CHANGELOG.md
+++ b/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)
diff --git a/config/default.yml b/config/default.yml
index 23ccd1728..34c81d061 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 fa4b851cd..cc43cec2b 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..0683c297a
--- /dev/null
+++ b/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 `%s` in `%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
diff --git a/lib/rubocop/rspec/language.rb b/lib/rubocop/rspec/language.rb
index cc26e7a80..8eb9c2427 100644
--- a/lib/rubocop/rspec/language.rb
+++ b/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 +
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..b6c645b40
--- /dev/null
+++ b/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