Stop lazy alias resolver from overwriting real classes

Fixes #1662.

PR #1621 made RDoc::Constant#is_alias_for fall through to a lazy
find_alias_for lookup that returned whatever class the constant's value
named in the current store. The lazy result then flowed into
RDoc::ClassModule#update_aliases, which unconditionally wrote a dup'd
alias copy into @store.classes_hash, with two safeguards present
elsewhere bypassed:

* Context#add_module_alias refuses to clobber an existing class entry
  (the historic BasicObject = BlankSlate guard), but update_aliases
  did not.
* The prism parser only registers an alias when the constant has
  document_self set (so :nodoc: is honored), but the lazy resolver
  ignored it.

In combination these meant `Foo = Bar # :nodoc:`, where a real `Foo`
class was parsed elsewhere, would silently replace the real class's
documentation with an alias copy of `Bar` -- the literal failure mode
in #1662 (the prism shim's `Ripper = Prism::Translation::Ripper`
clobbering the real Ripper class docs).

Architectural fix
-----------------

Split RDoc::Constant#is_alias_for back into a pure accessor and a
separate Constant#resolved_alias_target lookup. is_alias_for now
returns only what was recorded explicitly (by Context#add_module_alias,
by ClassModule#update_aliases, or by ri marshal load) and never mutates
state. resolved_alias_target is the opportunistic forward-reference
lookup, used only by update_aliases as a fallback when no explicit
alias is recorded; it honors document_self so :nodoc: constants
don't lazy-resolve.

The lazy lookup itself uses a new Constant#is_alias_for_path attribute
populated by the parsers, instead of regex-matching #value at lookup
time. Both the prism parser (via ConstantReadNode/ConstantPathNode
detection in constant_path_string) and the legacy ripper parser (via
on_const-only token accumulation in parse_constant_body) already know
whether the RHS is a constant reference at parse time; we now propagate
that explicitly rather than rediscovering it from a stringly-typed
value.

Mechanical fix
--------------

ClassModule#update_aliases falls back to const.resolved_alias_target
when const.is_alias_for is unset, and gates the destination write
behind a collision check that mirrors the one in
Context#add_module_alias: skip if classes_hash/modules_hash already
holds a real (non-alias) class at the destination name. The guard now
applies uniformly to both the explicit and lazy paths.

When the fallback resolves a forward-reference target, the resolved
class is also written back to const.is_alias_for so downstream
consumers (RDoc::Stats#report_constants, RDoc::Constant#marshal_dump)
observe the alias relationship the lazy lookup found -- otherwise
forward-ref aliases get reported as undocumented and the alias slot is
serialized as nil into ri data.

For that guard to compose with add_module_alias's pre-registration
flow (which already places an alias copy at the destination expecting
update_aliases to overwrite it with the properly-marked version),
add_module_alias now sets is_alias_for on the alias copy at creation
time. Existing alias copies are therefore distinguishable from real
classes by the unconditional guard.

Tests
-----

Parser-level regressions cover :nodoc: assignment, real-class
collision, the combined :nodoc:-plus-collision scenario from #1662,
the :stopdoc:/:startdoc: workaround path, an explicit alias followed
by a same-named class re-open (which under the new guard preserves
both the alias target's methods and the methods added by the re-open),
and the post-Store#complete is_alias_for persistence on a
forward-reference alias. Each asserts both the negative (the would-be
alias didn't clobber) and the positive (the alias target keeps its
own methods and aliases list). Unit tests on update_aliases assert
the same. The two existing PR #1621 tests
(test_constant_alias_reverse_order, test_repeated_constant_alias) are
updated to exercise the renamed resolved_alias_target API; the
underlying forward-reference behavior is preserved.

Author: claude

lib/rdoc/code_object/class_module.rb

@@ -847,7 +847,15 @@ def type
 
   def update_aliases
     constants.each do |const|
-      next unless cm = const.is_alias_for
+      cm = const.is_alias_for
+      if !cm && const.is_a?(RDoc::Constant)
+        cm = const.resolved_alias_target
+        # Persist the forward-reference resolution on the source constant
+        # so Stats#report_constants and Constant#marshal_dump observe the
+        # alias relationship that the lazy lookup found.
+        const.is_alias_for = cm if cm
+      end
+      next unless cm
 
       # Resolve chained aliases (A = B = C) to the real class/module.
       cm = @store.find_class_or_module(cm.full_name) || cm
@@ -866,6 +874,14 @@ def update_aliases
       end
       cm_alias.full_name = nil # force update for new parent
 
+      # Don't clobber a real (non-alias) class/module already living at this
+      # name. Mirrors the BasicObject = BlankSlate guard in
+      # Context#add_module_alias. Existing alias copies (set by
+      # add_module_alias or a previous update_aliases pass) carry is_alias_for,
+      # so they're still overwritable here.
+      existing = @store.find_class_or_module(cm_alias.full_name)
+      next if existing && !existing.is_alias_for
+
       cm_alias.aliases.clear
       cm_alias.is_alias_for = cm
 

lib/rdoc/code_object/constant.rb

@@ -26,6 +26,14 @@ class RDoc::Constant < RDoc::CodeObject
 
   attr_accessor :visibility
 
+  ##
+  # The constant path on the RHS when the RHS is a bare constant reference
+  # (+Foo = Bar+ or +Foo = Bar::Baz+). Captured at parse time so
+  # #resolved_alias_target doesn't have to re-derive it from the textual
+  # #value. nil for other RHS shapes.
+
+  attr_accessor :is_alias_for_path
+
   ##
   # Creates a new constant with +name+, +value+ and +comment+
 
@@ -35,8 +43,9 @@ def initialize(name, value, comment)
     @name  = name
     @value = value
 
-    @is_alias_for = nil
-    @visibility   = :public
+    @is_alias_for      = nil
+    @is_alias_for_path = nil
+    @visibility        = :public
 
     self.comment = comment
   end
@@ -83,7 +92,10 @@ def full_name
   end
 
   ##
-  # The module or class this constant is an alias for
+  # The module or class this constant is an alias for, when one was recorded
+  # explicitly (by RDoc::Context#add_module_alias, RDoc::ClassModule#update_aliases,
+  # or ri marshal load). Pure accessor; see #resolved_alias_target for the
+  # opportunistic lookup path.
 
   def is_alias_for
     case @is_alias_for
@@ -92,16 +104,22 @@ def is_alias_for
       @is_alias_for = found if found
       @is_alias_for
     else
-      @is_alias_for ||= find_alias_for
+      @is_alias_for
     end
   end
 
-  # Find alias constant for a value.
-  # This is used when the constant is parsed before the class or module defined in another file.
-  # Note that module nesting information is lost, so constant lookup is inaccurate.
-
-  def find_alias_for
-    parent.find_module_named(value) if value&.match?(/\A(::)?([A-Z][A-Za-z0-9_]*)(::[A-Z][A-Za-z0-9_]*)*\z/)
+  ##
+  # Returns the class/module this constant *would* alias if #is_alias_for_path
+  # was set by the parser and that path resolves to a known class/module, or
+  # nil. Used to support `Const = RHS` parsed before `class RHS;end` is defined
+  # in another file. Pure lookup; does not mutate state. Honors :nodoc:
+  # (returns nil if document_self is false). Note that module nesting
+  # information is lost, so constant lookup is inaccurate.
+
+  def resolved_alias_target
+    return nil unless document_self
+    return nil unless @is_alias_for_path
+    parent.find_module_named(@is_alias_for_path)
   end
 
   def inspect # :nodoc:

lib/rdoc/code_object/context.rb

@@ -544,6 +544,7 @@ def add_module_alias(from, from_name, to, file)
     new_to = from.dup
     new_to.name = to.name
     new_to.full_name = nil
+    new_to.is_alias_for = from
 
     if new_to.module? then
       @store.modules_hash[to_full_name] = new_to

lib/rdoc/parser/prism_ruby.rb

@@ -767,7 +767,7 @@ def find_or_create_constant_owner_name(constant_path)
 
   # Adds a constant
 
-  def add_constant(constant_name, rhs_name, start_line, end_line)
+  def add_constant(constant_name, rhs_name, start_line, end_line, alias_path: nil)
     comment, directives = consecutive_comment(start_line)
     handle_code_object_directives(@container, directives) if directives
     owner, name = find_or_create_constant_owner_name(constant_name)
@@ -776,19 +776,21 @@ def add_constant(constant_name, rhs_name, start_line, end_line)
     constant = RDoc::Constant.new(name, rhs_name, comment)
     constant.store = @store
     constant.line = start_line
+    constant.is_alias_for_path = alias_path
     record_location(constant)
     handle_modifier_directive(constant, start_line)
     handle_modifier_directive(constant, end_line)
     owner.add_constant(constant)
+    return unless alias_path
     mod =
-      if rhs_name =~ /^::/
-        @store.find_class_or_module(rhs_name)
+      if alias_path.start_with?('::')
+        @store.find_class_or_module(alias_path)
       else
-        full_name = resolve_constant_path(rhs_name)
+        full_name = resolve_constant_path(alias_path)
         @store.find_class_or_module(full_name)
       end
     if mod && constant.document_self
-      a = @container.add_module_alias(mod, rhs_name, constant, @top_level)
+      a = @container.add_module_alias(mod, alias_path, constant, @top_level)
       a.store = @store
       a.line = start_line
       record_location(a)
@@ -1056,23 +1058,27 @@ def visit_constant_path_write_node(node)
       path = constant_path_string(node.target)
       return unless path
 
+      alias_path = constant_path_string(node.value)
       @scanner.add_constant(
         path,
-        constant_path_string(node.value) || node.value.slice,
+        alias_path || node.value.slice,
         node.location.start_line,
-        node.location.end_line
+        node.location.end_line,
+        alias_path: alias_path
       )
       @scanner.skip_comments_until(node.location.end_line)
       # Do not traverse rhs not to document `A::B = Struct.new{def undocumentable_method; end}`
     end
 
     def visit_constant_write_node(node)
       @scanner.process_comments_until(node.location.start_line - 1)
+      alias_path = constant_path_string(node.value)
       @scanner.add_constant(
         node.name.to_s,
-        constant_path_string(node.value) || node.value.slice,
+        alias_path || node.value.slice,
         node.location.start_line,
-        node.location.end_line
+        node.location.end_line,
+        alias_path: alias_path
       )
       @scanner.skip_comments_until(node.location.end_line)
       # Do not traverse rhs not to document `A = Struct.new{def undocumentable_method; end}`

lib/rdoc/parser/ripper_ruby.rb

@@ -170,6 +170,7 @@ def create_attr(container, single, name, rw, comment) # :nodoc:
   # for "::") with the name from +constant+.
 
   def create_module_alias(container, constant, rhs_name) # :nodoc:
+    constant.is_alias_for_path = rhs_name
     mod = if rhs_name =~ /^::/ then
             @store.find_class_or_module rhs_name
           else

test/rdoc/code_object/class_module_test.rb

@@ -1459,6 +1459,56 @@ def test_update_aliases_reparent_root
     assert_equal 'Klass', store.classes_hash['Klass'].full_name
   end
 
+  def test_update_aliases_does_not_overwrite_existing_class_with_same_name
+    store = RDoc::Store.new(RDoc::Options.new)
+    top_level = store.add_file 'file.rb'
+
+    object = top_level.add_class RDoc::NormalClass, 'Object'
+    real_foo = top_level.add_class RDoc::NormalClass, 'Foo'
+    real_foo.add_method RDoc::AnyMethod.new(nil, 'real_method')
+
+    other = top_level.add_class RDoc::NormalClass, 'Other'
+    other.add_method RDoc::AnyMethod.new(nil, 'other_method')
+
+    const = RDoc::Constant.new 'Foo', 'Other', ''
+    const.is_alias_for_path = 'Other'
+    const.record_location top_level
+    object.add_constant const
+
+    object.update_aliases
+
+    assert_same real_foo, store.classes_hash['Foo']
+    assert_nil store.classes_hash['Foo'].is_alias_for
+    assert_equal ['real_method'], store.classes_hash['Foo'].method_list.map(&:name)
+    assert_same other, store.classes_hash['Other']
+    assert_nil other.is_alias_for
+    assert_equal ['other_method'], other.method_list.map(&:name)
+    assert_empty other.aliases
+  end
+
+  def test_update_aliases_skips_nodoc_constant
+    store = RDoc::Store.new(RDoc::Options.new)
+    top_level = store.add_file 'file.rb'
+
+    object = top_level.add_class RDoc::NormalClass, 'Object'
+    target = top_level.add_class RDoc::NormalClass, 'Target'
+    target.add_method RDoc::AnyMethod.new(nil, 'target_method')
+
+    const = RDoc::Constant.new 'NodocAlias', 'Target', ''
+    const.is_alias_for_path = 'Target'
+    const.record_location top_level
+    const.document_self = nil
+    object.add_constant const
+
+    object.update_aliases
+
+    assert_nil store.classes_hash['NodocAlias']
+    assert_nil store.modules_hash['NodocAlias']
+    assert_same target, store.classes_hash['Target']
+    assert_equal ['target_method'], target.method_list.map(&:name)
+    assert_empty target.aliases
+  end
+
   def test_update_includes
     a = RDoc::Include.new 'M1', nil
     b = RDoc::Include.new 'M2', nil