Skip to content

Commit

Permalink
Document is not a Comment. Document shouldn't be assigned to code_obj…
Browse files Browse the repository at this point in the history
…ect.comment

CodeObject#comment is normally String or Comment, but Marshal.dump and load creates CodeObject with comment=Document.
Some method requires Document stored in CodeObject#comment, some requires Comment.
We should stop mixing Document with Comment because it is mixing huge complexity and potential bugs to RDoc codebase.
  • Loading branch information
tompng committed Nov 16, 2024
1 parent 6852567 commit 523622d
Show file tree
Hide file tree
Showing 21 changed files with 141 additions and 180 deletions.
1 change: 0 additions & 1 deletion lib/rdoc/code_object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/rdoc/code_object/any_method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/rdoc/code_object/attr.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
25 changes: 14 additions & 11 deletions lib/rdoc/code_object/class_module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand All @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/rdoc/code_object/constant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
77 changes: 12 additions & 65 deletions lib/rdoc/code_object/context/section.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,13 @@ 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
if comment.is_a?(Array)
comment.each do |c|
@comments << extract_comment(c)
end
else
raise TypeError, "unknown comment type: #{comment.inspect}"
comment = extract_comment(comment)
@comments << comment unless comment.empty?
end
end

Expand All @@ -97,10 +91,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
Expand All @@ -115,8 +105,6 @@ def extract_comment comment
end
end

comment
when RDoc::Markup::Document then
comment
else
raise TypeError, "unknown comment #{comment.inspect}"
Expand All @@ -135,19 +123,8 @@ 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}"
@comments.map do |comment|
comment.file
end
end

Expand All @@ -170,34 +147,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

##
Expand All @@ -214,19 +172,8 @@ def plain_html
# +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}"
@comments.delete_if do |my_comment|
my_comment.file == comment.file
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/rdoc/code_object/top_level.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion lib/rdoc/comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

##
Expand Down Expand Up @@ -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
5 changes: 3 additions & 2 deletions lib/rdoc/parser/changelog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 16 additions & 15 deletions lib/rdoc/ri/driver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ def add_extension_modules_multiple out, store, modules # :nodoc:
with.each do |incl|
out << RDoc::Markup::Paragraph.new(incl.name)
out << RDoc::Markup::BlankLine.new
out << incl.comment
out << incl.comment.parse
end

unless wout.empty? then
Expand All @@ -534,7 +534,7 @@ def add_extension_modules_single out, store, include # :nodoc:

if include.comment then
out << RDoc::Markup::BlankLine.new
out << include.comment
out << include.comment.parse
end
end

Expand Down Expand Up @@ -651,12 +651,12 @@ def class_document name, found, klasses, includes, extends
##
# Adds the class +comment+ to +out+.

def class_document_comment out, comment # :nodoc:
unless comment.empty? then
def class_document_comment out, document # :nodoc:
unless document.empty? then
out << RDoc::Markup::Rule.new(1)

if comment.merged? then
parts = comment.parts
if document.merged? then
parts = document.parts
parts = parts.zip [RDoc::Markup::BlankLine.new] * parts.length
parts.flatten!
parts.pop
Expand All @@ -681,7 +681,7 @@ def class_document_constants out, klass # :nodoc:
constants = klass.constants.sort_by { |constant| constant.name }

list.items.concat constants.map { |constant|
parts = constant.comment.parts if constant.comment
parts = constant.comment.parse.parts
parts << RDoc::Markup::Paragraph.new('[not documented]') if
parts.empty?

Expand Down Expand Up @@ -898,7 +898,7 @@ def display_page name

page = store.load_page page_name

display page.comment
display page.comment.parse
end

##
Expand Down Expand Up @@ -1199,7 +1199,8 @@ def load_method store, cache, klass, type, name

store.load_method klass, "#{type}#{method}"
rescue RDoc::Store::MissingFileError => e
comment = RDoc::Comment.new("missing documentation at #{e.file}").parse
comment = RDoc::Comment.new("missing documentation at #{e.file}")
comment.parse

method = RDoc::AnyMethod.new nil, name
method.comment = comment
Expand Down Expand Up @@ -1361,21 +1362,21 @@ def parse_name name
# documentable items the class is added to +also_in+ instead.

def render_class out, store, klass, also_in # :nodoc:
comment = klass.comment
document = klass.comment.parse
# TODO the store's cache should always return an empty Array
class_methods = store.class_methods[klass.full_name] || []
instance_methods = store.instance_methods[klass.full_name] || []
attributes = store.attributes[klass.full_name] || []

if comment.empty? and
if document.empty? and
instance_methods.empty? and class_methods.empty? then
also_in << store
return
end

add_from out, store

class_document_comment out, comment
class_document_comment out, document

if class_methods or instance_methods or not klass.constants.empty? then
out << RDoc::Markup::Rule.new(1)
Expand Down Expand Up @@ -1423,16 +1424,16 @@ def render_method_comment out, method, alias_for = nil# :nodoc:
if alias_for
unless method.comment.nil? or method.comment.empty?
out << RDoc::Markup::BlankLine.new
out << method.comment
out << method.comment.parse
end
out << RDoc::Markup::BlankLine.new
out << RDoc::Markup::Paragraph.new("(This method is an alias for #{alias_for.full_name}.)")
out << RDoc::Markup::BlankLine.new
out << alias_for.comment
out << alias_for.comment.parse
out << RDoc::Markup::BlankLine.new
else
out << RDoc::Markup::BlankLine.new
out << method.comment
out << method.comment.parse
out << RDoc::Markup::BlankLine.new
end
end
Expand Down
Loading

0 comments on commit 523622d

Please sign in to comment.