From 5d51adeafb9cb63c0164a6c938b4fe064400132a Mon Sep 17 00:00:00 2001 From: Robert Fletcher Date: Sat, 13 Jun 2020 09:59:08 -0700 Subject: [PATCH] updates --- config/default.yml | 13 +- .../memoized_helpers_in_example_group.rb | 34 +++-- lib/rubocop/cop/rspec_cops.rb | 2 +- lib/rubocop/rspec/example_group.rb | 20 +++ manual/cops.md | 2 +- manual/cops_rspec.md | 144 +++++++++--------- .../memoized_helpers_in_example_group_spec.rb | 111 +++++++++++--- 7 files changed, 213 insertions(+), 113 deletions(-) diff --git a/config/default.yml b/config/default.yml index 30f06c904..d9f5fa8f6 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 for over-usage of `let` and `subject` blocks in example groups. + 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..af8f784ca 100644 --- a/lib/rubocop/cop/rspec/memoized_helpers_in_example_group.rb +++ b/lib/rubocop/cop/rspec/memoized_helpers_in_example_group.rb @@ -3,8 +3,9 @@ module RuboCop module Cop module RSpec - # Checks for usage of `let` blocks in specs. + # Checks for over-usage of `let` and `subject` blocks in example groups. # + # The maximum count can be configured. # 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` @@ -55,22 +56,35 @@ module RSpec # end # class MemoizedHelpersInExampleGroup < Cop - MSG = 'Avoid using `%s` ' \ - '– use a method call or local variable instead.' + MSG = 'Too many memoized helpers used. [%d/%d]' - def_node_matcher :let?, Helpers::ALL.send_pattern - def_node_matcher :subject?, Subject::ALL.send_pattern + def on_block(node) + return unless example_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/lib/rubocop/rspec/example_group.rb b/lib/rubocop/rspec/example_group.rb index 8a88ed3f0..e6b1e4fb2 100644 --- a/lib/rubocop/rspec/example_group.rb +++ b/lib/rubocop/rspec/example_group.rb @@ -14,6 +14,10 @@ class ExampleGroup < Concept ExampleGroups::ALL + SharedGroups::ALL + Includes::ALL ).block_pattern + def lets + lets_in_scope(node) + end + def subjects subjects_in_scope(node) end @@ -34,6 +38,12 @@ def subjects_in_scope(node) end end + def lets_in_scope(node) + node.each_child_node.flat_map do |child| + find_lets(child) + end + end + def find_subjects(node) return [] if scope_change?(node) @@ -44,6 +54,16 @@ def find_subjects(node) end end + def find_lets(node) + return [] if scope_change?(node) + + if let?(node) + [node] + else + lets_in_scope(node) + end + end + def hooks_in_scope(node) node.each_child_node.flat_map do |child| find_hooks(child) 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 88a801135..f5cd2f423 100644 --- a/manual/cops_rspec.md +++ b/manual/cops_rspec.md @@ -1890,6 +1890,79 @@ 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 for over-usage of `let` and `subject` blocks in example groups. + +The maximum count can be configured. +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 +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 +2395,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..c7864d9eb 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 + ^^^^^^^^^^^^^^^ Too many memoized helpers used. [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 + ^^^^^^^^^^^^^^^ Too many memoized helpers used. [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 + ^^^^^^^^^^^^^^^ Too many memoized helpers used. [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 + ^^^^^^^^^^^^^^^ Too many memoized helpers used. [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 + ^^^^^^^^^^^^^^^ Too many memoized helpers used. [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 + ^^^^^^^^^^^^^^^^^^^^^^ Too many memoized helpers used. [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 + ^^^^^^^^^^^^^^^ Too many memoized helpers used. [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