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 that lazy-resolved path
behind a collision check that mirrors the one in
Context#add_module_alias. Explicit aliases (already vetted by
add_module_alias) keep their existing path so the two compose.

Tests
-----

Parser-level regressions cover :nodoc: assignment, real-class
collision, the combined :nodoc:-plus-collision scenario from #1662,
and the :stopdoc:/:startdoc: workaround path. 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,13 @@ def type
 
   def update_aliases
     constants.each do |const|
-      next unless cm = const.is_alias_for
+      if (cm = const.is_alias_for)
+        via_lazy = false
+      elsif const.is_a?(RDoc::Constant) && (cm = const.resolved_alias_target)
+        via_lazy = true
+      else
+        next
+      end
 
       # Resolve chained aliases (A = B = C) to the real class/module.
       cm = @store.find_class_or_module(cm.full_name) || cm
@@ -866,6 +872,15 @@ def update_aliases
       end
       cm_alias.full_name = nil # force update for new parent
 
+      # For lazy-resolved aliases (Foo = Bar where Bar is parsed elsewhere
+      # and was not vetted by add_module_alias), don't clobber a real class
+      # already living at this name. add_module_alias performs its own
+      # collision check at context.rb so the explicit path is already safe.
+      if via_lazy
+        existing = @store.find_class_or_module(cm_alias.full_name)
+        next if existing && !existing.is_alias_for
+      end
+
       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/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

test/rdoc/parser/prism_ruby_test.rb

@@ -1689,9 +1689,9 @@ class Bar; end
       class Baz; end
     RUBY
     klass = @top_level.classes.first
-    assert_equal 'Foo::Bar', klass.find_constant_named('A').is_alias_for.full_name
-    assert_equal 'Foo::Bar', klass.find_constant_named('B').is_alias_for.full_name
-    assert_equal 'Baz', klass.find_constant_named('C').is_alias_for.full_name
+    assert_equal 'Foo::Bar', klass.find_constant_named('A').resolved_alias_target.full_name
+    assert_equal 'Foo::Bar', klass.find_constant_named('B').resolved_alias_target.full_name
+    assert_equal 'Baz', klass.find_constant_named('C').resolved_alias_target.full_name
   end
 
   def test_repeated_constant_alias
@@ -1713,8 +1713,107 @@ class Baz
     RUBY
     klass = @top_level.classes.first
     assert_equal 'Bar', klass.find_constant_named('A').value
-    assert_nil klass.find_constant_named('A').is_alias_for
-    assert_equal 'Baz', klass.find_constant_named('B').is_alias_for.full_name
+    assert_nil klass.find_constant_named('A').resolved_alias_target
+    assert_equal 'Baz', klass.find_constant_named('B').resolved_alias_target.full_name
+  end
+
+  def test_nodoc_constant_assignment_does_not_become_alias
+    util_parser <<~RUBY
+      class Outer
+        class Bar
+          def bar_method; end
+        end
+        Foo = Bar # :nodoc:
+      end
+    RUBY
+    @store.complete(:public)
+    outer = @store.classes_hash['Outer']
+    refute_nil outer
+    bar = outer.classes_hash['Bar']
+    refute_nil bar
+    assert_includes bar.method_list.map(&:name), 'bar_method'
+    assert_empty bar.aliases
+    assert_nil outer.classes_hash['Foo']
+    assert_nil outer.modules_hash['Foo']
+    foo_const = outer.constants.find { |c| c.name == 'Foo' }
+    refute_nil foo_const
+    assert_nil foo_const.is_alias_for
+    assert_nil foo_const.resolved_alias_target
+  end
+
+  def test_constant_alias_does_not_overwrite_real_class_with_same_name
+    util_parser <<~RUBY
+      class Foo
+        def real_method; end
+      end
+      class Bar
+        def other_method; end
+      end
+      Foo = Bar
+    RUBY
+    @store.complete(:public)
+    foo = @store.classes_hash['Foo']
+    refute_nil foo
+    assert_nil foo.is_alias_for
+    assert_includes foo.method_list.map(&:name), 'real_method'
+    refute_includes foo.method_list.map(&:name), 'other_method'
+    bar = @store.classes_hash['Bar']
+    refute_nil bar
+    assert_nil bar.is_alias_for
+    assert_includes bar.method_list.map(&:name), 'other_method'
+    assert_empty bar.aliases
+  end
+
+  def test_nodoc_constant_assignment_preserves_real_class_with_same_name
+    util_parser <<~RUBY
+      class Foo
+        def real_method; end
+      end
+      module Bar
+        class Foo
+          def shim_method; end
+        end
+      end
+      Foo = Bar::Foo # :nodoc:
+    RUBY
+    @store.complete(:public)
+    foo = @store.classes_hash['Foo']
+    refute_nil foo
+    assert_nil foo.is_alias_for
+    assert_includes foo.method_list.map(&:name), 'real_method'
+    refute_includes foo.method_list.map(&:name), 'shim_method'
+    bar_foo = @store.classes_hash['Bar::Foo']
+    refute_nil bar_foo
+    assert_nil bar_foo.is_alias_for
+    assert_includes bar_foo.method_list.map(&:name), 'shim_method'
+    assert_empty bar_foo.aliases
+  end
+
+  def test_stopdoc_constant_assignment_preserves_real_class_with_same_name
+    util_parser <<~RUBY
+      class Foo
+        def real_method; end
+      end
+      module Bar
+        class Foo
+          def shim_method; end
+        end
+      end
+      # :stopdoc:
+      Foo = Bar::Foo
+      # :startdoc:
+    RUBY
+    @store.complete(:public)
+    foo = @store.classes_hash['Foo']
+    refute_nil foo
+    assert_nil foo.is_alias_for
+    assert_includes foo.method_list.map(&:name), 'real_method'
+    refute_includes foo.method_list.map(&:name), 'shim_method'
+    bar_foo = @store.classes_hash['Bar::Foo']
+    refute_nil bar_foo
+    assert_nil bar_foo.is_alias_for
+    assert_includes bar_foo.method_list.map(&:name), 'shim_method'
+    assert_empty bar_foo.aliases
   end
 
   def test_constant_with_singleton_class