Merge pull request #1825 from rubocop/only-static-constants

bquorning · web-flow · commit f2593cd186fd · 2024-03-03T20:32:42.000+01:00
Add configuration option `OnlyStaticConstants` to `RSpec/DescribedClass`
diff --git 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
@@ -283,9 +283,10 @@ RSpec/DescribedClass:
   SupportedStyles:
     - described_class
     - explicit
+  OnlyStaticConstants: true
   SafeAutoCorrect: false
   VersionAdded: '1.0'
-  VersionChanged: '1.11'
+  VersionChanged: "<<next>>"
   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
@@ -887,16 +887,18 @@ end
 | Yes
 | Always (Unsafe)
 | 1.0
-| 1.11
+| <<next>>
 |===
 
 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
@@ -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
@@ -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
