From 1a3d3fedd7b0f73b8d9478945e5d7f6415fa32f8 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Fri, 7 Aug 2020 02:50:19 +0300 Subject: [PATCH] Enable MultipleMemoizedHelpers by default Switching the cop on by default to match RSpec recommendations from its docs and the community RSpec Style Guide recommendations. `let` _can_ enhance readability when used sparingly (1,2, or maybe 3 declarations) in any given example group, but that can quickly degrade with overuse. YMMV. https://github.com/rspec/rspec-core/blob/5d87c28bb828ba14b54a0d8cfddf044894009314/lib/rspec/core/memoized_helpers.rb#L257 https://reddit.com/r/ruby/comments/73nrhk/ama_the_authors_of_effective_testing_with_rspec_3/dnsyanp/ https://rspec.rubystyle.guide/#let-blocks Example change to dramatically reduce the number of used memoized helpers without resorting to methods https://github.com/rubocop-hq/rubocop/pull/8447 There is no limit to the number of memoized helpers being used in practice: 10 lets https://github.com/forem/forem/blob/master/spec/support/api_analytics_shared_examples.rb#L2 20 lets https://github.com/chef/chef/blob/master/spec/unit/provider/systemd_unit_spec.rb#L23 30 lets https://github.com/chef/chef/blob/master/spec/unit/policy_builder/policyfile_spec.rb#L24 40 lets https://github.com/diaspora/diaspora/blob/develop/spec/integration/migration_service_spec.rb#L7 50 lets https://github.com/chef/chef/blob/master/spec/functional/resource/dsc_script_spec.rb#L71 Allowing `subject` by default, i.e. subjects don't count towards the number of memoized helpers, since there is a dedicated `RSpec/MultipleSubjects` cop. Combination with it provides infinite possibilities to tweak the maximum allowed number of memoized helpers to meet the project style. --- config/default.yml | 4 +- .../cop/rspec/multiple_memoized_helpers.rb | 20 ++-- manual/cops_rspec.md | 24 ++-- .../rspec/multiple_memoized_helpers_spec.rb | 104 +++++++++--------- 4 files changed, 69 insertions(+), 83 deletions(-) diff --git a/config/default.yml b/config/default.yml index 61f67c9b7..946f854f8 100644 --- a/config/default.yml +++ b/config/default.yml @@ -396,8 +396,8 @@ RSpec/MultipleExpectations: RSpec/MultipleMemoizedHelpers: Description: Checks if example groups contain too many `let` and `subject` calls. - Enabled: false - AllowSubject: false + Enabled: true + AllowSubject: true Max: 5 StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleMemoizedHelpers diff --git a/lib/rubocop/cop/rspec/multiple_memoized_helpers.rb b/lib/rubocop/cop/rspec/multiple_memoized_helpers.rb index a7295f955..fb649053b 100644 --- a/lib/rubocop/cop/rspec/multiple_memoized_helpers.rb +++ b/lib/rubocop/cop/rspec/multiple_memoized_helpers.rb @@ -17,7 +17,7 @@ module RSpec # let!(:baz) { [] } # let(:qux) { [] } # let(:quux) { [] } - # subject(:quuz) { {} } + # let(:quuz) { {} } # end # # describe MyClass do @@ -28,7 +28,7 @@ module RSpec # context 'when stuff' do # let(:qux) { [] } # let(:quux) { [] } - # subject(:quuz) { {} } + # let(:quuz) { {} } # end # end # @@ -38,7 +38,7 @@ module RSpec # let!(:baz) { [] } # let(:qux) { [] } # let(:quux) { [] } - # subject(:quuz) { {} } + # let(:quuz) { {} } # end # # describe MyClass do @@ -51,7 +51,7 @@ module RSpec # context 'when other stuff' do # let(:qux) { [] } # let(:quux) { [] } - # subject(:quuz) { {} } + # let(:quuz) { {} } # end # end # @@ -59,9 +59,9 @@ module RSpec # # # rubocop.yml # # RSpec/MultipleMemoizedHelpers: - # # AllowSubject: true + # # AllowSubject: false # - # # good + # # bad - `subject` counts towards memoized helpers # describe MyClass do # let(:foo) { [] } # let(:bar) { [] } @@ -75,16 +75,12 @@ module RSpec # # # rubocop.yml # # RSpec/MultipleMemoizedHelpers: - # # Max: 0 + # # Max: 1 # # # bad # describe MyClass do # let(:foo) { [] } - # end - # - # # good - # describe MyClass do - # def foo; []; end + # let(:bar) { [] } # end # class MultipleMemoizedHelpers < Base diff --git a/manual/cops_rspec.md b/manual/cops_rspec.md index 61780d141..bc18cfc3d 100644 --- a/manual/cops_rspec.md +++ b/manual/cops_rspec.md @@ -2123,7 +2123,7 @@ Max | `1` | Integer Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged --- | --- | --- | --- | --- -Disabled | Yes | No | - | - +Enabled | Yes | No | - | - Checks if example groups contain too many `let` and `subject` calls. @@ -2141,7 +2141,7 @@ describe MyClass do let!(:baz) { [] } let(:qux) { [] } let(:quux) { [] } - subject(:quuz) { {} } + let(:quuz) { {} } end describe MyClass do @@ -2152,7 +2152,7 @@ describe MyClass do context 'when stuff' do let(:qux) { [] } let(:quux) { [] } - subject(:quuz) { {} } + let(:quuz) { {} } end end @@ -2162,7 +2162,7 @@ describe MyClass do let!(:baz) { [] } let(:qux) { [] } let(:quux) { [] } - subject(:quuz) { {} } + let(:quuz) { {} } end describe MyClass do @@ -2175,7 +2175,7 @@ describe MyClass do context 'when other stuff' do let(:qux) { [] } let(:quux) { [] } - subject(:quuz) { {} } + let(:quuz) { {} } end end ``` @@ -2184,9 +2184,9 @@ end ```ruby # rubocop.yml # RSpec/MultipleMemoizedHelpers: -# AllowSubject: true +# AllowSubject: false -# good +# bad - `subject` counts towards memoized helpers describe MyClass do let(:foo) { [] } let(:bar) { [] } @@ -2201,16 +2201,12 @@ end ```ruby # rubocop.yml # RSpec/MultipleMemoizedHelpers: -# Max: 0 +# Max: 1 # bad describe MyClass do let(:foo) { [] } -end - -# good -describe MyClass do - def foo; []; end + let(:bar) { [] } end ``` @@ -2218,7 +2214,7 @@ end Name | Default value | Configurable values --- | --- | --- -AllowSubject | `false` | Boolean +AllowSubject | `true` | Boolean Max | `5` | Integer ### References diff --git a/spec/rubocop/cop/rspec/multiple_memoized_helpers_spec.rb b/spec/rubocop/cop/rspec/multiple_memoized_helpers_spec.rb index 24b5424e6..771e03493 100644 --- a/spec/rubocop/cop/rspec/multiple_memoized_helpers_spec.rb +++ b/spec/rubocop/cop/rspec/multiple_memoized_helpers_spec.rb @@ -3,7 +3,7 @@ RSpec.describe RuboCop::Cop::RSpec::MultipleMemoizedHelpers, :config do let(:cop_config) { { 'Max' => 1 } } - it 'flags an offense when using more than max `#let` calls' do + it 'flags excessive `#let`' do expect_offense(<<~RUBY) describe Foo do ^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1] @@ -13,83 +13,66 @@ RUBY end - it 'flags an offense when using `#subject` without name' do + it 'flags excessive `#let!`' do expect_offense(<<~RUBY) describe Foo do ^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1] - subject { Foo.new } let(:foo) { Foo.new } + let!(:bar) { Bar.new } end RUBY end - it 'flags an offense when using `#subject` with name' do - expect_offense(<<~RUBY) - describe Foo do - ^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1] - subject(:foo) { Foo.new } - let(:bar) { Foo.new } - end - RUBY - end - - it 'flags an offense when using `#let!`' do - expect_offense(<<~RUBY) + it 'ignores a reasonable number of memoized helpers' do + expect_no_offenses(<<~RUBY) describe Foo do - ^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1] let(:foo) { Foo.new } - let!(:bar) { Foo.new } end RUBY end - it 'flags an offense when using `#subject!`' do - expect_offense(<<~RUBY) + it 'ignores overridden `#let`' do + expect_no_offenses(<<~RUBY) describe Foo do - ^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1] - subject!(:foo) { Foo.new } - let(:bar) { Foo.new } + let(:foo) { Foo.new } + context do + let(:foo) { Foo.new(1) } + context do + let(:foo) { Foo.new(1, 2) } + end + end end RUBY end - it 'does not flag an offense when using <= max `#let` calls' do + it 'ignores `#subject`' do expect_no_offenses(<<~RUBY) describe Foo do + subject(:bar) { Bar.new } let(:foo) { Foo.new } end RUBY end - it 'ignores overridden variables' do + it 'ignores `#subject!`' do expect_no_offenses(<<~RUBY) describe Foo do + subject!(:bar) { Bar.new } let(:foo) { Foo.new } - context do - let(:foo) { Foo.new(1) } - context do - let(:foo) { Foo.new(1, 2) } - end - end end RUBY end - it 'ignores overridden nameless subjects' do + it 'ignores `#subject` without a name' do expect_no_offenses(<<~RUBY) describe Foo do subject { Foo.new } - context do - subject { Foo.new(1) } - context do - subject { Foo.new(1, 2) } - end - end + let(:bar) { Bar.new } end RUBY end - it 'flags an offense when too many `#let` calls are nested' do + it 'flags nested `#let`' do expect_offense(<<~RUBY) describe Foo do let(:foo) { Foo.new } @@ -102,7 +85,7 @@ RUBY end - it 'does not flag an offense when `#let` calls are distributed' do + it 'ignores distributed `#let`' do expect_no_offenses(<<~RUBY) describe Foo do context 'when bloo' do @@ -116,38 +99,49 @@ RUBY end - it 'does not flag an offense when using `#before`' do - expect_no_offenses(<<~RUBY) - describe Foo do - before { foo } - end - RUBY - end - context 'when using AllowSubject configuration', :config do - let(:cop_config) { { 'Max' => 0, 'AllowSubject' => true } } + let(:cop_config) { { 'Max' => 1, 'AllowSubject' => false } } - it 'flags an offense when using `#let`' do + it 'flags `#subject` without name' do expect_offense(<<~RUBY) describe Foo do - ^^^^^^^^^^^^^^^ Example group has too many memoized helpers [1/0] + ^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1] + subject { Foo.new } let(:foo) { Foo.new } end RUBY end - it 'does not flag an offense when using `#subject` without a name' do - expect_no_offenses(<<~RUBY) + it 'flags `#subject`' do + expect_offense(<<~RUBY) describe Foo do - subject { Foo.new } + ^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1] + subject(:foo) { Foo.new } + let(:bar) { Foo.new } end RUBY end - it 'does not flag an offense when using `#subject` with a name' do + it 'flags `#subject!`' do + expect_offense(<<~RUBY) + describe Foo do + ^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1] + subject!(:foo) { Foo.new } + let(:bar) { Foo.new } + end + RUBY + end + + it 'ignores overridden subjects' do expect_no_offenses(<<~RUBY) describe Foo do - subject(:foo) { Foo.new } + subject { Foo.new } + context do + subject { Foo.new(1) } + context do + subject { Foo.new(1, 2) } + end + end end RUBY end