Skip to content

Commit 294e0bd

Browse files
committed
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
1 parent 5214a30 commit 294e0bd

8 files changed

Lines changed: 85 additions & 95 deletions

File tree

lib/rdoc/code_object.rb

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -291,19 +291,7 @@ def parent
291291
return @parent if @parent
292292
return nil unless @parent_name
293293

294-
if @parent_class == RDoc::TopLevel then
295-
@parent = @store.add_file @parent_name
296-
else
297-
@parent = @store.find_class_or_module @parent_name
298-
299-
return @parent if @parent
300-
301-
begin
302-
@parent = @store.load_class @parent_name
303-
rescue RDoc::Store::MissingFileError
304-
nil
305-
end
306-
end
294+
@parent = @store&.resolve_parent(@parent_name, @parent_class)
307295
end
308296

309297
##
@@ -361,6 +349,10 @@ def stop_doc
361349
def store=(store)
362350
@store = store
363351

352+
# When a CodeObject is loaded from Marshal data, its @file is a standalone
353+
# TopLevel that needs to be replaced with the canonical one from the store.
354+
@file = @store.add_file @file.full_name if @file && @store
355+
364356
return unless @track_visibility
365357

366358
if :nodoc == options.visibility then

lib/rdoc/code_object/any_method.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -306,15 +306,6 @@ def skip_description?
306306
has_call_seq? && call_seq.nil? && !!(is_alias_for || !aliases.empty?)
307307
end
308308

309-
##
310-
# Sets the store for this method and its referenced code objects.
311-
312-
def store=(store)
313-
super
314-
315-
@file = @store.add_file @file.full_name if @file
316-
end
317-
318309
##
319310
# For methods that +super+, find the superclass method that would be called.
320311

lib/rdoc/code_object/class_module.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -702,12 +702,12 @@ def remove_nodoc_children
702702

703703
modules_hash.each_key do |name|
704704
full_name = prefix + name
705-
modules_hash.delete name unless @store.modules_hash[full_name]
705+
modules_hash.delete name unless @store.find_module_named(full_name)
706706
end
707707

708708
classes_hash.each_key do |name|
709709
full_name = prefix + name
710-
classes_hash.delete name unless @store.classes_hash[full_name]
710+
classes_hash.delete name unless @store.find_class_named(full_name)
711711
end
712712
end
713713

@@ -875,7 +875,7 @@ def update_aliases
875875
def update_includes
876876
includes.reject! do |include|
877877
mod = include.module
878-
!(String === mod) && @store.modules_hash[mod.full_name].nil?
878+
!(String === mod) && @store.find_module_named(mod.full_name).nil?
879879
end
880880

881881
includes.uniq!
@@ -891,7 +891,7 @@ def update_extends
891891
extends.reject! do |ext|
892892
mod = ext.module
893893

894-
!(String === mod) && @store.modules_hash[mod.full_name].nil?
894+
!(String === mod) && @store.find_module_named(mod.full_name).nil?
895895
end
896896

897897
extends.uniq!

lib/rdoc/code_object/constant.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -174,15 +174,6 @@ def pretty_print(q) # :nodoc:
174174
end
175175
end
176176

177-
##
178-
# Sets the store for this class or module and its contained code objects.
179-
180-
def store=(store)
181-
super
182-
183-
@file = @store.add_file @file.full_name if @file
184-
end
185-
186177
def to_s # :nodoc:
187178
parent_name = parent ? parent.full_name : '(unknown)'
188179
if is_alias_for

lib/rdoc/code_object/context.rb

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -305,12 +305,11 @@ def add_class(class_type, given_name, superclass = '::Object')
305305
if full_name =~ /^(.+)::(\w+)$/ then
306306
name = $2
307307
ename = $1
308-
enclosing = @store.classes_hash[ename] || @store.modules_hash[ename]
308+
enclosing = @store.find_class_or_module(ename)
309309
# HACK: crashes in actionpack/lib/action_view/helpers/form_helper.rb (metaprogramming)
310310
unless enclosing then
311311
# try the given name at top level (will work for the above example)
312-
enclosing = @store.classes_hash[given_name] ||
313-
@store.modules_hash[given_name]
312+
enclosing = @store.find_class_or_module(given_name)
314313
return enclosing if enclosing
315314
# not found: create the parent(s)
316315
names = ename.split('::')
@@ -358,7 +357,7 @@ def add_class(class_type, given_name, superclass = '::Object')
358357
superclass = nil if superclass == full_name
359358
end
360359

361-
klass = @store.classes_hash[full_name]
360+
klass = @store.find_class_named(full_name)
362361

363362
if klass then
364363
# if TopLevel, it may not be registered in the classes:
@@ -384,8 +383,7 @@ def add_class(class_type, given_name, superclass = '::Object')
384383
else
385384
klass = class_type.new name, superclass
386385

387-
enclosing.add_class_or_module(klass, enclosing.classes_hash,
388-
@store.classes_hash)
386+
enclosing.add_class_or_module(klass, enclosing.classes_hash)
389387
end
390388
end
391389

@@ -395,13 +393,12 @@ def add_class(class_type, given_name, superclass = '::Object')
395393
end
396394

397395
##
398-
# Adds the class or module +mod+ to the modules or
399-
# classes Hash +self_hash+, and to +all_hash+ (either
400-
# <tt>TopLevel::modules_hash</tt> or <tt>TopLevel::classes_hash</tt>),
396+
# Adds the class or module +mod+ to the local Hash +self_hash+,
397+
# and registers it with the store,
401398
# unless #done_documenting is +true+. Sets the #parent of +mod+
402399
# to +self+, and its #section to #current_section. Returns +mod+.
403400

404-
def add_class_or_module(mod, self_hash, all_hash)
401+
def add_class_or_module(mod, self_hash)
405402
mod.section = current_section # TODO declaring context? something is
406403
# wrong here...
407404
mod.parent = self
@@ -412,7 +409,11 @@ def add_class_or_module(mod, self_hash, all_hash)
412409
self_hash[mod.name] = mod
413410
# this must be done AFTER adding mod to its parent, so that the full
414411
# name is correct:
415-
all_hash[mod.full_name] = mod
412+
if mod.module? then
413+
@store.modules_hash[mod.full_name] = mod
414+
else
415+
@store.classes_hash[mod.full_name] = mod
416+
end
416417
if @store.unmatched_constant_alias[mod.full_name] then
417418
to, file = @store.unmatched_constant_alias[mod.full_name]
418419
add_module_alias mod, mod.name, to, file
@@ -508,16 +509,16 @@ def add_module(class_type, name)
508509
return mod if mod
509510

510511
full_name = child_name name
511-
mod = @store.modules_hash[full_name] || class_type.new(name)
512+
mod = @store.find_module_named(full_name) || class_type.new(name)
512513

513-
add_class_or_module mod, @modules, @store.modules_hash
514+
add_class_or_module mod, @modules
514515
end
515516

516517
##
517518
# Adds a module by +RDoc::NormalModule+ instance. See also #add_module.
518519

519520
def add_module_by_normal_module(mod)
520-
add_class_or_module mod, @modules, @store.modules_hash
521+
add_class_or_module mod, @modules
521522
end
522523

523524
##
@@ -1222,8 +1223,9 @@ def upgrade_to_class(mod, class_type, enclosing)
12221223
klass.store = @store
12231224

12241225
# if it was there, then we keep it even if done_documenting
1226+
@store.modules_hash.delete mod.full_name
12251227
@store.classes_hash[mod.full_name] = klass
1226-
enclosing.classes_hash[mod.name] = klass
1228+
enclosing.classes_hash[mod.name] = klass
12271229

12281230
klass
12291231
end

lib/rdoc/code_object/method_attr.rb

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -147,15 +147,6 @@ def see
147147
@see
148148
end
149149

150-
##
151-
# Sets the store for this class or module and its contained code objects.
152-
153-
def store=(store)
154-
super
155-
156-
@file = @store.add_file @file.full_name if @file
157-
end
158-
159150
def find_see # :nodoc:
160151
return nil if singleton || is_alias_for
161152

@@ -172,7 +163,7 @@ def find_method_or_attribute(name) # :nodoc:
172163
return nil unless parent.respond_to? :ancestors
173164

174165
searched = parent.ancestors
175-
kernel = @store.modules_hash['Kernel']
166+
kernel = @store.find_module_named('Kernel')
176167

177168
searched << kernel if kernel &&
178169
parent != kernel && !searched.include?(kernel)

lib/rdoc/code_object/mixin.rb

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -77,43 +77,9 @@ def inspect # :nodoc:
7777

7878
def module
7979
return @module if @module
80-
81-
# search the current context
8280
return @name unless parent
83-
full_name = parent.child_name(@name)
84-
@module = @store.modules_hash[full_name]
85-
return @module if @module
86-
return @name if @name =~ /^::/
87-
88-
# search the includes before this one, in reverse order
89-
searched = parent.includes.take_while { |i| i != self }.reverse
90-
searched.each do |i|
91-
inc = i.module
92-
next if String === inc
93-
full_name = inc.child_name(@name)
94-
@module = @store.modules_hash[full_name]
95-
return @module if @module
96-
end
97-
98-
# go up the hierarchy of names
99-
up = parent.parent
100-
while up
101-
full_name = up.child_name(@name)
102-
@module = @store.modules_hash[full_name]
103-
return @module if @module
104-
up = up.parent
105-
end
106-
107-
@name
108-
end
109-
110-
##
111-
# Sets the store for this class or module and its contained code objects.
112-
113-
def store=(store)
114-
super
11581

116-
@file = @store.add_file @file.full_name if @file
82+
@module = @store&.resolve_mixin(@name, parent, self) || @name
11783
end
11884

11985
def to_s # :nodoc:

lib/rdoc/store.rb

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,63 @@ def modules_hash
719719
@modules_hash
720720
end
721721

722+
##
723+
# Resolves a parent CodeObject by +parent_name+ and +parent_class+.
724+
# This encapsulates the lazy parent resolution logic: if the parent is a
725+
# TopLevel, it is resolved via add_file; otherwise it is looked up in the
726+
# class/module registry, falling back to loading from disk.
727+
728+
def resolve_parent(parent_name, parent_class)
729+
if parent_class == RDoc::TopLevel
730+
add_file parent_name
731+
else
732+
find_class_or_module(parent_name) || begin
733+
load_class(parent_name)
734+
rescue MissingFileError
735+
nil
736+
end
737+
end
738+
end
739+
740+
##
741+
# Resolves a mixin's module reference by walking the namespace hierarchy.
742+
# +name+ is the module name to resolve, +parent_context+ is the Context
743+
# containing the mixin, and +mixin+ is the Mixin object itself (used to
744+
# limit the include search to only includes before this one).
745+
#
746+
# Returns the resolved RDoc::NormalModule, or nil if not found.
747+
748+
def resolve_mixin(name, parent_context, mixin)
749+
return nil unless parent_context
750+
751+
# search the current context
752+
full_name = parent_context.child_name(name)
753+
found = find_module_named(full_name)
754+
return found if found
755+
return nil if name =~ /^::/
756+
757+
# search the includes before this one, in reverse order
758+
searched = parent_context.includes.take_while { |i| i != mixin }.reverse
759+
searched.each do |i|
760+
inc = i.module
761+
next if String === inc
762+
full_name = inc.child_name(name)
763+
found = find_module_named(full_name)
764+
return found if found
765+
end
766+
767+
# go up the hierarchy of names
768+
up = parent_context.parent
769+
while up
770+
full_name = up.child_name(name)
771+
found = find_module_named(full_name)
772+
return found if found
773+
up = up.parent
774+
end
775+
776+
nil
777+
end
778+
722779
##
723780
# Returns the RDoc::TopLevel that is a file and has the given +name+
724781

0 commit comments

Comments
 (0)