Skip to content

Commit 485c8ed

Browse files
committed
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.
1 parent 1846a8a commit 485c8ed

3 files changed

Lines changed: 40 additions & 8 deletions

File tree

lib/rdoc/code_object/class_module.rb

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -848,13 +848,7 @@ def type
848848
def update_aliases
849849
constants.each do |const|
850850
cm = const.is_alias_for
851-
if !cm && const.is_a?(RDoc::Constant)
852-
cm = const.resolved_alias_target
853-
# Persist the forward-reference resolution on the source constant
854-
# so Stats#report_constants and Constant#marshal_dump observe the
855-
# alias relationship that the lazy lookup found.
856-
const.is_alias_for = cm if cm
857-
end
851+
cm ||= const.resolved_alias_target if const.is_a?(RDoc::Constant)
858852
next unless cm
859853

860854
# Resolve chained aliases (A = B = C) to the real class/module.
@@ -882,6 +876,11 @@ def update_aliases
882876
existing = @store.find_class_or_module(cm_alias.full_name)
883877
next if existing && !existing.is_alias_for
884878

879+
# Persist a lazy-resolved target so Stats#report_constants and
880+
# Constant#marshal_dump observe the alias relationship. Skipped
881+
# aliases (above) intentionally leave the constant unmarked.
882+
const.is_alias_for ||= cm
883+
885884
cm_alias.aliases.clear
886885
cm_alias.is_alias_for = cm
887886

lib/rdoc/parser/prism_ruby.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -790,7 +790,7 @@ def add_constant(constant_name, rhs_name, start_line, end_line, alias_path: nil)
790790
@store.find_class_or_module(full_name)
791791
end
792792
if mod && constant.document_self
793-
a = @container.add_module_alias(mod, alias_path, constant, @top_level)
793+
a = owner.add_module_alias(mod, alias_path, constant, @top_level)
794794
a.store = @store
795795
a.line = start_line
796796
record_location(a)

test/rdoc/parser/prism_ruby_test.rb

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1828,6 +1828,39 @@ def shim_method; end
18281828
assert_empty bar_foo.aliases
18291829
end
18301830

1831+
def test_constant_alias_collision_does_not_mismark_constant_as_alias
1832+
util_parser <<~RUBY
1833+
class Foo
1834+
def real_method; end
1835+
end
1836+
class Bar
1837+
def other_method; end
1838+
end
1839+
Foo = Bar
1840+
RUBY
1841+
@store.complete(:public)
1842+
foo_const = @store.classes_hash['Object'].find_constant_named('Foo')
1843+
refute_nil foo_const
1844+
assert_nil foo_const.is_alias_for, 'collision-skipped alias must not persist is_alias_for'
1845+
end
1846+
1847+
def test_qualified_constant_alias_registers_in_owner_namespace
1848+
util_parser <<~RUBY
1849+
class Bar
1850+
def bar_method; end
1851+
end
1852+
module Outer
1853+
end
1854+
Outer::Foo = Bar
1855+
RUBY
1856+
@store.complete(:public)
1857+
outer_foo = @store.classes_hash['Outer::Foo']
1858+
refute_nil outer_foo, 'Outer::Foo should be registered'
1859+
refute_nil outer_foo.is_alias_for
1860+
assert_equal 'Bar', outer_foo.is_alias_for.full_name
1861+
assert_nil @store.classes_hash['Foo'], 'qualified alias should not leak into top-level namespace'
1862+
end
1863+
18311864
def test_constant_alias_then_class_reopen_keeps_added_methods
18321865
util_parser <<~RUBY
18331866
class Bar

0 commit comments

Comments
 (0)