From f4db99c5bb44b37894a7d10ffa4874bf97df04c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Apr 2026 09:22:09 +0000 Subject: [PATCH 1/4] 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. --- lib/rdoc/code_object/class_module.rb | 18 ++- lib/rdoc/code_object/constant.rb | 38 +++-- lib/rdoc/code_object/context.rb | 1 + lib/rdoc/parser/prism_ruby.rb | 24 ++-- lib/rdoc/parser/ripper_ruby.rb | 1 + test/rdoc/code_object/class_module_test.rb | 50 +++++++ test/rdoc/parser/prism_ruby_test.rb | 153 ++++++++++++++++++++- 7 files changed, 260 insertions(+), 25 deletions(-) diff --git a/lib/rdoc/code_object/class_module.rb b/lib/rdoc/code_object/class_module.rb index de7665a589..9db62aea54 100644 --- a/lib/rdoc/code_object/class_module.rb +++ b/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 diff --git a/lib/rdoc/code_object/constant.rb b/lib/rdoc/code_object/constant.rb index 20015b86f1..a6dfa4f4cf 100644 --- a/lib/rdoc/code_object/constant.rb +++ b/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: diff --git a/lib/rdoc/code_object/context.rb b/lib/rdoc/code_object/context.rb index 6bdb475f00..a619ccfdf2 100644 --- a/lib/rdoc/code_object/context.rb +++ b/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 diff --git a/lib/rdoc/parser/prism_ruby.rb b/lib/rdoc/parser/prism_ruby.rb index e8dedd5c18..c13b612257 100644 --- a/lib/rdoc/parser/prism_ruby.rb +++ b/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,11 +1058,13 @@ 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}` @@ -1068,11 +1072,13 @@ def visit_constant_path_write_node(node) 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}` diff --git a/lib/rdoc/parser/ripper_ruby.rb b/lib/rdoc/parser/ripper_ruby.rb index f72c15bf80..0871bd7aa8 100644 --- a/lib/rdoc/parser/ripper_ruby.rb +++ b/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 diff --git a/test/rdoc/code_object/class_module_test.rb b/test/rdoc/code_object/class_module_test.rb index 2fb172a3f7..262882320c 100644 --- a/test/rdoc/code_object/class_module_test.rb +++ b/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 diff --git a/test/rdoc/parser/prism_ruby_test.rb b/test/rdoc/parser/prism_ruby_test.rb index f018123f7b..2cb6ccbce1 100644 --- a/test/rdoc/parser/prism_ruby_test.rb +++ b/test/rdoc/parser/prism_ruby_test.rb @@ -1689,9 +1689,30 @@ 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_forward_reference_constant_alias_persists_is_alias_for_after_complete + util_parser <<~RUBY + # Parsed first + class Foo + A = Bar + end + + # Parsed later + class Foo + class Bar; end + end + RUBY + klass = @top_level.classes.first + a_const = klass.find_constant_named('A') + @store.complete(:public) + refute_nil a_const.is_alias_for, + 'Store#complete should persist the forward-ref alias on the source constant ' \ + '(otherwise Stats#report_constants/Constant#marshal_dump miss it)' + assert_equal 'Foo::Bar', a_const.is_alias_for.full_name end def test_repeated_constant_alias @@ -1713,8 +1734,130 @@ 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_constant_alias_then_class_reopen_keeps_added_methods + util_parser <<~RUBY + class Bar + def bar_method; end + end + Foo = Bar + class Foo + def real_method; end + end + RUBY + @store.complete(:public) + foo = @store.classes_hash['Foo'] + refute_nil foo + refute_nil foo.is_alias_for + assert_equal 'Bar', foo.is_alias_for.full_name + assert_includes foo.method_list.map(&:name), 'bar_method' + assert_includes foo.method_list.map(&:name), 'real_method' + bar = @store.classes_hash['Bar'] + refute_nil bar + assert_nil bar.is_alias_for + assert_includes bar.method_list.map(&:name), 'bar_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' + 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 From 3e0ca4e9ee06243052e2b73b7a0657946f87edff Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Apr 2026 09:22:10 +0000 Subject: [PATCH 2/4] Document parser duality, alias flow, and ri marshal compat in AGENTS.md Replace the bare two-line "Pluggable System" subsection of Architecture Notes with three substantive subsections capturing invariants future agents need: * "Parsers and Generators" -- names the two Ruby parser classes (PrismRuby default, RipperRuby legacy), explains the RDocParserPrismTestCases mixin pattern, and notes that the ripper variant is gated by RDOC_USE_RIPPER_PARSER (so a local bundle exec rake only exercises prism by default; CI runs ripper in a separate job). * "Code Object Model and Constant Aliases" -- describes the two-phase build (parse-time add_constant/add_module_alias vs. Store#complete -> ClassModule#update_aliases), the invariant that safeguards on one path must mirror on the other, and the known lexical-scope approximation in Context#find_enclosing_module_named. * "Marshal / ri Data Compatibility" -- names the in-tree consumers (ri CLI in lib/rdoc/ri/driver.rb, ri --server servlet in lib/rdoc/ri/servlet.rb) and the per-class MARSHAL_VERSION discipline needed to keep locally-cached .ri data loadable across rdoc upgrades. --- AGENTS.md | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 81acf64ae5..4adbb959a3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -234,11 +234,25 @@ exe/ ## Architecture Notes -### Pluggable System +### Parsers and Generators -- **Parsers:** Ruby, C, Markdown, RD, Prism-based Ruby (experimental) +- **Parsers:** Prism-based Ruby (default, `RDoc::Parser::PrismRuby`), legacy ripper-based Ruby (`RDoc::Parser::RipperRuby`, opt-in via `RDOC_USE_RIPPER_PARSER=1`), C, Markdown, RD - **Generators:** HTML/Aliki (default), HTML/Darkfish (deprecated), RI, POT (gettext), JSON, Markup +Both Ruby parsers must produce equivalent code-object trees, so parser tests live in the `RDocParserPrismTestCases` module (`test/rdoc/parser/prism_ruby_test.rb`) and are included by both `RDocParserPrismRubyTest` and `RDocParserRipperRubyWithPrismRubyTestCasesTest`. The ripper variant is gated on `RDOC_USE_RIPPER_PARSER`, so `bundle exec rake` locally only runs prism; CI exercises ripper in a separate job. Add new parser tests to the mixin, and run `RDOC_USE_RIPPER_PARSER=1 bundle exec rake` locally before declaring a parser change done. + +### Code Object Model and Constant Aliases + +The code-object tree (`lib/rdoc/code_object/`) is built in two phases. Parse-time work happens in the parsers and `RDoc::Context` (`add_constant`, `add_module_alias`). Finalization happens in `Store#complete`, which calls `ClassModule#update_aliases` on each container — this is where forward-reference aliases (`Foo = Bar` parsed before `class Bar` in another file) get resolved via `Constant#resolved_alias_target`. + +If you add an invariant to one of these paths — for example the `Context#add_module_alias` collision guard that refuses to clobber an existing class — mirror it on the other. The two paths are not interchangeable: `add_module_alias` runs against partial store state and does extra bookkeeping (`unmatched_constant_alias`); `update_aliases` runs against the finalized store and writes the alias copies into `classes_hash` / `modules_hash`. + +**Known limitation: lexical scope.** `Context#find_enclosing_module_named` walks the syntactic parent chain as a stand-in for Ruby's lexical constant lookup. The prism parser doesn't represent module nesting via the parent chain at all (see the docstring on `Context#find_enclosing_module_named`), so alias resolution in deeply nested or re-opened classes can pick the wrong target. Fixing this properly requires capturing lexical scope at parse time — a feature change rather than an incremental fix. + +### Marshal / ri Data Compatibility + +`RDoc::Constant`, `RDoc::ClassModule`, and other code objects implement `marshal_dump` / `marshal_load` to persist ri data on disk, gated by a per-class `MARSHAL_VERSION` constant. The `ri` CLI (`lib/rdoc/ri/driver.rb`) and the `ri --server` servlet (`lib/rdoc/ri/servlet.rb`) read this format. Any change that alters the dumped array — adding/removing slots, reinterpreting an existing slot's meaning — needs `MARSHAL_VERSION` bumped and the loader taught to handle older payloads, otherwise locally-cached `.ri` data from an earlier rdoc version stops loading after an upgrade. + ### Live Preview Server (`RDoc::Server`) The server (`lib/rdoc/server.rb`) provides `rdoc --server` for live documentation preview. From 1846a8a9d7c352b657c57448d17e295b7d278ed5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 1 May 2026 08:41:45 +0000 Subject: [PATCH 3/4] Add forward-ref alias regression test for trailing-comment RHS Codex flagged a ripper-only path where Constant#is_alias_for_path stays nil for +Foo = Bar # comment+: parse_constant_body's on_const branch only fires create_module_alias when the const token is followed by on_nl/EOF, so the trailing comment short-circuits the recording. Pre-PR this was masked by Constant#find_alias_for's lookup-time regex on +value+, which this PR removes. Skip the actual fix -- the ripper parser is on its way out (loads with a deprecation banner, opt-in via RDOC_USE_RIPPER_PARSER=1) and isn't worth a backstop. Keep the test in the shared parser-mixin (RDocParserPrismTestCases) so prism is still guarded against the same shape, and omit it under the legacy ripper variant. --- test/rdoc/parser/prism_ruby_test.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/rdoc/parser/prism_ruby_test.rb b/test/rdoc/parser/prism_ruby_test.rb index 2cb6ccbce1..44c5808db9 100644 --- a/test/rdoc/parser/prism_ruby_test.rb +++ b/test/rdoc/parser/prism_ruby_test.rb @@ -1715,6 +1715,24 @@ class Bar; end assert_equal 'Foo::Bar', a_const.is_alias_for.full_name end + def test_constant_alias_with_trailing_comment_resolves_after_complete + omit if accept_legacy_bug? # ripper parser is deprecated; gap not worth fixing + util_parser <<~RUBY + class Foo + Trailing = Bar # trailing comment + end + class Bar; end + RUBY + @store.complete(:public) + foo = @store.classes_hash['Foo'] + trailing = foo.constants.find { |c| c.name == 'Trailing' } + refute_nil trailing + refute_nil trailing.is_alias_for, + 'a constant alias whose RHS is followed by a trailing comment ' \ + 'should still resolve to its target after Store#complete' + assert_equal 'Bar', trailing.is_alias_for.full_name + end + def test_repeated_constant_alias util_parser <<~RUBY # Parsed first From 485c8ed2de697045f8179ccc061a348440353f4d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 2 May 2026 06:56:27 +0000 Subject: [PATCH 4/4] Fix two alias-handling issues found in review * update_aliases persisted const.is_alias_for from the lazy resolved_alias_target lookup before the collision guard ran. When a real class already lived at the destination, the alias copy was correctly skipped but the constant was still mismarked, so Stats#report_constants and Constant#marshal_dump observed the alias relationship for an object the guard had just protected. Move the persistence below the guard so skipped aliases also skip the persistence. * In the prism parser, add_constant resolved owner/name from the full constant path but called @container.add_module_alias on the outer lexical container. For Outer::Foo = Bar parsed at top level, this registered the alias copy as classes_hash['Foo'] (using the container's child_name) instead of 'Outer::Foo', leaving a stray top-level entry that update_aliases later duplicated under the correct namespace. Call owner.add_module_alias instead. The ripper parser already handles this correctly because parse_constant reassigns its container to the resolved nested module. Tests: * test_constant_alias_collision_does_not_mismark_constant_as_alias asserts Foo's constant ends up with is_alias_for=nil when a real Foo class already exists alongside Foo = Bar. * test_qualified_constant_alias_registers_in_owner_namespace asserts Outer::Foo = Bar registers only at classes_hash['Outer::Foo'] and does not leak a top-level Foo entry. --- lib/rdoc/code_object/class_module.rb | 13 +++++------ lib/rdoc/parser/prism_ruby.rb | 2 +- test/rdoc/parser/prism_ruby_test.rb | 33 ++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/lib/rdoc/code_object/class_module.rb b/lib/rdoc/code_object/class_module.rb index 9db62aea54..5770d99810 100644 --- a/lib/rdoc/code_object/class_module.rb +++ b/lib/rdoc/code_object/class_module.rb @@ -848,13 +848,7 @@ def type def update_aliases constants.each do |const| 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 + cm ||= const.resolved_alias_target if const.is_a?(RDoc::Constant) next unless cm # Resolve chained aliases (A = B = C) to the real class/module. @@ -882,6 +876,11 @@ def update_aliases existing = @store.find_class_or_module(cm_alias.full_name) next if existing && !existing.is_alias_for + # Persist a lazy-resolved target so Stats#report_constants and + # Constant#marshal_dump observe the alias relationship. Skipped + # aliases (above) intentionally leave the constant unmarked. + const.is_alias_for ||= cm + cm_alias.aliases.clear cm_alias.is_alias_for = cm diff --git a/lib/rdoc/parser/prism_ruby.rb b/lib/rdoc/parser/prism_ruby.rb index c13b612257..28c0edb75a 100644 --- a/lib/rdoc/parser/prism_ruby.rb +++ b/lib/rdoc/parser/prism_ruby.rb @@ -790,7 +790,7 @@ def add_constant(constant_name, rhs_name, start_line, end_line, alias_path: nil) @store.find_class_or_module(full_name) end if mod && constant.document_self - a = @container.add_module_alias(mod, alias_path, constant, @top_level) + a = owner.add_module_alias(mod, alias_path, constant, @top_level) a.store = @store a.line = start_line record_location(a) diff --git a/test/rdoc/parser/prism_ruby_test.rb b/test/rdoc/parser/prism_ruby_test.rb index 44c5808db9..ab021ec664 100644 --- a/test/rdoc/parser/prism_ruby_test.rb +++ b/test/rdoc/parser/prism_ruby_test.rb @@ -1828,6 +1828,39 @@ def shim_method; end assert_empty bar_foo.aliases end + def test_constant_alias_collision_does_not_mismark_constant_as_alias + util_parser <<~RUBY + class Foo + def real_method; end + end + class Bar + def other_method; end + end + Foo = Bar + RUBY + @store.complete(:public) + foo_const = @store.classes_hash['Object'].find_constant_named('Foo') + refute_nil foo_const + assert_nil foo_const.is_alias_for, 'collision-skipped alias must not persist is_alias_for' + end + + def test_qualified_constant_alias_registers_in_owner_namespace + util_parser <<~RUBY + class Bar + def bar_method; end + end + module Outer + end + Outer::Foo = Bar + RUBY + @store.complete(:public) + outer_foo = @store.classes_hash['Outer::Foo'] + refute_nil outer_foo, 'Outer::Foo should be registered' + refute_nil outer_foo.is_alias_for + assert_equal 'Bar', outer_foo.is_alias_for.full_name + assert_nil @store.classes_hash['Foo'], 'qualified alias should not leak into top-level namespace' + end + def test_constant_alias_then_class_reopen_keeps_added_methods util_parser <<~RUBY class Bar