From e33f3c7c314ac92cb1bd0a6199aa8d528e4dc689 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