Store sidecar .rbs signatures separately from method objects

Sidecar .rbs signatures were being merged into MethodAttr#type_signature_lines
during load, which forced the store to keep a @rbs_signature_method_attrs
tracker so reloads could clear the previously-assigned signatures.  Method
objects ended up owning data that didn't come from their source file, and
every reload had to undo that mutation.

Move the signatures to a dedicated @rbs_signatures hash on the store and
look them up at render time:

* Store#merge_rbs_signatures now just stores the hash; the iteration over
  every class and method is gone.
* Store#rbs_signature_for(method_attr) does the lookup, including the
  initialize -> .new singleton mapping and the alias fallthrough to the
  canonical method.
* Store#clear_rbs_signatures becomes a one-liner setting the hash to nil.
* The @rbs_signature_method_attrs tracker is deleted along with
  assign_rbs_signature, and the unused rbs_key helper is gone too.

Read sites use `method.type_signature_lines || store.rbs_signature_for(method)`
so inline `#:` annotations still win over sidecar signatures:

* Generator::Aliki#type_signature_html — falls back to the store lookup.
* aliki/class.rhtml — computes the HTML once via `if (sig_html = ...)`.
* RI::Driver — same `||` fallthrough before rendering.

With the mutation gone, the server's reparse_and_refresh no longer needs
to call load_rbs_signatures when only .rb files change.  Newly-added
methods resolve through the existing string-keyed hash at render time, so
the reload is only required when .rbs files themselves change.  Ordering
between load and Store#complete no longer matters either; reverse the two
in rdoc.rb and server.rb so the flow reads "load all data, then finalize."

Tests in rdoc_store_test.rb assert the new lookup path; two integration
tests (rdoc_rdoc_test, rdoc_server_test) follow suit.  Two new tests pin
the invariants: sidecar load doesn't touch inline #: lines, and lookups
return nil when no signatures are loaded.  The old
test_merge_rbs_signatures_does_not_overwrite_inline_annotations is gone —
it became tautological once merge stopped touching method objects, and
the new test_rbs_signature_for_does_not_overwrite_inline_lookup already
pins the meaningful invariant.

Author: st0012

lib/rdoc/generator/aliki.rb

@@ -122,7 +122,7 @@ def write_search_index
   # Returns nil if no type signature is present.
 
   def type_signature_html(method_attr, from_path)
-    lines = method_attr.type_signature_lines
+    lines = method_attr.type_signature_lines || @store.rbs_signature_for(method_attr)
     return unless lines
 
     RDoc::RbsHelper.signature_to_html(

lib/rdoc/generator/template/aliki/class.rhtml

@@ -93,8 +93,8 @@
             <span class="method-name"><%= h attrib.name %></span>
             <span class="attribute-access-type">[<%= attrib.rw %>]</span>
           </a>
-          <%- if attrib.type_signature_lines %>
-          <span class="method-type-signature"><code><%= type_signature_html(attrib, klass.path) %></code></span>
+          <%- if (sig_html = type_signature_html(attrib, klass.path)) %>
+          <span class="method-type-signature"><code><%= sig_html %></code></span>
           <%- end %>
         </div>
 
@@ -154,8 +154,8 @@
             </div>
           <%- end %>
 
-          <%- if method.type_signature_lines %>
-          <pre class="method-type-signature"><code><%= type_signature_html(method, klass.path) %></code></pre>
+          <%- if (sig_html = type_signature_html(method, klass.path)) %>
+          <pre class="method-type-signature"><code><%= sig_html %></code></pre>
           <%- end %>
         </div>
 

lib/rdoc/rdoc.rb

@@ -474,8 +474,8 @@ def document(options)
 
       @options.default_title = "RDoc Documentation"
 
-      @store.complete @options.visibility
       load_rbs_signatures
+      @store.complete @options.visibility
 
       start_server
       exit
@@ -490,20 +490,22 @@ def document(options)
     @store.load_cache
 
     rbs_signatures_changed = rbs_signatures_changed?
-    # When only sig/*.rbs changed, no Ruby file would be reparsed and the
-    # cached HTML pipeline keeps no in-memory class data across runs, so
-    # force a full reparse to give merge_rbs_signatures a populated store.
+    # When only sig/*.rbs changed, no Ruby file would be reparsed under
+    # normal mtime checks.  The store cache holds class metadata but not
+    # live RDoc::Context objects, so the generator would have nothing to
+    # iterate.  Force a full reparse so updated signatures show up in
+    # the rendered output.
     @last_modified.clear if rbs_signatures_changed
 
     file_info = parse_files @options.files
     record_rbs_signature_mtimes
 
     @options.default_title = "RDoc Documentation"
 
-    @store.complete @options.visibility
-
     load_rbs_signatures
 
+    @store.complete @options.visibility
+
     @stats.coverage_level = @options.coverage_report
 
     if @options.coverage_report then

lib/rdoc/ri/driver.rb

@@ -1415,7 +1415,8 @@ def render_method(out, store, method, name) # :nodoc:
     out << RDoc::Markup::Rule.new(1)
 
     render_method_arguments out, method.arglists
-    render_method_type_signature out, method.type_signature_lines if method.type_signature_lines
+    sig = method.type_signature_lines || store.rbs_signature_for(method)
+    render_method_type_signature out, sig if sig
     render_method_superclass out, method
     if method.is_alias_for
       al = method.is_alias_for

lib/rdoc/server.rb

@@ -408,20 +408,20 @@ def reparse_and_refresh(changed_files, removed_files, rbs_changed: false)
         $stderr.puts "Re-parsed #{changed_file_names.join(', ')} (#{duration_ms}ms)"
       end
 
-      @store.complete(@options.visibility)
-      @store.invalidate_type_name_lookup unless changed_files.empty? && removed_files.empty?
-
-      if rbs_changed || !changed_files.empty?
+      if rbs_changed
         duration_ms = measure do
           @rdoc.load_rbs_signatures
           @rdoc.record_rbs_signature_mtimes
           @rdoc.rbs_signature_files.each do |file|
             @file_mtimes[file] = RDoc.safe_mtime(file)
           end
         end
-        $stderr.puts "Reloaded RBS signatures (#{duration_ms}ms)" if rbs_changed
+        $stderr.puts "Reloaded RBS signatures (#{duration_ms}ms)"
       end
 
+      @store.complete(@options.visibility)
+      @store.invalidate_type_name_lookup unless changed_files.empty? && removed_files.empty?
+
       @generator.refresh_store_data
       @page_cache.clear
       @last_change_time = Time.now.to_f

lib/rdoc/store.rb

@@ -381,61 +381,46 @@ def type_name_lookup
   end
 
   ##
-  # Merges RBS type signatures into code objects.
-  # Inline #: annotations take priority and are not overwritten.
+  # Stores RBS type signatures loaded from sidecar .rbs files, keyed by
+  # "ClassName#method" or "ClassName.method".  Replaces any previously
+  # stored set, so passing +{}+ clears it.  Inline +#:+ annotations on
+  # method objects are NOT touched — those are owned by the source file.
 
   def merge_rbs_signatures(signatures)
-    clear_rbs_signatures
-
-    all_classes_and_modules.each do |cm|
-      cm.method_list.each do |method|
-        next if method.type_signature_lines
-
-        sig = signatures[rbs_key(cm, method)]
-
-        # RBS keys constructors as #initialize, but RDoc renames them to .new
-        if !sig && method.name == 'new' && method.singleton
-          sig = signatures["#{cm.full_name}#initialize"]
-        end
-
-        assign_rbs_signature method, sig if sig
-      end
-
-      cm.attributes.each do |attr|
-        next if attr.type_signature_lines
-
-        if (sig = signatures[rbs_key(cm, attr)])
-          assign_rbs_signature attr, sig
-        end
-      end
-    end
+    @rbs_signatures = signatures
   end
 
-  def assign_rbs_signature(method_attr, signature) # :nodoc:
-    @rbs_signature_method_attrs ||= []
+  ##
+  # Returns the RBS type signature lines for +method_attr+ from loaded
+  # sidecar +.rbs+ files, or +nil+ if none.  Falls through to the
+  # canonical method for aliases, and handles +initialize+ -> +.new+
+  # singleton mapping.
 
-    method_attr.type_signature_lines = signature
-    @rbs_signature_method_attrs << method_attr
+  def rbs_signature_for(method_attr)
+    return nil unless @rbs_signatures
+    cm = method_attr.parent
+    return nil unless cm.respond_to?(:full_name)
 
-    method_attr.aliases.each do |aliased|
-      next if aliased.type_signature_lines
-      aliased.type_signature_lines = signature
-      @rbs_signature_method_attrs << aliased
-    end
-  end
+    key = method_attr.singleton ? "#{cm.full_name}.#{method_attr.name}" : "#{cm.full_name}##{method_attr.name}"
+    sig = @rbs_signatures[key]
 
-  def clear_rbs_signatures # :nodoc:
-    return unless @rbs_signature_method_attrs
+    # RBS keys constructors as #initialize, but RDoc renames them to .new
+    if !sig && method_attr.name == 'new' && method_attr.singleton
+      sig = @rbs_signatures["#{cm.full_name}#initialize"]
+    end
 
-    @rbs_signature_method_attrs.each do |method_attr|
-      method_attr.type_signature_lines = nil
+    # For aliases, fall through to the canonical method (its inline #:
+    # takes precedence over any sidecar signature on the alias's name).
+    if !sig && method_attr.is_alias_for
+      canonical = method_attr.is_alias_for
+      sig = canonical.type_signature_lines || rbs_signature_for(canonical)
     end
 
-    @rbs_signature_method_attrs.clear
+    sig
   end
 
-  def rbs_key(cm, method_attr) # :nodoc:
-    method_attr.singleton ? "#{cm.full_name}.#{method_attr.name}" : "#{cm.full_name}##{method_attr.name}"
+  def clear_rbs_signatures # :nodoc:
+    @rbs_signatures = nil
   end
 
   ##

test/rdoc/rdoc_rdoc_test.rb

@@ -106,7 +106,7 @@ def greet: () -> String
 
       example = store.find_class_or_module 'Example'
       greet = example.find_method 'greet', false
-      assert_equal ['() -> String'], greet.type_signature_lines
+      assert_equal ['() -> String'], store.rbs_signature_for(greet)
     end
   end
 
@@ -132,7 +132,7 @@ def greet: () -> String
       example.add_method method
 
       @rdoc.load_rbs_signatures
-      assert_equal ['() -> String'], method.type_signature_lines
+      assert_equal ['() -> String'], @rdoc.store.rbs_signature_for(method)
 
       File.write sig, "class Example\n  def greet: ( -> "
       @options.verbosity = 2
@@ -141,7 +141,7 @@ def greet: () -> String
       end
 
       assert_includes err, 'Failed to load RBS type signatures'
-      assert_nil method.type_signature_lines
+      assert_nil @rdoc.store.rbs_signature_for(method)
     end
   end
 

test/rdoc/rdoc_server_test.rb

@@ -99,6 +99,6 @@ def greet: () -> String
 
     example = @rdoc.store.find_class_or_module 'Example'
     greet = example.find_method 'greet', false
-    assert_equal ['() -> String'], greet.type_signature_lines
+    assert_equal ['() -> String'], @rdoc.store.rbs_signature_for(greet)
   end
 end

test/rdoc/rdoc_store_test.rb

@@ -1302,16 +1302,19 @@ def test_merge_rbs_signatures
       'Object#language' => ['String']
     )
 
-    assert_equal ['(String name) -> void'], m.type_signature_lines
-    assert_equal ['String'], a.type_signature_lines
+    assert_equal ['(String name) -> void'], @s.rbs_signature_for(m)
+    assert_equal ['String'], @s.rbs_signature_for(a)
+    # Inline #: lines on the method are untouched
+    assert_nil m.type_signature_lines
+    assert_nil a.type_signature_lines
   end
 
   def test_merge_rbs_signatures_singleton_method
     @s.merge_rbs_signatures(
       'Object.cmethod' => ['() -> String']
     )
 
-    assert_equal ['() -> String'], @cmeth.type_signature_lines
+    assert_equal ['() -> String'], @s.rbs_signature_for(@cmeth)
   end
 
   def test_merge_rbs_signatures_constructor
@@ -1323,22 +1326,22 @@ def test_merge_rbs_signatures_constructor
       'Object#initialize' => ['(String name) -> void']
     )
 
-    assert_equal ['(String name) -> void'], ctor.type_signature_lines
+    assert_equal ['(String name) -> void'], @s.rbs_signature_for(ctor)
   end
 
   def test_merge_rbs_signatures_clears_signatures_removed_in_subsequent_merge
     @s.merge_rbs_signatures(
       'Object#method' => ['() -> String']
     )
-    assert_equal ['() -> String'], @meth.type_signature_lines
+    assert_equal ['() -> String'], @s.rbs_signature_for(@meth)
 
     # Subsequent merge no longer mentions the key — the signature must be
     # cleared rather than left stale, so live-reload reflects deletions.
     @s.merge_rbs_signatures({})
-    assert_nil @meth.type_signature_lines
+    assert_nil @s.rbs_signature_for(@meth)
   end
 
-  def test_merge_rbs_signatures_propagates_to_method_alias
+  def test_rbs_signature_for_propagates_to_method_alias
     original = RDoc::AnyMethod.new nil, 'original'
     original.record_location @top_level
     @klass.add_method original
@@ -1351,11 +1354,11 @@ def test_merge_rbs_signatures_propagates_to_method_alias
       'Object#original' => ['() -> String']
     )
 
-    assert_equal ['() -> String'], original.type_signature_lines
-    assert_equal ['() -> String'], aliased.type_signature_lines
+    assert_equal ['() -> String'], @s.rbs_signature_for(original)
+    assert_equal ['() -> String'], @s.rbs_signature_for(aliased)
   end
 
-  def test_merge_rbs_signatures_propagates_to_attribute_alias
+  def test_rbs_signature_for_propagates_to_attribute_alias
     original = RDoc::Attr.new nil, 'language', 'R', ''
     original.record_location @top_level
     @klass.add_attribute original
@@ -1368,8 +1371,8 @@ def test_merge_rbs_signatures_propagates_to_attribute_alias
       'Object#language' => ['String']
     )
 
-    assert_equal ['String'], original.type_signature_lines
-    assert_equal ['String'], aliased.type_signature_lines
+    assert_equal ['String'], @s.rbs_signature_for(original)
+    assert_equal ['String'], @s.rbs_signature_for(aliased)
   end
 
   def test_merge_rbs_signatures_keeps_instance_and_singleton_attributes_separate
@@ -1386,8 +1389,24 @@ def test_merge_rbs_signatures_keeps_instance_and_singleton_attributes_separate
       'Object.language' => ['Integer']
     )
 
-    assert_equal ['String'], instance_attr.type_signature_lines
-    assert_equal ['Integer'], singleton_attr.type_signature_lines
+    assert_equal ['String'], @s.rbs_signature_for(instance_attr)
+    assert_equal ['Integer'], @s.rbs_signature_for(singleton_attr)
+  end
+
+  def test_rbs_signature_for_returns_nil_when_no_signatures_loaded
+    assert_nil @s.rbs_signature_for(@meth)
+  end
+
+  def test_rbs_signature_for_does_not_overwrite_inline_lookup
+    # Inline #: lives on the method itself; sidecar lookup is separate.
+    @meth.type_signature_lines = ['() -> String  # inline']
+    @s.merge_rbs_signatures(
+      'Object#method' => ['() -> String  # sidecar']
+    )
+
+    # Both sources are readable; the consumer chooses precedence (inline wins).
+    assert_equal ['() -> String  # inline'], @meth.type_signature_lines
+    assert_equal ['() -> String  # sidecar'], @s.rbs_signature_for(@meth)
   end
 
   def test_type_name_lookup
@@ -1434,17 +1453,4 @@ def test_type_name_lookup_top_level_class_wins_over_nested_namesake
   end
 
 
-  def test_merge_rbs_signatures_does_not_overwrite_inline_annotations
-    m = RDoc::AnyMethod.new(nil, 'greet')
-    m.params = '(name)'
-    m.type_signature_lines = ['(String) -> void']
-    @klass.add_method m
-
-    @s.merge_rbs_signatures(
-      'Object#greet' => ['(String name, ?Integer count) -> void']
-    )
-
-    assert_equal ['(String) -> void'], m.type_signature_lines
-  end
-
 end