From c58393526099f5dfbb896dc70c9fc399cf2abecd Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Sun, 22 Feb 2026 12:14:35 +0000 Subject: [PATCH] Decouple Store internals from CodeObject hierarchy Move resolution logic out of CodeObjects into Store: - Add `Store#resolve_parent` to encapsulate lazy parent loading (previously `CodeObject#parent` called `load_class` for disk I/O) - Add `Store#resolve_mixin` to encapsulate namespace-walking (previously 30 lines of scoping logic lived in `Mixin#module`) Consolidate file normalization in base `CodeObject#store=`: - Remove redundant `store=` overrides from `MethodAttr`, `AnyMethod`, `Mixin`, and `Constant` that all did identical `add_file` calls - Fix pre-existing bug where `AnyMethod` called `add_file` twice Replace direct hash reads with existing finder methods: - Use `find_class_or_module`, `find_class_named`, `find_module_named` instead of `@store.classes_hash[]` / `@store.modules_hash[]` Simplify `add_class_or_module` signature: - Drop the `all_hash` parameter that passed Store's internal hash by reference; method now registers directly via the hash accessors --- lib/rdoc/code_object.rb | 18 +++------ lib/rdoc/code_object/any_method.rb | 9 ----- lib/rdoc/code_object/class_module.rb | 8 ++-- lib/rdoc/code_object/constant.rb | 9 ----- lib/rdoc/code_object/context.rb | 32 ++++++++-------- lib/rdoc/code_object/method_attr.rb | 11 +----- lib/rdoc/code_object/mixin.rb | 36 +----------------- lib/rdoc/store.rb | 57 ++++++++++++++++++++++++++++ test/rdoc/parser/ruby_test.rb | 4 +- 9 files changed, 87 insertions(+), 97 deletions(-) diff --git a/lib/rdoc/code_object.rb b/lib/rdoc/code_object.rb index 388863b06c..45d6bc8987 100644 --- a/lib/rdoc/code_object.rb +++ b/lib/rdoc/code_object.rb @@ -291,19 +291,7 @@ def parent return @parent if @parent return nil unless @parent_name - if @parent_class == RDoc::TopLevel then - @parent = @store.add_file @parent_name - else - @parent = @store.find_class_or_module @parent_name - - return @parent if @parent - - begin - @parent = @store.load_class @parent_name - rescue RDoc::Store::MissingFileError - nil - end - end + @parent = @store&.resolve_parent(@parent_name, @parent_class) end ## @@ -361,6 +349,10 @@ def stop_doc def store=(store) @store = store + # When a CodeObject is loaded from Marshal data, its @file is a standalone + # TopLevel that needs to be replaced with the canonical one from the store. + @file = @store.add_file @file.full_name if @file && @store + return unless @track_visibility if :nodoc == options.visibility then diff --git a/lib/rdoc/code_object/any_method.rb b/lib/rdoc/code_object/any_method.rb index f56110ea11..f63c31bf2d 100644 --- a/lib/rdoc/code_object/any_method.rb +++ b/lib/rdoc/code_object/any_method.rb @@ -306,15 +306,6 @@ def skip_description? has_call_seq? && call_seq.nil? && !!(is_alias_for || !aliases.empty?) end - ## - # Sets the store for this method and its referenced code objects. - - def store=(store) - super - - @file = @store.add_file @file.full_name if @file - end - ## # For methods that +super+, find the superclass method that would be called. diff --git a/lib/rdoc/code_object/class_module.rb b/lib/rdoc/code_object/class_module.rb index 00e406b1a6..43c64a229c 100644 --- a/lib/rdoc/code_object/class_module.rb +++ b/lib/rdoc/code_object/class_module.rb @@ -702,12 +702,12 @@ def remove_nodoc_children modules_hash.each_key do |name| full_name = prefix + name - modules_hash.delete name unless @store.modules_hash[full_name] + modules_hash.delete name unless @store.find_module_named(full_name) end classes_hash.each_key do |name| full_name = prefix + name - classes_hash.delete name unless @store.classes_hash[full_name] + classes_hash.delete name unless @store.find_class_named(full_name) end end @@ -875,7 +875,7 @@ def update_aliases def update_includes includes.reject! do |include| mod = include.module - !(String === mod) && @store.modules_hash[mod.full_name].nil? + !(String === mod) && @store.find_module_named(mod.full_name).nil? end includes.uniq! @@ -891,7 +891,7 @@ def update_extends extends.reject! do |ext| mod = ext.module - !(String === mod) && @store.modules_hash[mod.full_name].nil? + !(String === mod) && @store.find_module_named(mod.full_name).nil? end extends.uniq! diff --git a/lib/rdoc/code_object/constant.rb b/lib/rdoc/code_object/constant.rb index 8823b0bd67..8b5c31e4cf 100644 --- a/lib/rdoc/code_object/constant.rb +++ b/lib/rdoc/code_object/constant.rb @@ -174,15 +174,6 @@ def pretty_print(q) # :nodoc: end end - ## - # Sets the store for this class or module and its contained code objects. - - def store=(store) - super - - @file = @store.add_file @file.full_name if @file - end - def to_s # :nodoc: parent_name = parent ? parent.full_name : '(unknown)' if is_alias_for diff --git a/lib/rdoc/code_object/context.rb b/lib/rdoc/code_object/context.rb index bd42248424..cb573f2a10 100644 --- a/lib/rdoc/code_object/context.rb +++ b/lib/rdoc/code_object/context.rb @@ -305,12 +305,11 @@ def add_class(class_type, given_name, superclass = '::Object') if full_name =~ /^(.+)::(\w+)$/ then name = $2 ename = $1 - enclosing = @store.classes_hash[ename] || @store.modules_hash[ename] + enclosing = @store.find_class_or_module(ename) # HACK: crashes in actionpack/lib/action_view/helpers/form_helper.rb (metaprogramming) unless enclosing then # try the given name at top level (will work for the above example) - enclosing = @store.classes_hash[given_name] || - @store.modules_hash[given_name] + enclosing = @store.find_class_or_module(given_name) return enclosing if enclosing # not found: create the parent(s) names = ename.split('::') @@ -358,7 +357,7 @@ def add_class(class_type, given_name, superclass = '::Object') superclass = nil if superclass == full_name end - klass = @store.classes_hash[full_name] + klass = @store.find_class_named(full_name) if klass then # if TopLevel, it may not be registered in the classes: @@ -384,8 +383,7 @@ def add_class(class_type, given_name, superclass = '::Object') else klass = class_type.new name, superclass - enclosing.add_class_or_module(klass, enclosing.classes_hash, - @store.classes_hash) + enclosing.add_class_or_module(klass, enclosing.classes_hash) end end @@ -395,13 +393,12 @@ def add_class(class_type, given_name, superclass = '::Object') end ## - # Adds the class or module +mod+ to the modules or - # classes Hash +self_hash+, and to +all_hash+ (either - # TopLevel::modules_hash or TopLevel::classes_hash), + # Adds the class or module +mod+ to the local Hash +self_hash+, + # and registers it with the store, # unless #done_documenting is +true+. Sets the #parent of +mod+ # to +self+, and its #section to #current_section. Returns +mod+. - def add_class_or_module(mod, self_hash, all_hash) + def add_class_or_module(mod, self_hash) mod.section = current_section # TODO declaring context? something is # wrong here... mod.parent = self @@ -412,7 +409,11 @@ def add_class_or_module(mod, self_hash, all_hash) self_hash[mod.name] = mod # this must be done AFTER adding mod to its parent, so that the full # name is correct: - all_hash[mod.full_name] = mod + if mod.module? then + @store.modules_hash[mod.full_name] = mod + else + @store.classes_hash[mod.full_name] = mod + end if @store.unmatched_constant_alias[mod.full_name] then to, file = @store.unmatched_constant_alias[mod.full_name] add_module_alias mod, mod.name, to, file @@ -508,16 +509,16 @@ def add_module(class_type, name) return mod if mod full_name = child_name name - mod = @store.modules_hash[full_name] || class_type.new(name) + mod = @store.find_module_named(full_name) || class_type.new(name) - add_class_or_module mod, @modules, @store.modules_hash + add_class_or_module mod, @modules end ## # Adds a module by +RDoc::NormalModule+ instance. See also #add_module. def add_module_by_normal_module(mod) - add_class_or_module mod, @modules, @store.modules_hash + add_class_or_module mod, @modules end ## @@ -1222,8 +1223,9 @@ def upgrade_to_class(mod, class_type, enclosing) klass.store = @store # if it was there, then we keep it even if done_documenting + @store.modules_hash.delete mod.full_name @store.classes_hash[mod.full_name] = klass - enclosing.classes_hash[mod.name] = klass + enclosing.classes_hash[mod.name] = klass klass end diff --git a/lib/rdoc/code_object/method_attr.rb b/lib/rdoc/code_object/method_attr.rb index 3169640982..db8664558e 100644 --- a/lib/rdoc/code_object/method_attr.rb +++ b/lib/rdoc/code_object/method_attr.rb @@ -147,15 +147,6 @@ def see @see end - ## - # Sets the store for this class or module and its contained code objects. - - def store=(store) - super - - @file = @store.add_file @file.full_name if @file - end - def find_see # :nodoc: return nil if singleton || is_alias_for @@ -172,7 +163,7 @@ def find_method_or_attribute(name) # :nodoc: return nil unless parent.respond_to? :ancestors searched = parent.ancestors - kernel = @store.modules_hash['Kernel'] + kernel = @store.find_module_named('Kernel') searched << kernel if kernel && parent != kernel && !searched.include?(kernel) diff --git a/lib/rdoc/code_object/mixin.rb b/lib/rdoc/code_object/mixin.rb index e7b37e0941..f3e54d7c4a 100644 --- a/lib/rdoc/code_object/mixin.rb +++ b/lib/rdoc/code_object/mixin.rb @@ -77,43 +77,9 @@ def inspect # :nodoc: def module return @module if @module - - # 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 - end - - ## - # Sets the store for this class or module and its contained code objects. - - def store=(store) - super - @file = @store.add_file @file.full_name if @file + @module = @store&.resolve_mixin(@name, parent, self) || @name end def to_s # :nodoc: diff --git a/lib/rdoc/store.rb b/lib/rdoc/store.rb index 57429e6aad..a2790e1c51 100644 --- a/lib/rdoc/store.rb +++ b/lib/rdoc/store.rb @@ -719,6 +719,63 @@ def modules_hash @modules_hash end + ## + # Resolves a parent CodeObject by +parent_name+ and +parent_class+. + # This encapsulates the lazy parent resolution logic: if the parent is a + # TopLevel, it is resolved via add_file; otherwise it is looked up in the + # class/module registry, falling back to loading from disk. + + def resolve_parent(parent_name, parent_class) + if parent_class == RDoc::TopLevel + add_file parent_name + else + find_class_or_module(parent_name) || begin + load_class(parent_name) + rescue MissingFileError + nil + end + end + end + + ## + # Resolves a mixin's module reference by walking the namespace hierarchy. + # +name+ is the module name to resolve, +parent_context+ is the Context + # containing the mixin, and +mixin+ is the Mixin object itself (used to + # limit the include search to only includes before this one). + # + # Returns the resolved RDoc::NormalModule, or nil if not found. + + def resolve_mixin(name, parent_context, mixin) + return nil unless parent_context + + # search the current context + full_name = parent_context.child_name(name) + found = find_module_named(full_name) + return found if found + return nil if name =~ /^::/ + + # search the includes before this one, in reverse order + searched = parent_context.includes.take_while { |i| i != mixin }.reverse + searched.each do |i| + inc = i.module + next if String === inc + full_name = inc.child_name(name) + found = find_module_named(full_name) + return found if found + end + + # go up the hierarchy of names + up = parent_context.parent + while up + full_name = up.child_name(name) + found = find_module_named(full_name) + return found if found + up = up.parent + end + + nil + end + ## # Returns the RDoc::TopLevel that is a file and has the given +name+ diff --git a/test/rdoc/parser/ruby_test.rb b/test/rdoc/parser/ruby_test.rb index 413de602f8..452ebd961d 100644 --- a/test/rdoc/parser/ruby_test.rb +++ b/test/rdoc/parser/ruby_test.rb @@ -2016,8 +2016,8 @@ def test_parse_method_constant @parser.parse_method m, RDoc::Parser::Ruby::NORMAL, tk, @comment - assert_empty @store.modules_hash.keys - assert_equal %w[M], @store.classes_hash.keys + assert_equal %w[M], @store.modules_hash.keys + assert_empty @store.classes_hash.keys end def test_parse_method_false