diff --git a/config/default.yml b/config/default.yml index 30f06c904..597ef7c63 100644 --- a/config/default.yml +++ b/config/default.yml @@ -347,6 +347,13 @@ RSpec/LetSetup: VersionAdded: '1.7' StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/LetSetup +RSpec/MemoizedHelpersInExampleGroup: + Description: Checks if example groups contain too many `let` and `subject` calls. + Enabled: false + AllowSubject: false + Max: 5 + StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MemoizedHelpersInExampleGroup + RSpec/MessageChain: Description: Check that chains of messages are not being stubbed. Enabled: true @@ -415,12 +422,6 @@ RSpec/NestedGroups: VersionChanged: '1.10' StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NestedGroups -RSpec/MemoizedHelpersInExampleGroup: - Description: Checks for usage of `let` blocks in specs. - Enabled: false - AllowSubject: false - StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MemoizedHelpersInExampleGroup - RSpec/NotToNot: Description: Checks for consistent method usage for negating expectations. Enabled: true diff --git a/lib/rubocop/cop/rspec/memoized_helpers_in_example_group.rb b/lib/rubocop/cop/rspec/memoized_helpers_in_example_group.rb index 06ae21a86..e18b36cf4 100644 --- a/lib/rubocop/cop/rspec/memoized_helpers_in_example_group.rb +++ b/lib/rubocop/cop/rspec/memoized_helpers_in_example_group.rb @@ -3,29 +3,55 @@ module RuboCop module Cop module RSpec - # Checks for usage of `let` blocks in specs. + # Checks if example groups contain too many `let` and `subject` calls. # - # This cop can be configured with the option `AllowSubject` which - # will configure the cop to only register offenses on explicit calls to - # `let` and not calls to `subject` + # This cop is configurable using the `Max` option and the `AllowSubject` + # which will configure the cop to only register offenses on calls to + # `let` and not calls to `subject`. # # @example # # bad # describe MyClass do # let(:foo) { [] } - # it { expect(foo).to be_empty } + # let(:bar) { [] } + # let!(:booger) { [] } + # subject { {} } + # subject(:wat) { {} } + # subject!(:boo) { {} } # end # # describe MyClass do - # subject(:foo) { [] } - # it { expect(foo).to be_empty } + # let(:foo) { [] } + # let(:bar) { [] } + # + # context 'when stuff' do + # let!(:booger) { [] } + # subject { {} } + # subject(:wat) { {} } + # subject!(:boo) { {} } + # end # end # # # good # describe MyClass do - # it do - # foo = [] - # expect(foo).to be_empty + # let(:foo) { [] } + # let!(:booger) { [] } + # subject { {} } + # subject(:wat) { {} } + # subject!(:boo) { {} } + # end + # + # describe MyClass do + # context 'when stuff' do + # let(:foo) { [] } + # let(:bar) { [] } + # let!(:booger) { [] } + # end + # + # context 'when other stuff' do + # subject { {} } + # subject(:wat) { {} } + # subject!(:boo) { {} } # end # end # @@ -38,39 +64,69 @@ module RSpec # # bad # describe MyClass do # let(:foo) { [] } - # it { expect(foo).to be_empty } + # let(:bar) { [] } + # let!(:booger) { [] } + # let(:subject) { {} } + # let(:wat) { {} } + # let!(:boo) { {} } # end # # # good # describe MyClass do - # subject(:foo) { [] } - # it { expect(foo).to be_empty } + # let(:foo) { [] } + # let(:bar) { [] } + # let!(:booger) { [] } + # subject { {} } + # subject(:wat) { {} } + # subject!(:boo) { {} } + # end + # + # @example with Max configuration + # + # # rubocop.yml + # # RSpec/MemoizedHelpersInExampleGroup: + # # Max: 0 + # + # # bad + # describe MyClass do + # let(:foo) { [] } # end # + # # good # describe MyClass do - # it do - # foo = [] - # expect(foo).to be_empty - # end + # def foo; []; end # end # class MemoizedHelpersInExampleGroup < Cop - MSG = 'Avoid using `%s` ' \ - '– use a method call or local variable instead.' + MSG = 'Example has too many memoized helpers [%d/%d]' - def_node_matcher :let?, Helpers::ALL.send_pattern - def_node_matcher :subject?, Subject::ALL.send_pattern + def on_block(node) + return unless spec_group?(node) - def on_send(node) - if subject?(node) && !allow_subject? - add_offense(node, message: format(MSG, method: 'subject')) - elsif let?(node) - add_offense(node, message: format(MSG, method: 'let')) + count = count_helpers(node) + + node.each_ancestor(:block) do |ancestor| + count += count_helpers(ancestor) end + + return if count <= max + + add_offense(node, message: format(MSG, count: count, max: max)) end private + def count_helpers(node) + example_group = RuboCop::RSpec::ExampleGroup.new(node) + count = example_group.lets.count + count += example_group.subjects.count unless allow_subject? + count + end + + def max + cop_config['Max'] + end + def allow_subject? cop_config['AllowSubject'] end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index 157b546b1..5ecabb42b 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -59,6 +59,7 @@ require_relative 'rspec/leaky_constant_declaration' require_relative 'rspec/let_before_examples' require_relative 'rspec/let_setup' +require_relative 'rspec/memoized_helpers_in_example_group' require_relative 'rspec/message_chain' require_relative 'rspec/message_expectation' require_relative 'rspec/message_spies' @@ -68,7 +69,6 @@ require_relative 'rspec/multiple_subjects' require_relative 'rspec/named_subject' require_relative 'rspec/nested_groups' -require_relative 'rspec/no_let' require_relative 'rspec/not_to_not' require_relative 'rspec/overwriting_setup' require_relative 'rspec/pending' diff --git a/manual/cops.md b/manual/cops.md index 9db4f9f50..5f57c2636 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -58,6 +58,7 @@ * [RSpec/LeakyConstantDeclaration](cops_rspec.md#rspecleakyconstantdeclaration) * [RSpec/LetBeforeExamples](cops_rspec.md#rspecletbeforeexamples) * [RSpec/LetSetup](cops_rspec.md#rspecletsetup) +* [RSpec/MemoizedHelpersInExampleGroup](cops_rspec.md#rspecmemoizedhelpersinexamplegroup) * [RSpec/MessageChain](cops_rspec.md#rspecmessagechain) * [RSpec/MessageExpectation](cops_rspec.md#rspecmessageexpectation) * [RSpec/MessageSpies](cops_rspec.md#rspecmessagespies) @@ -67,7 +68,6 @@ * [RSpec/MultipleSubjects](cops_rspec.md#rspecmultiplesubjects) * [RSpec/NamedSubject](cops_rspec.md#rspecnamedsubject) * [RSpec/NestedGroups](cops_rspec.md#rspecnestedgroups) -* [RSpec/MemoizedHelpersInExampleGroup](cops_rspec.md#rspecnolet) * [RSpec/NotToNot](cops_rspec.md#rspecnottonot) * [RSpec/OverwritingSetup](cops_rspec.md#rspecoverwritingsetup) * [RSpec/Pending](cops_rspec.md#rspecpending) diff --git a/manual/cops_rspec.md b/manual/cops_rspec.md index 7162dd635..e6976d713 100644 --- a/manual/cops_rspec.md +++ b/manual/cops_rspec.md @@ -1890,6 +1890,122 @@ end * [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/LetSetup](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/LetSetup) +## RSpec/MemoizedHelpersInExampleGroup + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +--- | --- | --- | --- | --- +Disabled | Yes | No | - | - + +Checks if example groups contain too many `let` and `subject` calls. + +This cop is configurable using the `Max` option and the `AllowSubject` +which will configure the cop to only register offenses on calls to +`let` and not calls to `subject`. + +### Examples + +```ruby +# bad +describe MyClass do + let(:foo) { [] } + let(:bar) { [] } + let!(:booger) { [] } + subject { {} } + subject(:wat) { {} } + subject!(:boo) { {} } +end + +describe MyClass do + let(:foo) { [] } + let(:bar) { [] } + + context 'when stuff' do + let!(:booger) { [] } + subject { {} } + subject(:wat) { {} } + subject!(:boo) { {} } + end +end + +# good +describe MyClass do + let(:foo) { [] } + let!(:booger) { [] } + subject { {} } + subject(:wat) { {} } + subject!(:boo) { {} } +end + +describe MyClass do + context 'when stuff' do + let(:foo) { [] } + let(:bar) { [] } + let!(:booger) { [] } + end + + context 'when other stuff' do + subject { {} } + subject(:wat) { {} } + subject!(:boo) { {} } + end +end +``` +#### with AllowSubject configuration + +```ruby +# rubocop.yml +# RSpec/MemoizedHelpersInExampleGroup: +# AllowSubject: true + +# bad +describe MyClass do + let(:foo) { [] } + let(:bar) { [] } + let!(:booger) { [] } + let(:subject) { {} } + let(:wat) { {} } + let!(:boo) { {} } +end + +# good +describe MyClass do + let(:foo) { [] } + let(:bar) { [] } + let!(:booger) { [] } + subject { {} } + subject(:wat) { {} } + subject!(:boo) { {} } +end +``` +#### with Max configuration + +```ruby +# rubocop.yml +# RSpec/MemoizedHelpersInExampleGroup: +# Max: 0 + +# bad +describe MyClass do + let(:foo) { [] } +end + +# good +describe MyClass do + def foo; []; end +end +``` + +### Configurable attributes + +Name | Default value | Configurable values +--- | --- | --- +AllowSubject | `false` | Boolean +Max | `5` | Integer + +### References + +* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MemoizedHelpersInExampleGroup](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MemoizedHelpersInExampleGroup) + ## RSpec/MessageChain Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged @@ -2322,77 +2438,6 @@ Max | `3` | Integer * [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NestedGroups](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NestedGroups) -## RSpec/MemoizedHelpersInExampleGroup - -Enabled by default | Supports autocorrection ---- | --- -Disabled | No - -Checks for usage of `let` blocks in specs. - -This cop can be configured with the option `AllowSubject` which -will configure the cop to only register offenses on explicit calls to -`let` and not calls to `subject` - -### Examples - -```ruby -# bad -describe MyClass do - let(:foo) { [] } - it { expect(foo).to be_empty } -end - -describe MyClass do - subject(:foo) { [] } - it { expect(foo).to be_empty } -end - -# good -describe MyClass do - it do - foo = [] - expect(foo).to be_empty - end -end -``` -#### with AllowSubject configuration - -```ruby -# rubocop.yml -# RSpec/MemoizedHelpersInExampleGroup: -# AllowSubject: true - -# bad -describe MyClass do - let(:foo) { [] } - it { expect(foo).to be_empty } -end - -# good -describe MyClass do - subject(:foo) { [] } - it { expect(foo).to be_empty } -end - -describe MyClass do - it do - foo = [] - expect(foo).to be_empty - end -end -``` - -### Configurable attributes - -Name | Default value | Configurable values ---- | --- | --- -AllowSubject | `false` | Boolean - -### References - -* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MemoizedHelpersInExampleGroup](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MemoizedHelpersInExampleGroup) - ## RSpec/NotToNot Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged diff --git a/spec/rubocop/cop/rspec/memoized_helpers_in_example_group_spec.rb b/spec/rubocop/cop/rspec/memoized_helpers_in_example_group_spec.rb index f09df812c..d71559b1e 100644 --- a/spec/rubocop/cop/rspec/memoized_helpers_in_example_group_spec.rb +++ b/spec/rubocop/cop/rspec/memoized_helpers_in_example_group_spec.rb @@ -1,63 +1,126 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::RSpec::MemoizedHelpersInExampleGroup do - subject(:cop) { described_class.new(config) } +RSpec.describe RuboCop::Cop::RSpec::MemoizedHelpersInExampleGroup, :config do + let(:cop_config) { { 'Max' => 1 } } - let(:config) { RuboCop::Config.new } - - # TODO: Write test code - # - # For example - it 'flags an offense when using `#let`' do + it 'flags an offense when using more than max `#let` calls' do expect_offense(<<~RUBY) - let(:foo) { Foo.new } - ^^^^^^^^^ Avoid using `let` – use a method call or local variable instead. + describe Foo do + ^^^^^^^^^^^^^^^ Example has too many memoized helpers [2/1] + let(:foo) { Foo.new } + let(:bar) { Bar.new } + end RUBY end it 'flags an offense when using `#subject` without name' do expect_offense(<<~RUBY) - subject { Foo.new } - ^^^^^^^ Avoid using `subject` – use a method call or local variable instead. + describe Foo do + ^^^^^^^^^^^^^^^ Example has too many memoized helpers [2/1] + subject { Foo.new } + let(:foo) { Foo.new } + end RUBY end it 'flags an offense when using `#subject` with name' do expect_offense(<<~RUBY) - subject(:foo) { Foo.new } - ^^^^^^^^^^^^^ Avoid using `subject` – use a method call or local variable instead. + describe Foo do + ^^^^^^^^^^^^^^^ Example has too many memoized helpers [2/1] + subject(:foo) { Foo.new } + let(:foo) { Foo.new } + end + RUBY + end + + it 'flags an offense when using `#let!`' do + expect_offense(<<~RUBY) + describe Foo do + ^^^^^^^^^^^^^^^ Example has too many memoized helpers [2/1] + let(:foo) { Foo.new } + let!(:foo) { Foo.new } + end + RUBY + end + + it 'flags an offense when using `#subject!`' do + expect_offense(<<~RUBY) + describe Foo do + ^^^^^^^^^^^^^^^ Example has too many memoized helpers [2/1] + subject!(:foo) { Foo.new } + let(:foo) { Foo.new } + end + RUBY + end + + it 'does not flag an offense when using <= max `#let` calls' do + expect_no_offenses(<<~RUBY) + describe Foo do + let(:foo) { Foo.new } + end + RUBY + end + + it 'flags an offense when too many `#let` calls are nested' do + expect_offense(<<~RUBY) + describe Foo do + let(:foo) { Foo.new } + + context 'when blah' do + ^^^^^^^^^^^^^^^^^^^^^^ Example has too many memoized helpers [2/1] + let(:bar) { Bar.new } + end + end + RUBY + end + + it 'does not flag an offense when `#let` calls are distributed' do + expect_no_offenses(<<~RUBY) + describe Foo do + context 'when bloo' do + let(:foo) { Foo.new } + end + + context 'when blah' do + let(:bar) { Bar.new } + end + end RUBY end it 'does not flag an offense when using `#before`' do expect_no_offenses(<<~RUBY) - before { foo } + describe Foo do + before { foo } + end RUBY end context 'when using AllowSubject configuration', :config do - subject(:cop) { described_class.new(config) } - - let(:cop_config) do - { 'AllowSubject' => true } - end + let(:cop_config) { { 'Max' => 0, 'AllowSubject' => true } } it 'flags an offense when using `#let`' do expect_offense(<<~RUBY) - let(:foo) { Foo.new } - ^^^^^^^^^ Avoid using `let` – use a method call or local variable instead. + describe Foo do + ^^^^^^^^^^^^^^^ Example has too many memoized helpers [1/0] + let(:foo) { Foo.new } + end RUBY end it 'does not flag an offense when using `#subject` without a name' do expect_no_offenses(<<~RUBY) - subject { Foo.new } + describe Foo do + subject { Foo.new } + end RUBY end it 'does not flag an offense when using `#subject` with a name' do expect_no_offenses(<<~RUBY) - subject(:foo) { Foo.new } + describe Foo do + subject(:foo) { Foo.new } + end RUBY end end