From aaebd28e4908f9c1b9af7642c75e244d55613f9e Mon Sep 17 00:00:00 2001 From: Tejas Bubane Date: Mon, 20 Apr 2020 23:28:17 +0530 Subject: [PATCH] Add new `RSpec/EmptyHook` cop Closes #811 --- CHANGELOG.md | 2 + config/default.yml | 6 + lib/rubocop/cop/rspec/empty_hook.rb | 44 +++ lib/rubocop/cop/rspec_cops.rb | 1 + manual/cops.md | 1 + manual/cops_rspec.md | 39 ++ spec/rubocop/cop/rspec/empty_hook_spec.rb | 418 ++++++++++++++++++++++ 7 files changed, 511 insertions(+) create mode 100644 lib/rubocop/cop/rspec/empty_hook.rb create mode 100644 spec/rubocop/cop/rspec/empty_hook_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b5a0d3230..8c01f326a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Add new `Capybara/VisibilityMatcher` cop. ([@aried3r][]) * Ignore String constants by `RSpec/Describe`. ([@AlexWayfer][]) * Drop support for ruby 2.3. ([@bquorning][]) +* Add new `RSpec/EmptyHook` cop. ([@tejasbubane][]) ## 1.38.1 (2020-02-15) @@ -497,3 +498,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@eitoball]: https://github.com/eitoball [@aried3r]: https://github.com/aried3r [@AlexWayfer]: https://github.com/AlexWayfer +[@tejasbubane]: https://github.com/tejasbubane diff --git a/config/default.yml b/config/default.yml index 2b1e87b16..b212884ef 100644 --- a/config/default.yml +++ b/config/default.yml @@ -105,6 +105,12 @@ RSpec/EmptyExampleGroup: CustomIncludeMethods: [] StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/EmptyExampleGroup +RSpec/EmptyHook: + Description: Checks for empty before and after hooks. + Enabled: true + VersionAdded: 1.38.2 + StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/EmptyHook + RSpec/EmptyLineAfterExample: Description: Checks if there is an empty line after example blocks. Enabled: true diff --git a/lib/rubocop/cop/rspec/empty_hook.rb b/lib/rubocop/cop/rspec/empty_hook.rb new file mode 100644 index 000000000..72f0b3306 --- /dev/null +++ b/lib/rubocop/cop/rspec/empty_hook.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Checks for empty before and after hooks. + # + # @example + # # bad + # before {} + # after do; end + # before(:all) do + # end + # after(:all) { } + # + # # good + # before { create_users } + # after do + # cleanup_users + # end + # before(:all) do + # create_feed + # end + # after(:all) { cleanup_feed } + class EmptyHook < Cop + MSG = 'Empty hook detected.' + + def_node_matcher :empty_hook?, <<~PATTERN + (block + $(send nil? {:prepend_before :before :append_before :around + :prepend_after :after :append_after} + (sym {:each :example :context :all :suite})?) + _ nil?) + PATTERN + + def on_block(node) + empty_hook?(node) do |hook| + add_offense(hook) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index f9b2e5d36..0231d2060 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -30,6 +30,7 @@ require_relative 'rspec/described_class_module_wrapping' require_relative 'rspec/dialect' require_relative 'rspec/empty_example_group' +require_relative 'rspec/empty_hook' require_relative 'rspec/empty_line_after_example' require_relative 'rspec/empty_line_after_example_group' require_relative 'rspec/empty_line_after_final_let' diff --git a/manual/cops.md b/manual/cops.md index f20653afe..20ddb9617 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -29,6 +29,7 @@ * [RSpec/DescribedClassModuleWrapping](cops_rspec.md#rspecdescribedclassmodulewrapping) * [RSpec/Dialect](cops_rspec.md#rspecdialect) * [RSpec/EmptyExampleGroup](cops_rspec.md#rspecemptyexamplegroup) +* [RSpec/EmptyHook](cops_rspec.md#rspecemptyhook) * [RSpec/EmptyLineAfterExample](cops_rspec.md#rspecemptylineafterexample) * [RSpec/EmptyLineAfterExampleGroup](cops_rspec.md#rspecemptylineafterexamplegroup) * [RSpec/EmptyLineAfterFinalLet](cops_rspec.md#rspecemptylineafterfinallet) diff --git a/manual/cops_rspec.md b/manual/cops_rspec.md index ca6524bee..1514b052d 100644 --- a/manual/cops_rspec.md +++ b/manual/cops_rspec.md @@ -639,6 +639,45 @@ CustomIncludeMethods | `[]` | Array * [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/EmptyExampleGroup](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/EmptyExampleGroup) +## RSpec/EmptyHook + +Enabled by default | Supports autocorrection +--- | --- +Enabled | No + +Checks for empty before and after hooks. + +### Examples + +```ruby +# bad +before {} +after do; end +before(:all) do +end +after(:all) { } + +# good +before { create_users } +after do + cleanup_users +end +before(:all) do + create_feed +end +after(:all) { cleanup_feed } +``` + +### Configurable attributes + +Name | Default value | Configurable values +--- | --- | --- +VersionAdded | `1.38.2` | String + +### References + +* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/EmptyHook](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/EmptyHook) + ## RSpec/EmptyLineAfterExample Enabled by default | Supports autocorrection diff --git a/spec/rubocop/cop/rspec/empty_hook_spec.rb b/spec/rubocop/cop/rspec/empty_hook_spec.rb new file mode 100644 index 000000000..91b3439b8 --- /dev/null +++ b/spec/rubocop/cop/rspec/empty_hook_spec.rb @@ -0,0 +1,418 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::EmptyHook do + subject(:cop) { described_class.new } + + context 'with `before` hook' do + it 'detects offense for empty `before`' do + expect_offense(<<~RUBY) + before {} + ^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `before` with :each scope' do + expect_offense(<<~RUBY) + before(:each) {} + ^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `before` with :example scope' do + expect_offense(<<~RUBY) + before(:example) {} + ^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `before` with :context scope' do + expect_offense(<<~RUBY) + before(:context) {} + ^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `before` with :all scope' do + expect_offense(<<~RUBY) + before(:all) {} + ^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `before` with :suite scope' do + expect_offense(<<~RUBY) + before(:suite) {} + ^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'accepts non-empty `before` hook' do + expect_no_offenses(<<~RUBY) + before { create_users } + RUBY + end + + it 'accepts multiline `before` hook' do + expect_no_offenses(<<~RUBY) + before(:all) do + create_users + create_products + end + RUBY + end + end + + context 'with `after` hook' do + it 'detects offense for empty `after`' do + expect_offense(<<~RUBY) + after {} + ^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `after` with :each scope' do + expect_offense(<<~RUBY) + after(:each) {} + ^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `after` with :example scope' do + expect_offense(<<~RUBY) + after(:example) {} + ^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `after` with :context scope' do + expect_offense(<<~RUBY) + after(:context) {} + ^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `after` with :all scope' do + expect_offense(<<~RUBY) + after(:all) {} + ^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `after` with :suite scope' do + expect_offense(<<~RUBY) + after(:suite) {} + ^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'accepts non-empty `after` hook' do + expect_no_offenses(<<~RUBY) + after { cleanup_users } + RUBY + end + + it 'accepts multiline `after` hook' do + expect_no_offenses(<<~RUBY) + after(:suite) do + cleanup_users + cleanup_products + end + RUBY + end + end + + context 'with `around` hook' do + it 'detects offense for empty `around`' do + expect_offense(<<~RUBY) + around {} + ^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `around` with :each scope' do + expect_offense(<<~RUBY) + around(:each) {} + ^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `around` with :example scope' do + expect_offense(<<~RUBY) + around(:example) {} + ^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `around` with :context scope' do + expect_offense(<<~RUBY) + around(:context) {} + ^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `around` with :all scope' do + expect_offense(<<~RUBY) + around(:all) {} + ^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `around` with :suite scope' do + expect_offense(<<~RUBY) + around(:suite) {} + ^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'accepts non-empty `around` hook' do + expect_no_offenses(<<~RUBY) + around { yield } + RUBY + end + + it 'accepts multiline `around` hook' do + expect_no_offenses(<<~RUBY) + around(:suite) do + setup_users + yield + end + RUBY + end + end + + context 'with `prepend_before` hook' do + it 'detects offense for empty `prepend_before`' do + expect_offense(<<~RUBY) + prepend_before {} + ^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `prepend_before` with :each scope' do + expect_offense(<<~RUBY) + prepend_before(:each) {} + ^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `prepend_before` with :example scope' do + expect_offense(<<~RUBY) + prepend_before(:example) {} + ^^^^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `prepend_before` with :context scope' do + expect_offense(<<~RUBY) + prepend_before(:context) {} + ^^^^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `prepend_before` with :all scope' do + expect_offense(<<~RUBY) + prepend_before(:all) {} + ^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `prepend_before` with :suite scope' do + expect_offense(<<~RUBY) + prepend_before(:suite) {} + ^^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'accepts non-empty `prepend_before` hook' do + expect_no_offenses(<<~RUBY) + prepend_before { create_users } + RUBY + end + + it 'accepts multiline `prepend_before` hook' do + expect_no_offenses(<<~RUBY) + prepend_before(:all) do + create_users + create_products + end + RUBY + end + end + + context 'with `append_before` hook' do + it 'detects offense for empty `append_before`' do + expect_offense(<<~RUBY) + append_before {} + ^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `append_before` with :each scope' do + expect_offense(<<~RUBY) + append_before(:each) {} + ^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `append_before` with :example scope' do + expect_offense(<<~RUBY) + append_before(:example) {} + ^^^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `append_before` with :context scope' do + expect_offense(<<~RUBY) + append_before(:context) {} + ^^^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `append_before` with :all scope' do + expect_offense(<<~RUBY) + append_before(:all) {} + ^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `append_before` with :suite scope' do + expect_offense(<<~RUBY) + append_before(:suite) {} + ^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'accepts non-empty `append_before` hook' do + expect_no_offenses(<<~RUBY) + append_before { create_users } + RUBY + end + + it 'accepts multiline `append_before` hook' do + expect_no_offenses(<<~RUBY) + append_before(:each) do + create_users + create_products + end + RUBY + end + end + + context 'with `prepend_after` hook' do + it 'detects offense for empty `prepend_after`' do + expect_offense(<<~RUBY) + prepend_after {} + ^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `prepend_after` with :each scope' do + expect_offense(<<~RUBY) + prepend_after(:each) {} + ^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `prepend_after` with :example scope' do + expect_offense(<<~RUBY) + prepend_after(:example) {} + ^^^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `prepend_after` with :context scope' do + expect_offense(<<~RUBY) + prepend_after(:context) {} + ^^^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `prepend_after` with :all scope' do + expect_offense(<<~RUBY) + prepend_after(:all) {} + ^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `prepend_after` with :suite scope' do + expect_offense(<<~RUBY) + prepend_after(:suite) {} + ^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'accepts non-empty `prepend_after` hook' do + expect_no_offenses(<<~RUBY) + prepend_after { cleanup_users } + RUBY + end + + it 'accepts multiline `prepend_after` hook' do + expect_no_offenses(<<~RUBY) + prepend_after(:all) do + cleanup_users + cleanup_products + end + RUBY + end + end + + context 'with `append_after` hook' do + it 'detects offense for empty `append_after`' do + expect_offense(<<~RUBY) + append_after {} + ^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `append_after` with :each scope' do + expect_offense(<<~RUBY) + append_after(:each) {} + ^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `append_after` with :example scope' do + expect_offense(<<~RUBY) + append_after(:example) {} + ^^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `append_after` with :context scope' do + expect_offense(<<~RUBY) + append_after(:context) {} + ^^^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `append_after` with :all scope' do + expect_offense(<<~RUBY) + append_after(:all) {} + ^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'detects offense for empty `append_after` with :suite scope' do + expect_offense(<<~RUBY) + append_after(:suite) {} + ^^^^^^^^^^^^^^^^^^^^ Empty hook detected. + RUBY + end + + it 'accepts non-empty `append_after` hook' do + expect_no_offenses(<<~RUBY) + append_after { cleanup_users } + RUBY + end + + it 'accepts multiline `append_after` hook' do + expect_no_offenses(<<~RUBY) + append_after(:all) do + cleanup_users + cleanup_products + end + RUBY + end + end +end