Merge pull request #1907 from rubocop/reduced-private-scope

Don't always define node patterns in private scope

lib/rubocop/cop/rspec/around_block.rb

@@ -31,6 +31,21 @@ class AroundBlock < Base
         MSG_UNUSED_ARG = 'You should call `%<arg>s.call` ' \
                          'or `%<arg>s.run`.'
 
+        # @!method hook_block(node)
+        def_node_matcher :hook_block, <<~PATTERN
+          (block (send nil? :around sym ?) (args $...) ...)
+        PATTERN
+
+        # @!method hook_numblock(node)
+        def_node_matcher :hook_numblock, <<~PATTERN
+          (numblock (send nil? :around sym ?) ...)
+        PATTERN
+
+        # @!method find_arg_usage(node)
+        def_node_search :find_arg_usage, <<~PATTERN
+          {(send $... {:call :run}) (send _ _ $...) (yield $...) (block-pass $...)}
+        PATTERN
+
         def on_block(node)
           hook_block(node) do |(example_proxy)|
             if example_proxy.nil?
@@ -49,21 +64,6 @@ def on_numblock(node)
 
         private
 
-        # @!method hook_block(node)
-        def_node_matcher :hook_block, <<~PATTERN
-          (block (send nil? :around sym ?) (args $...) ...)
-        PATTERN
-
-        # @!method hook_numblock(node)
-        def_node_matcher :hook_numblock, <<~PATTERN
-          (numblock (send nil? :around sym ?) ...)
-        PATTERN
-
-        # @!method find_arg_usage(node)
-        def_node_search :find_arg_usage, <<~PATTERN
-          {(send $... {:call :run}) (send _ _ $...) (yield $...) (block-pass $...)}
-        PATTERN
-
         def add_no_arg_offense(node)
           add_offense(node, message: MSG_NO_ARG)
         end

lib/rubocop/cop/rspec/be.rb

@@ -23,18 +23,16 @@ class Be < Base
 
         RESTRICT_ON_SEND = Runners.all
 
+        # @!method be_without_args(node)
+        def_node_matcher :be_without_args, <<~PATTERN
+          (send _ #Runners.all $(send nil? :be))
+        PATTERN
+
         def on_send(node)
           be_without_args(node) do |matcher|
             add_offense(matcher.loc.selector)
           end
         end
-
-        private
-
-        # @!method be_without_args(node)
-        def_node_matcher :be_without_args, <<~PATTERN
-          (send _ #Runners.all $(send nil? :be))
-        PATTERN
       end
     end
   end

lib/rubocop/cop/rspec/be_empty.rb

@@ -19,16 +19,6 @@ class BeEmpty < Base
         MSG = 'Use `be_empty` matchers for checking an empty array.'
         RESTRICT_ON_SEND = %i[contain_exactly match_array].freeze
 
-        def on_send(node)
-          expect_array_matcher?(node.parent) do |expect|
-            add_offense(expect) do |corrector|
-              corrector.replace(expect, 'be_empty')
-            end
-          end
-        end
-
-        private
-
         # @!method expect_array_matcher?(node)
         def_node_matcher :expect_array_matcher?, <<~PATTERN
           (send
@@ -41,6 +31,14 @@ def on_send(node)
             _?
           )
         PATTERN
+
+        def on_send(node)
+          expect_array_matcher?(node.parent) do |expect|
+            add_offense(expect) do |corrector|
+              corrector.replace(expect, 'be_empty')
+            end
+          end
+        end
       end
     end
   end

lib/rubocop/cop/rspec/be_eq.rb

@@ -29,20 +29,18 @@ class BeEq < Base
         MSG = 'Prefer `be` over `eq`.'
         RESTRICT_ON_SEND = %i[eq].freeze
 
+        # @!method eq_type_with_identity?(node)
+        def_node_matcher :eq_type_with_identity?, <<~PATTERN
+          (send nil? :eq {true false nil})
+        PATTERN
+
         def on_send(node)
           return unless eq_type_with_identity?(node)
 
           add_offense(node.loc.selector) do |corrector|
             corrector.replace(node.loc.selector, 'be')
           end
         end
-
-        private
-
-        # @!method eq_type_with_identity?(node)
-        def_node_matcher :eq_type_with_identity?, <<~PATTERN
-          (send nil? :eq {true false nil})
-        PATTERN
       end
     end
   end

lib/rubocop/cop/rspec/be_eql.rb

@@ -43,20 +43,18 @@ class BeEql < Base
         MSG = 'Prefer `be` over `eql`.'
         RESTRICT_ON_SEND = %i[to].freeze
 
+        # @!method eql_type_with_identity(node)
+        def_node_matcher :eql_type_with_identity, <<~PATTERN
+          (send _ :to $(send nil? :eql {true false int float sym nil}))
+        PATTERN
+
         def on_send(node)
           eql_type_with_identity(node) do |eql|
             add_offense(eql.loc.selector) do |corrector|
               corrector.replace(eql.loc.selector, 'be')
             end
           end
         end
-
-        private
-
-        # @!method eql_type_with_identity(node)
-        def_node_matcher :eql_type_with_identity, <<~PATTERN
-          (send _ :to $(send nil? :eql {true false int float sym nil}))
-        PATTERN
       end
     end
   end

lib/rubocop/cop/rspec/be_nil.rb

@@ -32,6 +32,16 @@ class BeNil < Base
         BE_NIL_MSG = 'Prefer `be_nil` over `be(nil)`.'
         RESTRICT_ON_SEND = %i[be be_nil].freeze
 
+        # @!method be_nil_matcher?(node)
+        def_node_matcher :be_nil_matcher?, <<~PATTERN
+          (send nil? :be_nil)
+        PATTERN
+
+        # @!method nil_value_expectation?(node)
+        def_node_matcher :nil_value_expectation?, <<~PATTERN
+          (send nil? :be nil)
+        PATTERN
+
         def on_send(node)
           case style
           when :be
@@ -43,16 +53,6 @@ def on_send(node)
 
         private
 
-        # @!method be_nil_matcher?(node)
-        def_node_matcher :be_nil_matcher?, <<~PATTERN
-          (send nil? :be_nil)
-        PATTERN
-
-        # @!method nil_value_expectation?(node)
-        def_node_matcher :nil_value_expectation?, <<~PATTERN
-          (send nil? :be nil)
-        PATTERN
-
         def check_be_style(node)
           return unless be_nil_matcher?(node)
 

lib/rubocop/cop/rspec/before_after_all.rb

@@ -26,6 +26,11 @@ class BeforeAfterAll < Base
 
         RESTRICT_ON_SEND = Set[:before, :after].freeze
 
+        # @!method before_or_after_all(node)
+        def_node_matcher :before_or_after_all, <<~PATTERN
+          $(send _ RESTRICT_ON_SEND (sym {:all :context}))
+        PATTERN
+
         def on_send(node)
           before_or_after_all(node) do |hook|
             add_offense(
@@ -34,13 +39,6 @@ def on_send(node)
             )
           end
         end
-
-        private
-
-        # @!method before_or_after_all(node)
-        def_node_matcher :before_or_after_all, <<~PATTERN
-          $(send _ RESTRICT_ON_SEND (sym {:all :context}))
-        PATTERN
       end
     end
   end

lib/rubocop/cop/rspec/change_by_zero.rb

@@ -67,18 +67,6 @@ class ChangeByZero < Base
         CHANGE_METHODS = Set[:change, :a_block_changing, :changing].freeze
         RESTRICT_ON_SEND = CHANGE_METHODS.freeze
 
-        def on_send(node)
-          expect_change_with_arguments(node.parent) do |change|
-            register_offense(node.parent, change)
-          end
-
-          expect_change_with_block(node.parent.parent) do |change|
-            register_offense(node.parent.parent, change)
-          end
-        end
-
-        private
-
         # @!method expect_change_with_arguments(node)
         def_node_matcher :expect_change_with_arguments, <<~PATTERN
           (send
@@ -101,6 +89,18 @@ def on_send(node)
           $(send nil? CHANGE_METHODS ...)
         PATTERN
 
+        def on_send(node)
+          expect_change_with_arguments(node.parent) do |change|
+            register_offense(node.parent, change)
+          end
+
+          expect_change_with_block(node.parent.parent) do |change|
+            register_offense(node.parent.parent, change)
+          end
+        end
+
+        private
+
         # rubocop:disable Metrics/MethodLength
         def register_offense(node, change_node)
           if compound_expectations?(node)

lib/rubocop/cop/rspec/context_method.rb

@@ -29,6 +29,15 @@ class ContextMethod < Base
 
         MSG = 'Use `describe` for testing methods.'
 
+        # @!method context_method(node)
+        def_node_matcher :context_method, <<~PATTERN
+          (block
+            (send #rspec? :context
+              ${(str #method_name?) (dstr (str #method_name?) ...)}
+            ...)
+          ...)
+        PATTERN
+
         def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
           context_method(node) do |context|
             add_offense(context) do |corrector|
@@ -39,15 +48,6 @@ def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
 
         private
 
-        # @!method context_method(node)
-        def_node_matcher :context_method, <<~PATTERN
-          (block
-            (send #rspec? :context
-              ${(str #method_name?) (dstr (str #method_name?) ...)}
-            ...)
-          ...)
-        PATTERN
-
         def method_name?(description)
           description.start_with?('.', '#')
         end

lib/rubocop/cop/rspec/context_wording.rb

@@ -60,6 +60,11 @@ class ContextWording < Base
 
         MSG = 'Context description should match %<patterns>s.'
 
+        # @!method context_wording(node)
+        def_node_matcher :context_wording, <<~PATTERN
+          (block (send #rspec? { :context :shared_context } $({str dstr xstr} ...) ...) ...)
+        PATTERN
+
         def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
           context_wording(node) do |context|
             if bad_pattern?(context)
@@ -71,11 +76,6 @@ def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
 
         private
 
-        # @!method context_wording(node)
-        def_node_matcher :context_wording, <<~PATTERN
-          (block (send #rspec? { :context :shared_context } $({str dstr xstr} ...) ...) ...)
-        PATTERN
-
         def allowed_patterns
           super + prefix_regexes
         end

lib/rubocop/cop/rspec/describe_class.rb

@@ -40,16 +40,6 @@ class DescribeClass < Base
         MSG = 'The first argument to describe should be ' \
               'the class or module being tested.'
 
-        def on_top_level_group(node)
-          return if example_group_with_ignored_metadata?(node.send_node)
-
-          not_a_const_described(node.send_node) do |described|
-            add_offense(described)
-          end
-        end
-
-        private
-
         # @!method example_group_with_ignored_metadata?(node)
         def_node_matcher :example_group_with_ignored_metadata?, <<~PATTERN
           (send #rspec? :describe ... (hash <#ignored_metadata? ...>))
@@ -65,6 +55,16 @@ def on_top_level_group(node)
           (pair $sym $sym)
         PATTERN
 
+        def on_top_level_group(node)
+          return if example_group_with_ignored_metadata?(node.send_node)
+
+          not_a_const_described(node.send_node) do |described|
+            add_offense(described)
+          end
+        end
+
+        private
+
         def ignored_metadata?(node)
           sym_pair(node) do |key, value|
             ignored_metadata[key.value.to_s].to_a.include?(value.value.to_s)

lib/rubocop/cop/rspec/describe_method.rb

@@ -23,14 +23,6 @@ class DescribeMethod < Base
         MSG = 'The second argument to describe should be the method ' \
               "being tested. '#instance' or '.class'."
 
-        def on_top_level_group(node)
-          second_string_literal_argument(node) do |argument|
-            add_offense(argument) unless method_name?(argument)
-          end
-        end
-
-        private
-
         # @!method second_string_literal_argument(node)
         def_node_matcher :second_string_literal_argument, <<~PATTERN
           (block
@@ -43,6 +35,14 @@ def on_top_level_group(node)
           {(str #method_name_prefix?) (dstr (str #method_name_prefix?) ...)}
         PATTERN
 
+        def on_top_level_group(node)
+          second_string_literal_argument(node) do |argument|
+            add_offense(argument) unless method_name?(argument)
+          end
+        end
+
+        private
+
         def method_name_prefix?(description)
           description.start_with?('.', '#')
         end

lib/rubocop/cop/rspec/describe_symbol.rb

@@ -21,18 +21,16 @@ class DescribeSymbol < Base
         MSG = 'Avoid describing symbols.'
         RESTRICT_ON_SEND = %i[describe].freeze
 
+        # @!method describe_symbol?(node)
+        def_node_matcher :describe_symbol?, <<~PATTERN
+          (send #rspec? :describe $sym ...)
+        PATTERN
+
         def on_send(node)
           describe_symbol?(node) do |match|
             add_offense(match)
           end
         end
-
-        private
-
-        # @!method describe_symbol?(node)
-        def_node_matcher :describe_symbol?, <<~PATTERN
-          (send #rspec? :describe $sym ...)
-        PATTERN
       end
     end
   end