From 350b50f5bf2bf43acf7c6478b0333d63f28a57be Mon Sep 17 00:00:00 2001 From: ydah Date: Sat, 2 Mar 2024 03:14:30 +0900 Subject: [PATCH] Add configuration option `OnlyStaticConstants` to `RSpec/DescribedClass` Fix: https://github.com/rubocop/rubocop-rspec/issues/1741#issuecomment-1973221319 Fix: https://github.com/rubocop/rubocop-rspec/issues/1741#issuecomment-1973604382 --- CHANGELOG.md | 1 + config/default.yml | 3 +- docs/modules/ROOT/pages/cops_rspec.adoc | 32 ++++++- lib/rubocop/cop/rspec/described_class.rb | 31 +++++-- .../rubocop/cop/rspec/described_class_spec.rb | 88 ++++++++++++------- 5 files changed, 116 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cda77667..dc83dec2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Master (Unreleased) - Fix a false positive for `RSpec/RepeatedSubjectCall` when `subject.method_call`. ([@ydah]) +- Add configuration option `OnlyStaticConstants` to `RSpec/DescribedClass`. ([@ydah]) ## 2.27.0 (2024-03-01) diff --git a/config/default.yml b/config/default.yml index 5a1905e6e..20ed04996 100644 --- a/config/default.yml +++ b/config/default.yml @@ -283,9 +283,10 @@ RSpec/DescribedClass: SupportedStyles: - described_class - explicit + OnlyStaticConstants: true SafeAutoCorrect: false VersionAdded: '1.0' - VersionChanged: '1.11' + VersionChanged: "<>" Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DescribedClass RSpec/DescribedClassModuleWrapping: diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index c2fb78a3e..8c1069cf0 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -887,7 +887,7 @@ end | Yes | Always (Unsafe) | 1.0 -| 1.11 +| <> |=== Checks that tests use `described_class`. @@ -895,8 +895,10 @@ Checks that tests use `described_class`. If the first argument of describe is a class, the class is exposed to each example via described_class. -This cop can be configured using the `EnforcedStyle` and `SkipBlocks` -options. +This cop can be configured using the `EnforcedStyle`, `SkipBlocks` +and `OnlyStaticConstants` options. +`OnlyStaticConstants` is only relevant when `EnforcedStyle` is +`described_class`. There's a known caveat with rspec-rails's `controller` helper that runs its block in a different context, and `described_class` is not @@ -924,6 +926,26 @@ describe MyClass do end ---- +==== `OnlyStaticConstants: true` (default) + +[source,ruby] +---- +# good +describe MyClass do + subject { MyClass::CONSTANT } +end +---- + +==== `OnlyStaticConstants: false` + +[source,ruby] +---- +# bad +describe MyClass do + subject { MyClass::CONSTANT } +end +---- + ==== `EnforcedStyle: explicit` [source,ruby] @@ -967,6 +989,10 @@ end | EnforcedStyle | `described_class` | `described_class`, `explicit` + +| OnlyStaticConstants +| `true` +| Boolean |=== === References diff --git a/lib/rubocop/cop/rspec/described_class.rb b/lib/rubocop/cop/rspec/described_class.rb index 3506b5728..389e675c5 100644 --- a/lib/rubocop/cop/rspec/described_class.rb +++ b/lib/rubocop/cop/rspec/described_class.rb @@ -8,8 +8,10 @@ module RSpec # If the first argument of describe is a class, the class is exposed to # each example via described_class. # - # This cop can be configured using the `EnforcedStyle` and `SkipBlocks` - # options. + # This cop can be configured using the `EnforcedStyle`, `SkipBlocks` + # and `OnlyStaticConstants` options. + # `OnlyStaticConstants` is only relevant when `EnforcedStyle` is + # `described_class`. # # @example `EnforcedStyle: described_class` (default) # # bad @@ -22,6 +24,18 @@ module RSpec # subject { described_class.do_something } # end # + # @example `OnlyStaticConstants: true` (default) + # # good + # describe MyClass do + # subject { MyClass::CONSTANT } + # end + # + # @example `OnlyStaticConstants: false` + # # bad + # describe MyClass do + # subject { MyClass::CONSTANT } + # end + # # @example `EnforcedStyle: explicit` # # bad # describe MyClass do @@ -54,7 +68,7 @@ module RSpec # end # end # - class DescribedClass < Base + class DescribedClass < Base # rubocop:disable Metrics/ClassLength extend AutoCorrector include ConfigurableEnforcedStyle include Namespace @@ -112,14 +126,17 @@ def autocorrect(corrector, match) def find_usage(node, &block) yield(node) if offensive?(node) - - return if scope_change?(node) + return if scope_change?(node) || allowed?(node) node.each_child_node do |child| find_usage(child, &block) end end + def allowed?(node) + node.const_type? && only_static_constants? + end + def message(offense) if style == :described_class format(MSG, replacement: DESCRIBED_CLASS, src: offense) @@ -139,6 +156,10 @@ def skippable_block?(node) node.block_type? && !rspec_block?(node) && cop_config['SkipBlocks'] end + def only_static_constants? + cop_config.fetch('OnlyStaticConstants', true) + end + def offensive?(node) if style == :described_class offensive_described_class?(node) diff --git a/spec/rubocop/cop/rspec/described_class_spec.rb b/spec/rubocop/cop/rspec/described_class_spec.rb index aa689e3ae..6dba8b763 100644 --- a/spec/rubocop/cop/rspec/described_class_spec.rb +++ b/spec/rubocop/cop/rspec/described_class_spec.rb @@ -148,36 +148,6 @@ module MyModule RUBY end - it 'flags class with constant' do - expect_offense(<<~RUBY) - describe MyClass do - subject { MyClass::FOO } - ^^^^^^^ Use `described_class` instead of `MyClass`. - end - RUBY - - expect_correction(<<~RUBY) - describe MyClass do - subject { described_class::FOO } - end - RUBY - end - - it 'flags class with subclasses' do - expect_offense(<<~RUBY) - describe MyClass do - subject { MyClass::Subclass } - ^^^^^^^ Use `described_class` instead of `MyClass`. - end - RUBY - - expect_correction(<<~RUBY) - describe MyClass do - subject { described_class::Subclass } - end - RUBY - end - it 'ignores non-matching namespace defined on `describe` level' do expect_no_offenses(<<~RUBY) describe MyNamespace::MyClass do @@ -310,6 +280,64 @@ module SomeGem end RUBY end + + context 'when OnlyStaticConstants is `true`' do + let(:cop_config) do + { 'EnforcedStyle' => :described_class, 'OnlyStaticConstants' => true } + end + + it 'ignores class with constant' do + expect_no_offenses(<<~RUBY) + describe MyClass do + subject { MyClass::FOO } + end + RUBY + end + + it 'ignores class with subclasses' do + expect_no_offenses(<<~RUBY) + describe MyClass do + subject { MyClass::Subclass } + end + RUBY + end + end + + context 'when OnlyStaticConstants is `false`' do + let(:cop_config) do + { 'EnforcedStyle' => :described_class, 'OnlyStaticConstants' => false } + end + + it 'flags class with constant' do + expect_offense(<<~RUBY) + describe MyClass do + subject { MyClass::FOO } + ^^^^^^^ Use `described_class` instead of `MyClass`. + end + RUBY + + expect_correction(<<~RUBY) + describe MyClass do + subject { described_class::FOO } + end + RUBY + end + + it 'flags class with subclasses' do + expect_offense(<<~RUBY) + describe MyClass do + subject { MyClass::Subclass } + ^^^^^^^ Use `described_class` instead of `MyClass`. + end + RUBY + + expect_correction(<<~RUBY) + describe MyClass do + subject { described_class::Subclass } + end + RUBY + end + end end context 'when EnforcedStyle is :explicit' do