Refactor on_existing_matching_groups.

- Use a functional style using recursion instead of
  mutating a `groups` array imperatively. I find a
  functional style easier to reason about.
- Split off the hook-specific logic, but preserve
  the general traversal logic.
- Remove duplication of `Metadata.build_hash_from`.
- Only call `Metadata.build_hash_from` if needed. If
  we are dealing with an :example hook there is no need.

Author: myronmarston

lib/rspec/core/configuration.rb

@@ -1749,8 +1749,7 @@ def before(scope=nil, *meta, &block)
         handle_suite_hook(scope, meta) do
           @before_suite_hooks << Hooks::BeforeHook.new(block, {})
         end || begin
-          metadata = Metadata.build_hash_from(meta.dup)
-          on_existing_matching_groups(metadata, scope) { |g| g.before(scope, *meta, &block) }
+          add_hook_to_existing_matching_groups(meta, scope) { |g| g.before(scope, *meta, &block) }
           super(scope, *meta, &block)
         end
       end
@@ -1773,8 +1772,7 @@ def prepend_before(scope=nil, *meta, &block)
         handle_suite_hook(scope, meta) do
           @before_suite_hooks.unshift Hooks::BeforeHook.new(block, {})
         end || begin
-          metadata = Metadata.build_hash_from(meta.dup)
-          on_existing_matching_groups(metadata, scope) { |g| g.prepend_before(scope, *meta, &block) }
+          add_hook_to_existing_matching_groups(meta, scope) { |g| g.prepend_before(scope, *meta, &block) }
           super(scope, *meta, &block)
         end
       end
@@ -1792,8 +1790,7 @@ def after(scope=nil, *meta, &block)
         handle_suite_hook(scope, meta) do
           @after_suite_hooks.unshift Hooks::AfterHook.new(block, {})
         end || begin
-          metadata = Metadata.build_hash_from(meta.dup)
-          on_existing_matching_groups(metadata, scope) { |g| g.after(scope, *meta, &block) }
+          add_hook_to_existing_matching_groups(meta, scope) { |g| g.after(scope, *meta, &block) }
           super(scope, *meta, &block)
         end
       end
@@ -1816,8 +1813,7 @@ def append_after(scope=nil, *meta, &block)
         handle_suite_hook(scope, meta) do
           @after_suite_hooks << Hooks::AfterHook.new(block, {})
         end || begin
-          metadata = Metadata.build_hash_from(meta.dup)
-          on_existing_matching_groups(metadata, scope) { |g| g.append_after(scope, *meta, &block) }
+          add_hook_to_existing_matching_groups(meta, scope) { |g| g.append_after(scope, *meta, &block) }
           super(scope, *meta, &block)
         end
       end
@@ -1826,8 +1822,7 @@ def append_after(scope=nil, *meta, &block)
       #
       # See {Hooks#around} for full `around` hook docs.
       def around(scope=nil, *meta, &block)
-        metadata = Metadata.build_hash_from(meta.dup)
-        on_existing_matching_groups(metadata, scope) { |g| g.around(scope, *meta, &block) }
+        add_hook_to_existing_matching_groups(meta, scope) { |g| g.around(scope, *meta, &block) }
         super(scope, *meta, &block)
       end
 
@@ -2031,21 +2026,32 @@ def configure_group_with(group, module_list, application_method)
         end
       end
 
-      def on_existing_matching_groups(meta, scope=:ignore)
-        groups = world.example_groups.dup
-
-        until groups.empty?
-          group = groups.shift
+      def add_hook_to_existing_matching_groups(meta, scope, &block)
+        # For example hooks, we have to apply it to each of the top level
+        # groups, even if the groups do not match. When we apply it, we
+        # apply it with the metadata, so it will only apply to examples
+        # in the group that match the metadata.
+        # #2280 for background and discussion.
+        if scope == :example || scope == :each || scope.nil?
+          world.example_groups.each(&block)
+        else
+          meta = Metadata.build_hash_from(meta.dup)
+          on_existing_matching_groups(meta, &block)
+        end
+      end
 
-          if scope == :example || scope == :each || scope.nil? ||
-              meta.empty? || MetadataFilter.apply?(:any?, meta, group.metadata)
-            yield group
-          else
-            groups.concat(group.children)
+      def on_existing_matching_groups(meta)
+        world.traverse_example_group_trees_until do |group|
+          metadata_applies_to_group?(meta, group).tap do |applies|
+            yield group if applies
           end
         end
       end
 
+      def metadata_applies_to_group?(meta, group)
+        meta.empty? || MetadataFilter.apply?(:any?, meta, group.metadata)
+      end
+
       if RSpec::Support::RubyFeatures.module_prepends_supported?
         def safe_prepend(mod, host)
           host.__send__(:prepend, mod) unless host < mod

lib/rspec/core/example_group.rb

@@ -450,6 +450,20 @@ def self.children
         @children ||= []
       end
 
+      # @private
+      # Traverses the tree of groups, starting with `self`, then the children, recursively.
+      # Halts the traversal of a branch of the tree as soon as the passed block returns true.
+      # Note that siblings groups and their sub-trees will continue to be explored.
+      # This is intended to make it easy to find the top-most group that satisfies some
+      # condition.
+      def self.traverse_tree_until(&block)
+        return if yield self
+
+        children.each do |child|
+          child.traverse_tree_until(&block)
+        end
+      end
+
       # @private
       def self.next_runnable_index_for(file)
         if self == ExampleGroup

lib/rspec/core/world.rb

@@ -92,6 +92,19 @@ def all_examples
         FlatMap.flat_map(all_example_groups) { |g| g.examples }
       end
 
+      # @private
+      # Traverses the tree of each top level group.
+      # For each it yields the group, then the children, recursively.
+      # Halts the traversal of a branch of the tree as soon as the passed block returns true.
+      # Note that siblings groups and their sub-trees will continue to be explored.
+      # This is intended to make it easy to find the top-most group that satisfies some
+      # condition.
+      def traverse_example_group_trees_until(&block)
+        example_groups.each do |group|
+          group.traverse_tree_until(&block)
+        end
+      end
+
       # @api private
       #
       # Find line number of previous declaration.
@@ -215,6 +228,9 @@ def self.registered_example_group_files
           []
         end
 
+        def self.traverse_example_group_trees_until
+        end
+
         # :nocov:
         def self.example_groups
           []