Stop lazy alias resolver from overwriting real classes

PR #1621 made RDoc::Constant#is_alias_for fall through to a lazy
find_alias_for lookup that returns whatever class the constant's value
names 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`.

Refactor:

* Split the lazy resolver out of the getter. RDoc::Constant#is_alias_for
  is now a pure accessor over @is_alias_for; the opportunistic
  value-shaped lookup is exposed as #resolved_alias_target, which is
  pure (no mutation) and honors :nodoc:.
* RDoc::ClassModule#update_aliases is the single caller of the lazy
  path and gates writes behind a collision check that mirrors
  Context#add_module_alias when the alias was lazy-resolved. Explicit
  aliases (vetted by add_module_alias's own collision guard) keep their
  existing behavior so add_module_alias + update_aliases still compose.

Tests:

* Parser-level regressions cover :nodoc: assignment, real-class
  collision, the combined scenario, and the :stopdoc:/:startdoc:
  workaround path.
* Unit tests on update_aliases cover both the collision skip and the
  :nodoc: skip.
* The two 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.respond_to?(:resolved_alias_target) && (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

@@ -83,7 +83,12 @@ def full_name
   end
 
   ##
-  # The module or class this constant is an alias for
+  # The module or class this constant is an alias for, if one was recorded
+  # explicitly (by the parser via RDoc::Context#add_module_alias, by
+  # RDoc::ClassModule#update_aliases, or by ri marshal load). Pure accessor:
+  # never consults the store on its own. Callers that want to opportunistically
+  # resolve a class-path-shaped value to a class/module should use
+  # #resolved_alias_target.
 
   def is_alias_for
     case @is_alias_for
@@ -92,16 +97,21 @@ 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 its #value names
+  # one in the current store, 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 value&.match?(/\A(::)?([A-Z][A-Za-z0-9_]*)(::[A-Z][A-Za-z0-9_]*)*\z/)
+    parent.find_module_named(value)
   end
 
   def inspect # :nodoc:

test/rdoc/code_object/class_module_test.rb

@@ -1459,6 +1459,47 @@ 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'
+
+    const = RDoc::Constant.new 'Foo', '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_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'
+
+    const = RDoc::Constant.new 'NodocAlias', '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_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,87 @@ 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
+        end
+        Foo = Bar # :nodoc:
+      end
+    RUBY
+    @store.complete(:public)
+    outer = @store.classes_hash['Outer']
+    refute_nil outer
+    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'
+  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'
+  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'
   end
 
   def test_constant_with_singleton_class