Refactor DescribeClass to use TopLevelGroup

rubocop · Aug 2, 2020 · c0857af · c0857af
1 parent 3b461a2
commit c0857af
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 * Add a new base cop class `::RuboCop::Cop::RSpec::Base`. The old base class `::RuboCop::Cop::RSpec::Cop` is deprecated, and will be removed in the next major release. ([@bquorning][])
 * Add support for subject detection after includes and example groups in `RSpec/LeadingSubject`. ([@pirj][])
 * Ignore trailing punctuation in context description prefix. ([@elliterate][])
+* Improve `RSpec/NestedGroups`, `RSpec/FilePath`, `RSpec/DescribeMethod`, `RSpec/MultipleDescribes`, `RSpec/DescribeClass`'s top-level example group detection. ([@pirj][])
 
 ## 1.42.0 (2020-07-09)
 

diff --git a/config/default.yml b/config/default.yml
@@ -74,7 +74,7 @@ RSpec/ContextWording:
   StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ContextWording
 
 RSpec/DescribeClass:
-  Description: Check that the first argument to the top level describe is a constant.
+  Description: Check that the first argument to the top-level describe is a constant.
   Enabled: true
   VersionAdded: '1.0'
   StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DescribeClass

diff --git a/lib/rubocop/cop/rspec/describe_class.rb b/lib/rubocop/cop/rspec/describe_class.rb
@@ -3,7 +3,7 @@
 module RuboCop
   module Cop
     module RSpec
-      # Check that the first argument to the top level describe is a constant.
+      # Check that the first argument to the top-level describe is a constant.
       #
       # @example
       #   # bad
@@ -22,49 +22,40 @@ module RSpec
       #   describe "A feature example", type: :feature do
       #   end
       class DescribeClass < Base
-        include RuboCop::RSpec::TopLevelDescribe
+        include RuboCop::RSpec::TopLevelGroup
 
         MSG = 'The first argument to describe should be '\
               'the class or module being tested.'
 
-        def_node_matcher :valid_describe?, <<-PATTERN
-          {
-            (send #rspec? :describe const ...)
-            (send #rspec? :describe)
-          }
-        PATTERN
-
-        def_node_matcher :describe_with_rails_metadata?, <<-PATTERN
-          (send #rspec? :describe !const ...
-            (hash <#rails_metadata? ...>)
-          )
-        PATTERN
-
         def_node_matcher :rails_metadata?, <<-PATTERN
           (pair
             (sym :type)
-            (sym {
-                   :channel :controller :helper :job :mailer :model :request
-                   :routing :view :feature :system :mailbox
-                 }
-            )
+            (sym { :channel :controller :helper :job :mailer :model :request
+                   :routing :view :feature :system :mailbox })
           )
         PATTERN
 
-        def on_top_level_describe(node, (described_value, _))
-          return if shared_group?(root_node)
-          return if valid_describe?(node)
-          return if describe_with_rails_metadata?(node)
-          return if string_constant_describe?(described_value)
+        def_node_matcher :example_group_with_rails_metadata?, <<~PATTERN
+          (send #rspec? :describe ... (hash <#rails_metadata? ...>))
+        PATTERN
+
+        def_node_matcher :not_a_const_described, <<~PATTERN
+          (send #rspec? :describe $[!const !#string_constant?] ...)
+        PATTERN
+
+        def on_top_level_group(top_level_node)
+          return if example_group_with_rails_metadata?(top_level_node.send_node)
 
-          add_offense(described_value)
+          not_a_const_described(top_level_node.send_node) do |described|
+            add_offense(described)
+          end
         end
 
         private
 
-        def string_constant_describe?(described_value)
-          described_value.str_type? &&
-            described_value.value.match?(/^(?:(?:::)?[A-Z]\w*)+$/)
+        def string_constant?(described)
+          described.str_type? &&
+            described.value.match?(/^(?:(?:::)?[A-Z]\w*)+$/)
         end
       end
     end

diff --git a/manual/cops_rspec.md b/manual/cops_rspec.md
@@ -322,7 +322,7 @@ Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChan
 --- | --- | --- | --- | ---
 Enabled | Yes | No | 1.0 | -
 
-Check that the first argument to the top level describe is a constant.
+Check that the first argument to the top-level describe is a constant.
 
 ### Examples