Fix module recursive lookup bug

Author: tompng

lib/rdoc/context.rb

@@ -1261,4 +1261,50 @@ def upgrade_to_class mod, class_type, enclosing
 
   autoload :Section, "#{__dir__}/context/section"
 
+  ##
+  # Attempts to locate the module object in this context.
+  #
+  # The scoping rules of Ruby to resolve the name of an included module are:
+  # - first search constant directly defined in nested modules from inside to outside
+  # - if not found, look into the children of included modules,
+  #   in reverse inclusion order;
+  # - if still not found, look into included modules of Object
+
+  def lookup_module(name, before: nil, searched: {}.compare_by_identity)
+    # Search nested modules first
+    nesting = self
+    while nesting
+      full_name = nesting.child_name(name)
+      mod = @store.modules_hash[full_name]
+      return mod if mod
+      nesting = nesting.parent
+    end
+
+    # Search included modules recursively
+    mod = find_module(name, before: before, searched: searched)
+    return mod if mod
+
+    # Search Object recursively
+    top_level.object_class.find_module(name, searched: searched)
+  end
+
+  ##
+  # Recursively search for a module in this context and its includes.
+
+  def find_module(name, before: nil, searched: {}.compare_by_identity)
+    return if searched.include?(self)
+    searched[self] = true
+    full_name = child_name(name)
+    mod = @store.modules_hash[full_name]
+    return mod if mod
+
+    # recursively search the includes in reverse order
+    includes.take_while { |i| i != before }.reverse_each do |i|
+      inc = i.module
+      next if String === inc
+      mod = inc.find_module(name, searched: searched)
+      return mod if mod
+    end
+    nil
+  end
 end

lib/rdoc/mixin.rb

@@ -59,49 +59,16 @@ def inspect # :nodoc:
   # Attempts to locate the included module object.  Returns the name if not
   # known.
   #
-  # The scoping rules of Ruby to resolve the name of an included module are:
-  # - first look into the children of the current context;
-  # - if not found, look into the children of included modules,
-  #   in reverse inclusion order;
-  # - if still not found, go up the hierarchy of names.
-  #
-  # This method has <code>O(n!)</code> behavior when the module calling
-  # include is referencing nonexistent modules.  Avoid calling #module until
-  # after all the files are parsed.  This behavior is due to ruby's constant
-  # lookup behavior.
-  #
-  # As of the beginning of October, 2011, no gem includes nonexistent modules.
+  # Avoid calling #module until after all the files are parsed.
+  # This behavior is due to ruby's constant lookup behavior.
 
   def module
     return @module if @module
+    return @module = @name unless parent
+    return @module = @name if @name.start_with?('::')
 
-    # search the current context
-    return @name unless parent
-    full_name = parent.child_name(@name)
-    @module = @store.modules_hash[full_name]
-    return @module if @module
-    return @name if @name =~ /^::/
-
-    # search the includes before this one, in reverse order
-    searched = parent.includes.take_while { |i| i != self }.reverse
-    searched.each do |i|
-      inc = i.module
-      next if String === inc
-      full_name = inc.child_name(@name)
-      @module = @store.modules_hash[full_name]
-      return @module if @module
-    end
-
-    # go up the hierarchy of names
-    up = parent.parent
-    while up
-      full_name = up.child_name(@name)
-      @module = @store.modules_hash[full_name]
-      return @module if @module
-      up = up.parent
-    end
-
-    @name
+    # search the includes before this one
+    @module = parent.lookup_module(@name, before: self) || @name
   end
 
   ##

test/rdoc/test_rdoc_include.rb

@@ -63,7 +63,7 @@ def test_module_extended
     m1_k1.add_include i1_k0_m4
 
     assert_equal [i1_m1, i1_m2, i1_m3, i1_m4, i1_k0_m4], m1_k1.includes
-    assert_equal [m1_m2_k0_m4, m1_m2_m3_m4, m1_m2_m3, m1_m2, m1, @object,
+    assert_equal [m1_m2_k0_m4, m1_m2_m4, m1_m3, m1_m2, m1, @object,
                   'BasicObject'], m1_k1.ancestors
 
     m1_k2 = m1.add_class RDoc::NormalClass, 'Klass2'
@@ -96,6 +96,32 @@ def test_module_extended
     assert_equal [m1_m2_m4, m1_m2, m1, @object, 'BasicObject'], m1_k3.ancestors
   end
 
+  def test_include_through_include
+    top_level = @store.add_file 'file.rb'
+
+    mod1 = top_level.add_module RDoc::NormalModule, 'Mod1'
+    mod2 = top_level.add_module RDoc::NormalModule, 'Mod2'
+    mod3 = top_level.add_module RDoc::NormalModule, 'Mod3'
+    submod = mod1.add_module RDoc::NormalModule, 'Sub'
+    mod2.add_include RDoc::Include.new('Mod1', '')
+    mod3.add_include RDoc::Include.new('Mod2', '')
+    mod3.add_include RDoc::Include.new('Sub', '')
+    assert_equal [submod, mod2], mod3.ancestors
+  end
+
+  def test_include_through_top_level_include
+    top_level = @store.add_file 'file.rb'
+
+    mod1 = top_level.add_module RDoc::NormalModule, 'Mod1'
+    mod2 = top_level.add_module RDoc::NormalModule, 'Mod2'
+    mod3 = mod2.add_module RDoc::NormalModule, 'Mod3'
+    submod = mod1.add_module RDoc::NormalModule, 'Sub'
+    mod2.add_include RDoc::Include.new('Mod1', '')
+    top_level.add_include RDoc::Include.new('Mod2', '')
+    mod3.add_include RDoc::Include.new('Sub', '')
+    assert_equal [submod], mod3.ancestors
+  end
+
   def test_store_equals
     incl = RDoc::Include.new 'M', nil
     incl.record_location RDoc::TopLevel.new @top_level.name