Stop accepting Document objects as CodeObject#comment (#1210)

`CodeObject#comment` is normally String or Comment, but Marshal dump and
load creates CodeObject with Document as a comment. So there are two
types of CodeObject that shouldn't be mixed:
- ParsedCodeObject: comment is a String or a Comment
- MarshalLoadedCodeObject: comment is a Document

This is implicitly doubling the number of total classes in RDoc. It is
mixing huge complexity and potential bugs to RDoc codebase. Some method
only accepts ParsedCodeObject. Some method only accepts
MarshalLoadedCodeObject. It is difficult to know.
```ruby
def add_extension_modules_single out, store, include
  # include should be RDoc::Markup::(MarshalLoaded)Include. Does not accept RDoc::Markup::(Parsed)Include
  ...
  out << RDoc::Markup::BlankLine.new
  out << include.comment
end
```

It looks like document is assigned to comment to represent
parse-result-cached comment. Alternatively, `Comment#document=` can be
used to represent it.

Ideally, we should avoid mixing String with Comment, but String it is
too frequently used. I think it will be easy to remove mixing String
after switching to PrismRuby Parser.

I believe we should do this refactor someday but maybe not before Ruby
3.4 because the release date is pretty close.

---------

Co-authored-by: Stan Lo <[email protected]>

Author: tompng

lib/rdoc/code_object.rb

@@ -141,7 +141,6 @@ def initialize_visibility # :nodoc:
   def comment=(comment)
     @comment = case comment
                when NilClass               then ''
-               when RDoc::Markup::Document then comment
                when RDoc::Comment          then comment.normalize
                else
                  if comment and not comment.empty? then

lib/rdoc/code_object/any_method.rb

@@ -198,7 +198,7 @@ def marshal_load array
     @full_name     = array[2]
     @singleton     = array[3]
     @visibility    = array[4]
-    @comment       = array[5]
+    @comment       = RDoc::Comment.from_document array[5]
     @call_seq      = array[6]
     @block_params  = array[7]
     #                      8 handled below
@@ -210,8 +210,8 @@ def marshal_load array
     @section_title = array[14]
     @is_alias_for  = array[15]
 
-    array[8].each do |new_name, comment|
-      add_alias RDoc::Alias.new(nil, @name, new_name, comment, @singleton)
+    array[8].each do |new_name, document|
+      add_alias RDoc::Alias.new(nil, @name, new_name, RDoc::Comment.from_document(document), @singleton)
     end
 
     @parent_name ||= if @full_name =~ /#/ then

lib/rdoc/code_object/attr.rb

@@ -136,7 +136,7 @@ def marshal_load array
     @full_name     = array[2]
     @rw            = array[3]
     @visibility    = array[4]
-    @comment       = array[5]
+    @comment       = RDoc::Comment.from_document array[5]
     @singleton     = array[6] || false # MARSHAL_VERSION == 0
     #                      7 handled below
     @parent_name   = array[8]

lib/rdoc/code_object/class_module.rb

@@ -359,12 +359,14 @@ def marshal_load array # :nodoc:
     @name       = array[1]
     @full_name  = array[2]
     @superclass = array[3]
-    @comment    = array[4]
+    document    = array[4]
 
-    @comment_location = if RDoc::Markup::Document === @comment.parts.first then
-                          @comment
+    @comment    = RDoc::Comment.from_document document
+
+    @comment_location = if RDoc::Markup::Document === document.parts.first then
+                          document
                         else
-                          RDoc::Markup::Document.new @comment
+                          RDoc::Markup::Document.new document
                         end
 
     array[5].each do |name, rw, visibility, singleton, file|
@@ -378,18 +380,18 @@ def marshal_load array # :nodoc:
       attr.record_location RDoc::TopLevel.new file
     end
 
-    array[6].each do |constant, comment, file|
+    array[6].each do |constant, document, file|
       case constant
       when RDoc::Constant then
         add_constant constant
       else
-        constant = add_constant RDoc::Constant.new(constant, nil, comment)
+        constant = add_constant RDoc::Constant.new(constant, nil, RDoc::Comment.from_document(document))
         constant.record_location RDoc::TopLevel.new file
       end
     end
 
-    array[7].each do |name, comment, file|
-      incl = add_include RDoc::Include.new(name, comment)
+    array[7].each do |name, document, file|
+      incl = add_include RDoc::Include.new(name, RDoc::Comment.from_document(document))
       incl.record_location RDoc::TopLevel.new file
     end
 
@@ -406,8 +408,8 @@ def marshal_load array # :nodoc:
       end
     end
 
-    array[9].each do |name, comment, file|
-      ext = add_extend RDoc::Extend.new(name, comment)
+    array[9].each do |name, document, file|
+      ext = add_extend RDoc::Extend.new(name, RDoc::Comment.from_document(document))
       ext.record_location RDoc::TopLevel.new file
     end if array[9] # Support Marshal version 1
 
@@ -444,7 +446,8 @@ def merge class_module
 
       document = document.merge other_document
 
-      @comment = @comment_location = document
+      @comment = RDoc::Comment.from_document(document)
+      @comment_location = document
     end
 
     cm = class_module

lib/rdoc/code_object/constant.rb

@@ -133,7 +133,7 @@ def marshal_dump
   # * #parent_name
 
   def marshal_load array
-    initialize array[1], nil, array[5]
+    initialize array[1], nil, RDoc::Comment.from_document(array[5])
 
     @full_name     = array[2]
     @visibility    = array[3] || :public

lib/rdoc/code_object/context/section.rb

@@ -61,19 +61,10 @@ def == other
   # Adds +comment+ to this section
 
   def add_comment comment
-    comment = extract_comment comment
-
-    return if comment.empty?
-
-    case comment
-    when RDoc::Comment then
-      @comments << comment
-    when RDoc::Markup::Document then
-      @comments.concat comment.parts
-    when Array then
-      @comments.concat comment
-    else
-      raise TypeError, "unknown comment type: #{comment.inspect}"
+    comments = Array(comment)
+    comments.each do |c|
+      extracted_comment = extract_comment(c)
+      @comments << extracted_comment unless extracted_comment.empty?
     end
   end
 
@@ -97,10 +88,6 @@ def aref
 
   def extract_comment comment
     case comment
-    when Array then
-      comment.map do |c|
-        extract_comment c
-      end
     when nil
       RDoc::Comment.new ''
     when RDoc::Comment then
@@ -115,8 +102,6 @@ def extract_comment comment
                        end
       end
 
-      comment
-    when RDoc::Markup::Document then
       comment
     else
       raise TypeError, "unknown comment #{comment.inspect}"
@@ -135,20 +120,7 @@ def hash # :nodoc:
   # The files comments in this section come from
 
   def in_files
-    return [] if @comments.empty?
-
-    case @comments
-    when Array then
-      @comments.map do |comment|
-        comment.file
-      end
-    when RDoc::Markup::Document then
-      @comment.parts.map do |document|
-        document.file
-      end
-    else
-      raise RDoc::Error, "BUG: unknown comment class #{@comments.class}"
-    end
+    @comments.map(&:file)
   end
 
   ##
@@ -170,34 +142,15 @@ def marshal_load array
     @parent  = nil
 
     @title    = array[1]
-    @comments = array[2]
+    @comments = array[2].parts.map { |doc| RDoc::Comment.from_document(doc) }
   end
 
   ##
   # Parses +comment_location+ into an RDoc::Markup::Document composed of
   # multiple RDoc::Markup::Documents with their file set.
 
   def parse
-    case @comments
-    when String then
-      super
-    when Array then
-      docs = @comments.map do |comment, location|
-        doc = super comment
-        doc.file = location if location
-        doc
-      end
-
-      RDoc::Markup::Document.new(*docs)
-    when RDoc::Comment then
-      doc = super @comments.text, comments.format
-      doc.file = @comments.location
-      doc
-    when RDoc::Markup::Document then
-      return @comments
-    else
-      raise ArgumentError, "unknown comment class #{comments.class}"
-    end
+    RDoc::Markup::Document.new(*@comments.map(&:parse))
   end
 
   ##
@@ -213,20 +166,9 @@ def plain_html
   # Removes a comment from this section if it is from the same file as
   # +comment+
 
-  def remove_comment comment
-    return if @comments.empty?
-
-    case @comments
-    when Array then
-      @comments.delete_if do |my_comment|
-        my_comment.file == comment.file
-      end
-    when RDoc::Markup::Document then
-      @comments.parts.delete_if do |document|
-        document.file == comment.file.name
-      end
-    else
-      raise RDoc::Error, "BUG: unknown comment class #{@comments.class}"
+  def remove_comment target_comment
+    @comments.delete_if do |stored_comment|
+      stored_comment.file == target_comment.file
     end
   end
 

lib/rdoc/code_object/top_level.rb

@@ -214,7 +214,7 @@ def marshal_load array # :nodoc:
     initialize array[1]
 
     @parser  = array[2]
-    @comment = array[3]
+    @comment = RDoc::Comment.from_document array[3]
 
     @file_stat          = nil
   end

lib/rdoc/comment.rb

@@ -126,7 +126,7 @@ def extract_call_seq method
   # A comment is empty if its text String is empty.
 
   def empty?
-    @text.empty?
+    @text.empty? && (@document.nil? || @document.empty?)
   end
 
   ##
@@ -226,4 +226,14 @@ def tomdoc?
     @format == 'tomdoc'
   end
 
+  ##
+  # Create a new parsed comment from a document
+
+  def self.from_document(document) # :nodoc:
+    comment = RDoc::Comment.new('')
+    comment.document = document
+    comment.location = RDoc::TopLevel.new(document.file) if document.file
+    comment
+  end
+
 end

lib/rdoc/generator/darkfish.rb

@@ -783,20 +783,13 @@ def template_for file, page = true, klass = ERB
     template
   end
 
-  # Returns an excerpt of the content for usage in meta description tags
-  def excerpt(content)
-    text = case content
+  # Returns an excerpt of the comment for usage in meta description tags
+  def excerpt(comment)
+    text = case comment
     when RDoc::Comment
-      content.text
-    when RDoc::Markup::Document
-      # This case is for page files that are not markdown nor rdoc
-      # We convert them to markdown for now as it's easier to extract the text
-      formatter = RDoc::Markup::ToMarkdown.new
-      formatter.start_accepting
-      formatter.accept_document(content)
-      formatter.end_accepting
+      comment.text
     else
-      content
+      comment
     end
 
     # Match from a capital letter to the first period, discarding any links, so

lib/rdoc/parser/changelog.rb

@@ -210,8 +210,9 @@ def scan
     grouped_entries = group_entries entries
 
     doc = create_document grouped_entries
-
-    @top_level.comment = doc
+    comment = RDoc::Comment.new(@content)
+    comment.document = doc
+    @top_level.comment = comment
 
     @top_level
   end