Skip to content

Commit 6f98be5

Browse files
committed
Replace alias-path regex with parser-supplied AST info
The lazy resolver previously inspected RDoc::Constant#value with a regex to guess whether the RHS looked like a constant path. Both parsers already know this from the AST/token stream: * The Prism parser distinguishes ConstantReadNode/ConstantPathNode values via constant_path_string in lib/rdoc/parser/prism_ruby.rb. * The legacy ripper parser only builds rhs_name from :on_const and `::` tokens in lib/rdoc/parser/ripper_ruby.rb, so reaching create_module_alias already implies the RHS is a constant path. Track that explicit knowledge on the constant via a new RDoc::Constant#is_alias_for_path attribute and drop the regex from #resolved_alias_target. The lazy resolver now activates only when the parser actually saw a constant reference, which is more accurate than the regex (no false positives on string-shaped values that happen to match) and removes a one-off pattern that was duplicated work. Tests: * Each parser-level test that asserts a real class is preserved now also asserts that the would-be alias target (Bar, Bar::Foo) keeps its own methods and doesn't end up in someone else's #aliases. * The unit-level update_aliases tests assert the same: the would-be target keeps its method list and aliases stay empty. These positive assertions guard against a future regression where the real class survives but the alias target is somehow corrupted or has its aliases list mutated.
1 parent 434d2ef commit 6f98be5

5 files changed

Lines changed: 66 additions & 18 deletions

File tree

lib/rdoc/code_object/constant.rb

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,16 @@ class RDoc::Constant < RDoc::CodeObject
2626

2727
attr_accessor :visibility
2828

29+
##
30+
# The constant path the RHS names, when the RHS is a bare constant
31+
# reference (e.g. +Foo = Bar+ or +Foo = Bar::Baz+). Set by the parser at
32+
# parse time, when AST/token info is available, so the lazy resolver does
33+
# not have to re-examine the textual #value to decide whether it could be a
34+
# class/module reference. nil for any other RHS shape (literals, calls,
35+
# expressions).
36+
37+
attr_accessor :is_alias_for_path
38+
2939
##
3040
# Creates a new constant with +name+, +value+ and +comment+
3141

@@ -35,8 +45,9 @@ def initialize(name, value, comment)
3545
@name = name
3646
@value = value
3747

38-
@is_alias_for = nil
39-
@visibility = :public
48+
@is_alias_for = nil
49+
@is_alias_for_path = nil
50+
@visibility = :public
4051

4152
self.comment = comment
4253
end
@@ -102,16 +113,17 @@ def is_alias_for
102113
end
103114

104115
##
105-
# Returns the class/module this constant *would* alias if its #value names
106-
# one in the current store, or nil. Used to support `Const = RHS` parsed
107-
# before `class RHS;end` is defined in another file. Pure lookup; does not
108-
# mutate state. Honors :nodoc: (returns nil if document_self is false). Note
109-
# that module nesting information is lost, so constant lookup is inaccurate.
116+
# Returns the class/module this constant *would* alias if #is_alias_for_path
117+
# was set by the parser and that path resolves to a known class/module, or
118+
# nil. Used to support `Const = RHS` parsed before `class RHS;end` is defined
119+
# in another file. Pure lookup; does not mutate state. Honors :nodoc:
120+
# (returns nil if document_self is false). Note that module nesting
121+
# information is lost, so constant lookup is inaccurate.
110122

111123
def resolved_alias_target
112124
return nil unless document_self
113-
return nil unless value&.match?(/\A(::)?([A-Z][A-Za-z0-9_]*)(::[A-Z][A-Za-z0-9_]*)*\z/)
114-
parent.find_module_named(value)
125+
return nil unless @is_alias_for_path
126+
parent.find_module_named(@is_alias_for_path)
115127
end
116128

117129
def inspect # :nodoc:

lib/rdoc/parser/prism_ruby.rb

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ def find_or_create_constant_owner_name(constant_path)
767767

768768
# Adds a constant
769769

770-
def add_constant(constant_name, rhs_name, start_line, end_line)
770+
def add_constant(constant_name, rhs_name, start_line, end_line, alias_path: nil)
771771
comment, directives = consecutive_comment(start_line)
772772
handle_code_object_directives(@container, directives) if directives
773773
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)
776776
constant = RDoc::Constant.new(name, rhs_name, comment)
777777
constant.store = @store
778778
constant.line = start_line
779+
constant.is_alias_for_path = alias_path
779780
record_location(constant)
780781
handle_modifier_directive(constant, start_line)
781782
handle_modifier_directive(constant, end_line)
782783
owner.add_constant(constant)
784+
return unless alias_path
783785
mod =
784-
if rhs_name =~ /^::/
785-
@store.find_class_or_module(rhs_name)
786+
if alias_path.start_with?('::')
787+
@store.find_class_or_module(alias_path)
786788
else
787-
full_name = resolve_constant_path(rhs_name)
789+
full_name = resolve_constant_path(alias_path)
788790
@store.find_class_or_module(full_name)
789791
end
790792
if mod && constant.document_self
791-
a = @container.add_module_alias(mod, rhs_name, constant, @top_level)
793+
a = @container.add_module_alias(mod, alias_path, constant, @top_level)
792794
a.store = @store
793795
a.line = start_line
794796
record_location(a)
@@ -1056,23 +1058,27 @@ def visit_constant_path_write_node(node)
10561058
path = constant_path_string(node.target)
10571059
return unless path
10581060

1061+
alias_path = constant_path_string(node.value)
10591062
@scanner.add_constant(
10601063
path,
1061-
constant_path_string(node.value) || node.value.slice,
1064+
alias_path || node.value.slice,
10621065
node.location.start_line,
1063-
node.location.end_line
1066+
node.location.end_line,
1067+
alias_path: alias_path
10641068
)
10651069
@scanner.skip_comments_until(node.location.end_line)
10661070
# Do not traverse rhs not to document `A::B = Struct.new{def undocumentable_method; end}`
10671071
end
10681072

10691073
def visit_constant_write_node(node)
10701074
@scanner.process_comments_until(node.location.start_line - 1)
1075+
alias_path = constant_path_string(node.value)
10711076
@scanner.add_constant(
10721077
node.name.to_s,
1073-
constant_path_string(node.value) || node.value.slice,
1078+
alias_path || node.value.slice,
10741079
node.location.start_line,
1075-
node.location.end_line
1080+
node.location.end_line,
1081+
alias_path: alias_path
10761082
)
10771083
@scanner.skip_comments_until(node.location.end_line)
10781084
# Do not traverse rhs not to document `A = Struct.new{def undocumentable_method; end}`

lib/rdoc/parser/ripper_ruby.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ def create_attr(container, single, name, rw, comment) # :nodoc:
170170
# for "::") with the name from +constant+.
171171

172172
def create_module_alias(container, constant, rhs_name) # :nodoc:
173+
constant.is_alias_for_path = rhs_name
173174
mod = if rhs_name =~ /^::/ then
174175
@store.find_class_or_module rhs_name
175176
else

test/rdoc/code_object/class_module_test.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,8 +1468,10 @@ def test_update_aliases_does_not_overwrite_existing_class_with_same_name
14681468
real_foo.add_method RDoc::AnyMethod.new(nil, 'real_method')
14691469

14701470
other = top_level.add_class RDoc::NormalClass, 'Other'
1471+
other.add_method RDoc::AnyMethod.new(nil, 'other_method')
14711472

14721473
const = RDoc::Constant.new 'Foo', 'Other', ''
1474+
const.is_alias_for_path = 'Other'
14731475
const.record_location top_level
14741476
object.add_constant const
14751477

@@ -1478,6 +1480,9 @@ def test_update_aliases_does_not_overwrite_existing_class_with_same_name
14781480
assert_same real_foo, store.classes_hash['Foo']
14791481
assert_nil store.classes_hash['Foo'].is_alias_for
14801482
assert_equal ['real_method'], store.classes_hash['Foo'].method_list.map(&:name)
1483+
assert_same other, store.classes_hash['Other']
1484+
assert_nil other.is_alias_for
1485+
assert_equal ['other_method'], other.method_list.map(&:name)
14811486
assert_empty other.aliases
14821487
end
14831488

@@ -1487,8 +1492,10 @@ def test_update_aliases_skips_nodoc_constant
14871492

14881493
object = top_level.add_class RDoc::NormalClass, 'Object'
14891494
target = top_level.add_class RDoc::NormalClass, 'Target'
1495+
target.add_method RDoc::AnyMethod.new(nil, 'target_method')
14901496

14911497
const = RDoc::Constant.new 'NodocAlias', 'Target', ''
1498+
const.is_alias_for_path = 'Target'
14921499
const.record_location top_level
14931500
const.document_self = nil
14941501
object.add_constant const
@@ -1497,6 +1504,8 @@ def test_update_aliases_skips_nodoc_constant
14971504

14981505
assert_nil store.classes_hash['NodocAlias']
14991506
assert_nil store.modules_hash['NodocAlias']
1507+
assert_same target, store.classes_hash['Target']
1508+
assert_equal ['target_method'], target.method_list.map(&:name)
15001509
assert_empty target.aliases
15011510
end
15021511

test/rdoc/parser/prism_ruby_test.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1721,13 +1721,18 @@ def test_nodoc_constant_assignment_does_not_become_alias
17211721
util_parser <<~RUBY
17221722
class Outer
17231723
class Bar
1724+
def bar_method; end
17241725
end
17251726
Foo = Bar # :nodoc:
17261727
end
17271728
RUBY
17281729
@store.complete(:public)
17291730
outer = @store.classes_hash['Outer']
17301731
refute_nil outer
1732+
bar = outer.classes_hash['Bar']
1733+
refute_nil bar
1734+
assert_includes bar.method_list.map(&:name), 'bar_method'
1735+
assert_empty bar.aliases
17311736
assert_nil outer.classes_hash['Foo']
17321737
assert_nil outer.modules_hash['Foo']
17331738
foo_const = outer.constants.find { |c| c.name == 'Foo' }
@@ -1752,6 +1757,11 @@ def other_method; end
17521757
assert_nil foo.is_alias_for
17531758
assert_includes foo.method_list.map(&:name), 'real_method'
17541759
refute_includes foo.method_list.map(&:name), 'other_method'
1760+
bar = @store.classes_hash['Bar']
1761+
refute_nil bar
1762+
assert_nil bar.is_alias_for
1763+
assert_includes bar.method_list.map(&:name), 'other_method'
1764+
assert_empty bar.aliases
17551765
end
17561766

17571767
def test_nodoc_constant_assignment_preserves_real_class_with_same_name
@@ -1772,6 +1782,11 @@ def shim_method; end
17721782
assert_nil foo.is_alias_for
17731783
assert_includes foo.method_list.map(&:name), 'real_method'
17741784
refute_includes foo.method_list.map(&:name), 'shim_method'
1785+
bar_foo = @store.classes_hash['Bar::Foo']
1786+
refute_nil bar_foo
1787+
assert_nil bar_foo.is_alias_for
1788+
assert_includes bar_foo.method_list.map(&:name), 'shim_method'
1789+
assert_empty bar_foo.aliases
17751790
end
17761791

17771792
def test_stopdoc_constant_assignment_preserves_real_class_with_same_name
@@ -1794,6 +1809,11 @@ def shim_method; end
17941809
assert_nil foo.is_alias_for
17951810
assert_includes foo.method_list.map(&:name), 'real_method'
17961811
refute_includes foo.method_list.map(&:name), 'shim_method'
1812+
bar_foo = @store.classes_hash['Bar::Foo']
1813+
refute_nil bar_foo
1814+
assert_nil bar_foo.is_alias_for
1815+
assert_includes bar_foo.method_list.map(&:name), 'shim_method'
1816+
assert_empty bar_foo.aliases
17971817
end
17981818

17991819
def test_constant_with_singleton_class

0 commit comments

Comments
 (0)