Refactor comment_location from Array to Hash

lib/rdoc/code_object/class_module.rb

@@ -30,22 +30,20 @@ class RDoc::ClassModule < RDoc::Context
   attr_accessor :constant_aliases
 
   ##
-  # An array of `[comment, location]` pairs documenting this class/module.
+  # A hash of <tt>{ location => [comments] }</tt> documenting this class/module.
   # Use #add_comment to add comments.
   #
+  # Ruby hashes maintain insertion order, so comments render in the order
+  # they were first added. Each location maps to an array of comments,
+  # allowing a class reopened in the same file to accumulate multiple comments.
+  #
   # Before marshalling:
-  # - +comment+ is a String
   # - +location+ is an RDoc::TopLevel
+  # - +comments+ are Strings
   #
   # After unmarshalling:
-  # - +comment+ is an RDoc::Markup::Document
   # - +location+ is a filename String
-  #
-  # These type changes are acceptable (for now) because:
-  # - +comment+: Both String and Document respond to #empty?, and #parse
-  #   returns Document as-is (see RDoc::Text#parse)
-  # - +location+: Only used by #parse to set Document#file, which accepts
-  #   both TopLevel (extracts relative_name) and String
+  # - +comments+ are RDoc::Markup::Documents
 
   attr_accessor :comment_location
 
@@ -63,8 +61,8 @@ class RDoc::ClassModule < RDoc::Context
   def self.from_module(class_type, mod)
     klass = class_type.new mod.name
 
-    mod.comment_location.each do |comment, location|
-      klass.add_comment comment, location
+    mod.comment_location.each do |location, comments|
+      comments.each { |comment| klass.add_comment comment, location }
     end
 
     klass.parent = mod.parent
@@ -125,7 +123,7 @@ def initialize(name, superclass = nil)
     @is_alias_for     = nil
     @name             = name
     @superclass       = superclass
-    @comment_location = [] # Array of [comment, location] pairs
+    @comment_location = {} # Hash of { location => [comments] }
 
     super()
   end
@@ -147,11 +145,7 @@ def add_comment(comment, location)
                 normalize_comment comment
               end
 
-    if location.parser == RDoc::Parser::C
-      @comment_location.delete_if { |(_, l)| l == location }
-    end
-
-    @comment_location << [comment, location]
+    (@comment_location[location] ||= []) << comment
 
     self.comment = original
   end
@@ -270,7 +264,7 @@ def document_self_or_methods
   def documented?
     return true if @received_nodoc
     return false if @comment_location.empty?
-    @comment_location.any? { |comment, _| not comment.empty? }
+    @comment_location.each_value.any? { |comments| comments.any? { |c| not c.empty? } }
   end
 
   ##
@@ -411,9 +405,9 @@ def marshal_load(array) # :nodoc:
     @comment    = RDoc::Comment.from_document document
 
     @comment_location = if document.parts.first.is_a?(RDoc::Markup::Document)
-                          document.parts.map { |doc| [doc, doc.file] }
+                          document.parts.group_by(&:file)
                         else
-                          [[document, document.file]]
+                          { document.file => [document] }
                         end
 
     array[5].each do |name, rw, visibility, singleton, file|
@@ -495,9 +489,9 @@ def merge(class_module)
       @comment = RDoc::Comment.from_document(document)
 
       @comment_location = if document.parts.first.is_a?(RDoc::Markup::Document)
-                            document.parts.map { |doc| [doc, doc.file] }
+                            document.parts.group_by(&:file)
                           else
-                            [[document, document.file]]
+                            { document.file => [document] }
                           end
     end
 
@@ -643,11 +637,13 @@ def parse(comment_location)
     case comment_location
     when String then
       super
-    when Array then
-      docs = comment_location.map do |comment, location|
-        doc = super comment
-        doc.file = location
-        doc
+    when Hash then
+      docs = comment_location.flat_map do |location, comments|
+        comments.map do |comment|
+          doc = super comment
+          doc.file = location
+          doc
+        end
       end
 
       RDoc::Markup::Document.new(*docs)
@@ -745,7 +741,7 @@ def search_record
   # Returns an HTML snippet of the first comment for search results.
 
   def search_snippet
-    first_comment = @comment_location.first&.first
+    first_comment = @comment_location.each_value.first&.first
     return '' unless first_comment && !first_comment.empty?
 
     snippet(first_comment)

lib/rdoc/i18n/text.rb

@@ -89,9 +89,9 @@ def each_line(raw, &block)
     case raw
     when RDoc::Comment
       raw.text.each_line(&block)
-    when Array
-      raw.each do |comment, location|
-        each_line(comment, &block)
+    when Hash
+      raw.each_value do |comments|
+        comments.each { |comment| each_line(comment, &block) }
       end
     else
       raw.each_line(&block)

test/rdoc/code_object/class_module_test.rb

@@ -12,21 +12,21 @@ def test_add_comment
     comment_tl1 = RDoc::Comment.new('# comment 1', @top_level, :ruby)
     cm.add_comment comment_tl1, tl1
 
-    assert_equal [[comment_tl1, tl1]], cm.comment_location
+    assert_equal({ tl1 => [comment_tl1] }, cm.comment_location)
     assert_equal 'comment 1', cm.comment.text
 
     comment_tl2 = RDoc::Comment.new('# comment 2', @top_level, :ruby)
     cm.add_comment comment_tl2, tl2
 
-    assert_equal [[comment_tl1, tl1], [comment_tl2, tl2]], cm.comment_location
+    assert_equal({ tl1 => [comment_tl1], tl2 => [comment_tl2] }, cm.comment_location)
     assert_equal "comment 1\n---\ncomment 2", cm.comment
 
     comment_tl3 = RDoc::Comment.new('# * comment 3', @top_level, :ruby)
     cm.add_comment comment_tl3, tl3
 
-    assert_equal [[comment_tl1, tl1],
-                  [comment_tl2, tl2],
-                  [comment_tl3, tl3]], cm.comment_location
+    assert_equal({ tl1 => [comment_tl1],
+                   tl2 => [comment_tl2],
+                   tl3 => [comment_tl3] }, cm.comment_location)
     assert_equal "comment 1\n---\ncomment 2\n---\n* comment 3", cm.comment
   end
 
@@ -38,7 +38,7 @@ def test_add_comment_comment
     assert_equal 'comment', cm.comment.text
   end
 
-  def test_add_comment_duplicate
+  def test_add_comment_same_file_reopened_class
     tl1 = @store.add_file 'one.rb'
 
     cm = RDoc::ClassModule.new 'Klass'
@@ -47,8 +47,8 @@ def test_add_comment_duplicate
     cm.add_comment comment1, tl1
     cm.add_comment comment2, tl1
 
-    assert_equal [[comment1, tl1],
-                  [comment2, tl1]], cm.comment_location
+    # Both comments should appear in the rendered description
+    assert_equal({ tl1 => [comment1, comment2] }, cm.comment_location)
   end
 
   def test_add_comment_stopdoc
@@ -156,7 +156,7 @@ def test_from_module_comment
 
     klass = RDoc::ClassModule.from_module RDoc::NormalClass, klass
 
-    assert_equal [['really a class', tl]], klass.comment_location
+    assert_equal({ tl => ['really a class'] }, klass.comment_location)
   end
 
   def test_marshal_dump
@@ -631,7 +631,7 @@ def test_search_snippet_after_marshal
     assert_match(/class comment/, snippet)
   end
 
-  def test_comment_location_is_array_after_marshal
+  def test_comment_location_is_hash_after_marshal
     @store.path = Dir.tmpdir
     tl = @store.add_file 'file.rb'
 
@@ -642,10 +642,11 @@ def test_comment_location_is_array_after_marshal
     loaded = Marshal.load Marshal.dump cm
     loaded.store = @store
 
-    assert_kind_of Array, loaded.comment_location
-    assert_equal 1, loaded.comment_location.length
+    assert_kind_of Hash, loaded.comment_location
+    assert_equal 1, loaded.comment_location.size
 
-    comment, location = loaded.comment_location.first
+    location, comments = loaded.comment_location.first
+    comment = comments.first
     assert_kind_of RDoc::Markup::Document, comment
     # After marshal, location is the filename string (from doc.file)
     assert_equal tl.relative_name, location
@@ -668,7 +669,8 @@ def test_merge
     assert c1.current_section, 'original current_section'
     assert c2.current_section, 'merged current_section'
 
-    comment, location = c2.comment_location.first
+    location, comments = c2.comment_location.first
+    comment = comments.first
     assert_kind_of RDoc::Markup::Document, comment
     assert_equal tl.relative_name, location
   end
@@ -793,7 +795,7 @@ def test_merge_comment
     inner2 = @RM::Document.new @RM::Paragraph.new 'klass 2'
     inner2.file = 'two.rb'
 
-    expected = @RM::Document.new inner2, inner1
+    expected = @RM::Document.new inner1, inner2
 
     assert_equal expected, cm1.comment.parse
   end
@@ -1217,17 +1219,15 @@ def test_parse_comment_location
     cm.add_comment 'comment 1', tl1
     cm.add_comment 'comment 2', tl2
 
-    assert_kind_of Array, cm.comment_location
-    assert_equal 2, cm.comment_location.length
-    assert_equal 'comment 1', cm.comment_location[0][0]
-    assert_equal tl1, cm.comment_location[0][1]
-    assert_equal 'comment 2', cm.comment_location[1][0]
-    assert_equal tl2, cm.comment_location[1][1]
+    assert_kind_of Hash, cm.comment_location
+    assert_equal 2, cm.comment_location.size
+    assert_equal ['comment 1'], cm.comment_location[tl1]
+    assert_equal ['comment 2'], cm.comment_location[tl2]
 
     cm = Marshal.load Marshal.dump cm
 
-    # After marshal, comment_location should still be an array
-    assert_kind_of Array, cm.comment_location
+    # After marshal, comment_location should still be a hash
+    assert_kind_of Hash, cm.comment_location
 
     # parse() produces a Document with parts for each comment
     parsed = cm.parse(cm.comment_location)

test/rdoc/parser/c_test.rb

@@ -318,8 +318,7 @@ def test_do_classes_duplicate_class
 
     klass = util_get_class content, 'cFoo'
     assert_equal 1, klass.comment_location.size
-    first = klass.comment_location.first
-    first_comment = first[0]
+    first_comment = klass.comment_location.each_value.first&.first
     assert_equal 'first', first_comment.text
   end
 

test/rdoc/parser/prism_ruby_test.rb

@@ -1939,7 +1939,7 @@ module Foo
       RDoc::Comment.new('comment b', @top_level)
     ]
 
-    assert_equal expected, mod.comment_location.map { |c, _l| c }
+    assert_equal expected, mod.comment_location[@top_level]
   end
 
   def test_enddoc

test/rdoc/parser/ruby_test.rb

@@ -801,9 +801,7 @@ def test_parse_class_in_a_file_repeatedly
 
     foo = @top_level.classes.first
     assert_equal 'Foo', foo.full_name
-    assert_equal [[comment_a, @top_level],
-                  [comment_b, @top_level],
-                  [comment_c, @top_level]], foo.comment_location
+    assert_equal({ @top_level => [comment_a, comment_b, comment_c] }, foo.comment_location)
     assert_equal [@top_level], foo.in_files
     assert_equal 1, foo.line
   end
@@ -3722,7 +3720,7 @@ module Foo
       RDoc::Comment.new('comment b', @top_level)
     ]
 
-    assert_equal expected, foo.comment_location.map { |c, l| c }
+    assert_equal expected, foo.comment_location[@top_level]
   end
 
   def test_scan_meta_method_block

test/rdoc/rdoc_store_test.rb

@@ -924,12 +924,13 @@ def test_save_class_merge
 
     loaded = s.load_class('Object')
 
-    # After loading, comment_location is an array (not a Document)
-    assert_kind_of Array, loaded.comment_location
-    assert_equal 1, loaded.comment_location.length
+    # After loading, comment_location is a hash (not a Document)
+    assert_kind_of Hash, loaded.comment_location
+    assert_equal 1, loaded.comment_location.size
 
     # Verify content is preserved
-    comment, location = loaded.comment_location.first
+    location, comments = loaded.comment_location.first
+    comment = comments.first
     assert_kind_of @RM::Document, comment
     assert_equal 'new comment', comment.parts[0].text
     assert_equal @top_level.relative_name, location