Skip to content

Commit df14870

Browse files
tompngclaude
andauthored
Rename #initialize to ::new before registering it to the container (#1802)
## Background The Ruby parser renames an instance method `initialize` to `::new` for documentation purposes. This rename happened *after* `container.add_method`, with a comment claiming the ordering is intentional: "Rename after add_method to register duplicated 'new' and 'initialize' defined in c and ruby". The actual reason for this placement is older. In the Ripper-based streaming parser, documentation modifiers such as `:notnew:` were read *after* the method line, so at `add_method` time the parser simply did not know yet whether the method should be renamed: ```ruby # Having now read the method parameters and documentation modifiers, we # now know whether we have to rename #initialize to ::new ``` (lib/rdoc/parser/ripper_ruby.rb, removed in #1690) The Prism parser processes directives and modifier lines before `add_method`, so this constraint is gone. The post-add placement was a consequence of the streaming parser's information ordering, not a design goal — which is why it is safe to retire the post-add mutation pattern now. The comment in the current code was a port-time rationalization of an observable side effect. ## What the old placement actually did Registering the method under the `#initialize` key and renaming it afterwards had two effects: 1. `Context#methods_hash` was left keyed by a stale name (`#initialize` pointing at a method now called `::new`). This is one of the obstacles to making `Context#find_method` hash-based (see the discussion in #1796). 2. Duplicate detection in `Context#add_method` was bypassed. A class documenting both `::new` (an explicit `def self.new`, or a C-defined `new`) and `#initialize` ended up with two `::new` entries on its page. ## Change Move the rename block before `container.add_method`. All information it uses (the method name, `singleton`, and `dont_rename_initialize` set by the `:notnew:` directive) is already available at that point. With the rename in place before registration, the normal deduplication applies: the first registration wins, and the duplicate is reported by the existing "Duplicate method" warning (visible with `--verbose`). ## Corpus diff Compared per-class method lists (name, singleton, visibility, file) for whole corpora, before vs after: | Corpus | Changes | | --- | --- | | ruby/ruby | 4 classes lose a duplicated `::new` entry: `Gem::Package::TarReader`, `Gem::Package::TarWriter`, `Gem::Resolver::APISpecification`, `JSON::Ext::Generator::State` | | activesupport 8.1.3 | 1 class: `ActiveSupport::Deprecation::DeprecatedConstantProxy` | | rdoc itself | no change | All other entries are identical. Each changed class defines both `def self.new` and `def initialize` (or, for `JSON::Ext::Generator::State`, a C-defined `new` and a Ruby `initialize` — the exact "c and ruby" case the old comment referred to). On master these pages show `new` twice, with a duplicated `id="method-c-new"` anchor (invalid HTML; the index can only link to the first entry); with this change they show it once. ## Cleanup The second commit removes the `dont_rename_initialize` keyword argument of `internal_add_method`. It has never been passed a truthy value since its introduction: one call site passes an explicit `false` and the other relies on the `false` default. It is unrelated to `AnyMethod#dont_rename_initialize` (set by the `:notnew:` directive), which remains the live mechanism; removing the constant-false parameter also removes the confusion of two same-named flags in one method. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent f4e3c3b commit df14870

2 files changed

Lines changed: 36 additions & 11 deletions

File tree

‎lib/rdoc/parser/ruby.rb‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,6 @@ def handle_meta_method_comment(comment, directives, node)
451451
@container,
452452
comment: comment,
453453
directives: directives,
454-
dont_rename_initialize: false,
455454
line_no: line_no,
456455
visibility: visibility,
457456
singleton: @singleton || singleton_method,
@@ -722,7 +721,7 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:
722721
)
723722
end
724723

725-
private def internal_add_method(method_name, container, comment:, dont_rename_initialize: false, directives:, modifier_comment_lines: nil, line_no:, visibility:, singleton:, params:, calls_super:, block_params:, tokens:, type_signature_lines: nil) # :nodoc:
724+
private def internal_add_method(method_name, container, comment:, directives:, modifier_comment_lines: nil, line_no:, visibility:, singleton:, params:, calls_super:, block_params:, tokens:, type_signature_lines: nil) # :nodoc:
726725
meth = RDoc::AnyMethod.new(method_name, singleton: singleton)
727726
meth.comment = comment
728727
handle_code_object_directives(meth, directives) if directives
@@ -746,16 +745,10 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:
746745
meth.calls_super = calls_super
747746
meth.block_params ||= block_params if block_params
748747
meth.type_signature_lines = type_signature_lines
749-
container.add_method(meth)
750-
record_location(meth)
751-
meth.start_collecting_tokens(:ruby)
752-
tokens.each do |token|
753-
meth.token_stream << token
754-
end
755748

756-
# Rename after add_method to register duplicated 'new' and 'initialize'
757-
# defined in c and ruby.
758-
if !dont_rename_initialize && method_name == 'initialize' && !singleton
749+
# An instance method `initialize` is documented as `::new` unless the
750+
# :notnew: directive is given
751+
if method_name == 'initialize' && !singleton
759752
if meth.dont_rename_initialize
760753
meth.visibility = :protected
761754
else
@@ -764,6 +757,13 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:
764757
meth.visibility = :public
765758
end
766759
end
760+
761+
record_location(meth)
762+
container.add_method(meth)
763+
meth.start_collecting_tokens(:ruby)
764+
tokens.each do |token|
765+
meth.token_stream << token
766+
end
767767
end
768768

769769
# Find or create module or class from a given module name using Ruby lexical

‎test/rdoc/parser/ruby_test.rb‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,31 @@ def initialize(*args)
700700
assert_equal expected, arglists
701701
end
702702

703+
def test_class_new_and_initialize_are_registered_once
704+
util_parser <<~RUBY
705+
class A
706+
# new doc
707+
def self.new(x); super; end
708+
# initialize doc
709+
def initialize(x); end
710+
end
711+
712+
class B
713+
# initialize doc
714+
def initialize(x); end
715+
# new doc
716+
def self.new(x); super; end
717+
end
718+
RUBY
719+
720+
a, b = @top_level.classes
721+
assert_equal ['A::new'], a.method_list.map(&:full_name)
722+
assert_equal 'new doc', a.method_list.first.comment.text
723+
assert_equal ['::new'], a.methods_hash.keys
724+
assert_equal ['B::new'], b.method_list.map(&:full_name)
725+
assert_equal 'initialize doc', b.method_list.first.comment.text
726+
end
727+
703728
def test_class_mistaken_for_module
704729
util_parser <<~RUBY
705730
class A::Foo; end

0 commit comments

Comments
 (0)