Change comment directive parsing (#1149)

Fix comment directive parsing problem

# Problem of comment parsing

The main problem is that `@preprocess.handle` parses comment, removes
directive, and process code_object at the same time.
This pull request change RDoc to parse comment and extract directives
first, and then apply directives to code object.

## Flow of legacy RDoc parsing method
For example parsing this code
```ruby
class A
  # :yields: x, y
  # :args:   a, b
  # :call-seq: 
  #--
  # :not-new:
  # :category: foobar
  #++
  #   initialize(x, y, z)
  def initialize(*args, &block); end
end
```

### Step 1
RDoc performs `@preprocess.hanlde` to RDoc::NormalClass.
- `:category:` is applied to klass and replaced with blank line
- `:not-new:` and `:yields:` are replaced with blank line. maybe bug.
- `:args:   a, b` is replaced with `:args: a, b`

### Step 2
RDoc performs `@preprocess.hanlde` to RDoc::AnyMethod.
`:args: a, b` is applied to `meth.params`.

### Step 3
RDoc removes private section that starts with `#--` and ends with `#++`.

### Step 4
RDoc normalizes comment by removing `#` and indentation.

### Step 5
RDoc extracts `":call-seq:\n initialize(x, y, z)` from comment and apply
to method object.

## Problems

RDoc removes directives and expand `:include:` twice in some case, and
once in other case.
To avoid all directives removed in the first `@preprocess.handle`,
preprocess needs directive-replace mechanizm which is making things
complex.

Private section and call-seq are processed later. This is making RDoc
accept weird comment like directive inside private section and private
section inside call-seq.

Handling meta programming method is also hard.
`@preprocess.handle(comment, code_object)` requires code object already
created.
We need to parse the comment to know the code object type (method or
attribute). After that, we can finally parse the comment with the code
object.

C comments are also complicated. :include: can include text containing
`*/`.
Removing directive line and private section from the comment might
remove `/*` and `*/` which makes normalize_comment fail.
The original implementation was avoiding this by using different
processing order than ruby parser. This is not consistent.

# Solution

We need to parse comment first and only once to extract directives.
Expand `:include:`, read directives (including `:call-seq:`), remove
private section at the same time.
Comment parser should return normalized comment text and directives as
an attribute hash. Directive should also contain line number.

# Changed things

## :call-seq:
New type of directive called "multiline directive" is introduced to make
`:call-seq:` also a directive.
```
# :multiline-directive:
#   html
#     head
#       title
#
#     body
#       header
#       footer
```
Multiline directive ends with blank line. This restriction is for
compatibility with old RDoc.
Some invalid multiline directive (unindented, ends with other directive)
is also accepted with warning.

The resuld of parsing this call-seq is changed. I think it get better.
```
# :call-seq:
#   STDIN.getc()     -> string # Only this line was call-seq
#
#   STDIN.getc(a)    -> string
#
#   STDIN.getc(a, b) -> string
#   $stdin.getc(c)   -> string # It's now call-seq until this line
#
# :other:
```

## Private section

`#----foobar` was accepted as private section start.
`#++++foobar` was decomposed to `#++`(private end) and `++foobar`(normal
comment).
Start is now `/^#-{2,}$/` (two or more -), end is now `/^#\+{2}$/`
(exactly two +).

## Unhandled directives

In old RDoc, unhandled directive `# :unknown: foo` remain in normal
comment.
Now it is removed just like other directives. Unhandled directive is
appended to code object's metadata. It does not make sence to leave
metadata in the comment. I think this was just a side effect of avoiding
double parsing problem.

## Normalize and remove private section

Everything is done in parse phase

## C and Simple parser

C used to accept `/*\n# :directive:\n*/` but now only accepts `*
:directive:`.
Changes for call-seq, private section and unhandled directive described
above are also applied to C and Simple parser.

# Old comment parsing

`RDoc::Markup::PreProcess#handle` `RDoc::Comment#extract_call_seq`
`RDoc::Comment#remove_private` is only used from `RDoc::Parser::Ruby`.
We can remove them in the future.

# Diff (updated: 2025/02/02)

I compared generated html files of rdoc itself and in `ruby/ruby`.

## HTML meta tag content (ruby/ruby)
Files:
```
Date/Error.html
Enumerator/Generator.html
Enumerator/Producer.html
Enumerator/Yielder.html
Fiddle/Pointer.html
UnicodeNormalize.html
```
Example diff
```html
<meta name="description" content="class Date::Error: Exception for invalid date/time ">
↓
<meta name="description" content="class Date::Error: Exception for invalid date/time">
```

## OpenSSL/Timestamp/Factory.html (ruby/ruby)

This invalid document is parsed differentl
```c
/* Document-class: OpenSSL::Timestamp::Factory
 * Document for default_policy_id
 * call-seq:
 *       factory.default_policy_id = "string" -> string
 * Document for serial_number
 * call-seq:
 *       factory.serial_number = number -> number
 * Document for gen_time
 * call-seq:
 *       factory.gen_time = Time -> Time
 */
```

## Win32.html (ruby/ruby, RDOC_USE_PRISM_PARSER)
This will no longer considered to be a private section(invisible comment
surrounded by -- and ++)
```
--- info
--- num_keys
```

## History_rdoc.html (ruby/rdoc)

Parsing this part is improved.
```md
* Bug fixes
  * `ri []` and other special methods now work properly.  Issue #52 by
    ddebernardy.
  * `ri` now has space between class comments from multiple files.
  * :stopdoc: no longer creates Object references.  Issue #55 by Simon Chiang
  * :nodoc: works on class aliases now.  Issue #51 by Steven G. Harms
  * Remove tokenizer restriction on header lengths for verbatim sections.
    Issue #49 by trans
```
The [current
document](https://ruby.github.io/rdoc/History_rdoc.html#label-3.9+-2F+2011-07-30)
looks like `* :stopdoc:` and `* :nodoc:` was processed as directive.

## lib/rdoc/markdown_kpeg.html (ruby/rdoc)
Maybe it shouldn't be documented.
https://ruby.github.io/rdoc/lib/rdoc/markdown_kpeg.html

## RDoc/MarkupReference.html (ruby/rdoc, RDOC_USE_PRISM_PARSER)
`<pre>:call-seq: ` →  `<pre>:call-seq:` (trailing space removed)

## RDoc/Parser/Ruby.html (ruby/rdoc, RDOC_USE_PRISM_PARSER)
Escape of `# \:method: or :attr: directives in +comment+.` is now
working.
Note that this is related to an old bug in master branch
```
class Foo
  # A string constant with
  # \:nodoc: (this is documented. :nodoc: is escaped)
  A = ':nodoc:
  # Prints the word
  # \:nodoc: (this method is not documented. :nodoc: is not escaped)
  def print_colon_nodoc = puts(':nodoc:')
end
```

Author: tompng

History.rdoc

@@ -163,7 +163,7 @@
   * Moved old DEVELOPERS file to CONTRIBUTING to match github conventions.
   * TomDoc output now has a "Returns" heading.  Issue #234 by Brian Henderson
   * Metaprogrammed methods can now use the :args: directive in addition to the
-    :call-seq: directive.  Issue #236 by Mike Moore.
+    \:call-seq: directive.  Issue #236 by Mike Moore.
   * Sections can be linked to using "@" like labels.  If a section and a label
     have the same name the section will be preferred.  Issue #233 by Brian
     Henderson.

doc/rdoc/markup_reference.rb

@@ -1264,7 +1264,7 @@ def dummy_instance_method(foo, bar); end;
   #
   # Here is the <tt>:call-seq:</tt> directive given for the method:
   #
-  #   :call-seq:
+  #   \:call-seq:
   #     call_seq_directive(foo, bar)
   #     Can be anything -> bar
   #     Also anything more -> baz or bat

lib/rdoc/comment.rb

@@ -162,6 +162,12 @@ def normalize
     self
   end
 
+  # Change normalized, when creating already normalized comment.
+
+  def normalized=(value)
+    @normalized = value
+  end
+
   ##
   # Was this text normalized?
 
@@ -223,14 +229,190 @@ def tomdoc?
     @format == 'tomdoc'
   end
 
-  ##
-  # Create a new parsed comment from a document
+  MULTILINE_DIRECTIVES = %w[call-seq].freeze # :nodoc:
 
-  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
+  # There are more, but already handled by RDoc::Parser::C
+  COLON_LESS_DIRECTIVES = %w[call-seq Document-method].freeze # :nodoc:
+
+  DIRECTIVE_OR_ESCAPED_DIRECTIV_REGEXP = /\A(?<colon>\\?:|:?)(?<directive>[\w-]+):(?<param>.*)/
+
+  private_constant :MULTILINE_DIRECTIVES, :COLON_LESS_DIRECTIVES, :DIRECTIVE_OR_ESCAPED_DIRECTIV_REGEXP
+
+  class << self
+
+    ##
+    # Create a new parsed comment from a document
 
+    def from_document(document) # :nodoc:
+      comment = RDoc::Comment.new('')
+      comment.document = document
+      comment.location = RDoc::TopLevel.new(document.file) if document.file
+      comment
+    end
+
+    # Parse comment, collect directives as an attribute and return [normalized_comment_text, directives_hash]
+    # This method expands include and removes everything not needed in the document text, such as
+    # private section, directive line, comment characters `# /* * */` and indent spaces.
+    #
+    # RDoc comment consists of include, directive, multiline directive, private section and comment text.
+    #
+    # Include
+    #   # :include: filename
+    #
+    # Directive
+    #   # :directive-without-value:
+    #   # :directive-with-value: value
+    #
+    # Multiline directive (only :call-seq:)
+    #   # :multiline-directive:
+    #   #   value1
+    #   #   value2
+    #
+    # Private section
+    #   #--
+    #   # private comment
+    #   #++
+
+    def parse(text, filename, line_no, type, &include_callback)
+      case type
+      when :ruby
+        text = text.gsub(/^#+/, '') if text.start_with?('#')
+        private_start_regexp = /^-{2,}$/
+        private_end_regexp = /^\+{2}$/
+        indent_regexp = /^\s*/
+      when :c
+        private_start_regexp = /^(\s*\*)?-{2,}$/
+        private_end_regexp = /^(\s*\*)?\+{2}$/
+        indent_regexp = /^\s*(\/\*+|\*)?\s*/
+        text = text.gsub(/\s*\*+\/\s*\z/, '')
+      when :simple
+        # Unlike other types, this implementation only looks for two dashes at
+        # the beginning of the line. Three or more dashes are considered to be
+        # a rule and ignored.
+        private_start_regexp = /^-{2}$/
+        private_end_regexp = /^\+{2}$/
+        indent_regexp = /^\s*/
+      end
+
+      directives = {}
+      lines = text.split("\n")
+      in_private = false
+      comment_lines = []
+      until lines.empty?
+        line = lines.shift
+        read_lines = 1
+        if in_private
+          # If `++` appears in a private section that starts with `--`, private section ends.
+          in_private = false if line.match?(private_end_regexp)
+          line_no += read_lines
+          next
+        elsif line.match?(private_start_regexp)
+          # If `--` appears in a line, private section starts.
+          in_private = true
+          line_no += read_lines
+          next
+        end
+
+        prefix = line[indent_regexp]
+        prefix_indent = ' ' * prefix.size
+        line = line.byteslice(prefix.bytesize..)
+
+        if (directive_match = DIRECTIVE_OR_ESCAPED_DIRECTIV_REGEXP.match(line))
+          colon = directive_match[:colon]
+          directive = directive_match[:directive]
+          raw_param = directive_match[:param]
+          param = raw_param.strip
+        else
+          colon = directive = raw_param = param = nil
+        end
+
+        if !directive
+          comment_lines << prefix_indent + line
+        elsif colon == '\\:'
+          # If directive is escaped, unescape it
+          comment_lines << prefix_indent + line.sub('\\:', ':')
+        elsif raw_param.start_with?(':') || (colon.empty? && !COLON_LESS_DIRECTIVES.include?(directive))
+          # Something like `:toto::` is not a directive
+          # Only few directives allows to start without a colon
+          comment_lines << prefix_indent + line
+        elsif directive == 'include'
+          filename_to_include = param
+          include_callback.call(filename_to_include, prefix_indent).lines.each { |l| comment_lines << l.chomp }
+        elsif MULTILINE_DIRECTIVES.include?(directive)
+          value_lines = take_multiline_directive_value_lines(directive, filename, line_no, lines, prefix_indent.size, indent_regexp, !param.empty?)
+          read_lines += value_lines.size
+          lines.shift(value_lines.size)
+          unless param.empty?
+            # Accept `:call-seq: first-line\n  second-line` for now
+            value_lines.unshift(param)
+          end
+          value = value_lines.join("\n")
+          directives[directive] = [value.empty? ? nil : value, line_no]
+        else
+          directives[directive] = [param.empty? ? nil : param, line_no]
+        end
+        line_no += read_lines
+      end
+
+      normalized_comment = String.new(encoding: text.encoding) << normalize_comment_lines(comment_lines).join("\n")
+      [normalized_comment, directives]
+    end
+
+    # Remove preceding indent spaces and blank lines from the comment lines
+
+    private def normalize_comment_lines(lines)
+      blank_line_regexp = /\A\s*\z/
+      lines = lines.dup
+      lines.shift while lines.first&.match?(blank_line_regexp)
+      lines.pop while lines.last&.match?(blank_line_regexp)
+
+      min_spaces = lines.map do |l|
+        l.match(/\A *(?=\S)/)&.end(0)
+      end.compact.min
+      if min_spaces && min_spaces > 0
+        lines.map { |l| l[min_spaces..] || '' }
+      else
+        lines
+      end
+    end
+
+    # Take value lines of multiline directive
+
+    private def take_multiline_directive_value_lines(directive, filename, line_no, lines, base_indent_size, indent_regexp, has_param)
+      return [] if lines.empty?
+
+      first_indent_size = lines.first.match(indent_regexp).end(0)
+
+      # Blank line or unindented line is not part of multiline-directive value
+      return [] if first_indent_size <= base_indent_size
+
+      if has_param
+        # :multiline-directive: line1
+        #   line2
+        #   line3
+        #
+        value_lines = lines.take_while do |l|
+          l.rstrip.match(indent_regexp).end(0) > base_indent_size
+        end
+        min_indent = value_lines.map { |l| l.match(indent_regexp).end(0) }.min
+        value_lines.map { |l| l[min_indent..] }
+      else
+        # Take indented lines accepting blank lines between them
+        value_lines = lines.take_while do |l|
+          l = l.rstrip
+          indent = l[indent_regexp]
+          if indent == l || indent.size >= first_indent_size
+            true
+          end
+        end
+        value_lines.map! { |l| (l[first_indent_size..] || '').chomp }
+
+        if value_lines.size != lines.size && !value_lines.last.empty?
+          warn "#{filename}:#{line_no} Multiline directive :#{directive}: should end with a blank line."
+        end
+        value_lines.pop while value_lines.last&.empty?
+        value_lines
+      end
+    end
+  end
 end

lib/rdoc/markup/pre_process.rb

@@ -97,18 +97,15 @@ def initialize(input_file_name, include_path)
   # RDoc::CodeObject#metadata for details.
 
   def handle(text, code_object = nil, &block)
-    first_line = 1
     if RDoc::Comment === text then
       comment = text
       text = text.text
-      first_line = comment.line || 1
     end
 
     # regexp helper (square brackets for optional)
     # $1      $2  $3        $4      $5
     # [prefix][\]:directive:[spaces][param]newline
-    text = text.lines.map.with_index(first_line) do |line, num|
-      next line unless line =~ /\A([ \t]*(?:#|\/?\*)?[ \t]*)(\\?):([\w-]+):([ \t]*)(.+)?(\r?\n|$)/
+    text = text.gsub(/^([ \t]*(?:#|\/?\*)?[ \t]*)(\\?):([\w-]+):([ \t]*)(.+)?(\r?\n|$)/) do
       # skip something like ':toto::'
       next $& if $4.empty? and $5 and $5[0, 1] == ':'
 
@@ -122,21 +119,48 @@ def handle(text, code_object = nil, &block)
         comment.format = $5.downcase
         next "#{$1.strip}\n"
       end
-
-      handle_directive $1, $3, $5, code_object, text.encoding, num, &block
-    end.join
+      handle_directive $1, $3, $5, code_object, text.encoding, &block
+    end
 
     if comment then
       comment.text = text
     else
       comment = text
     end
 
+    run_post_processes(comment, code_object)
+
+    text
+  end
+
+  # Apply directives to a code object
+
+  def run_pre_processes(comment_text, code_object, start_line_no, type)
+    comment_text, directives = parse_comment(comment_text, start_line_no, type)
+    directives.each do |directive, (param, line_no)|
+      handle_directive('', directive, param, code_object)
+    end
+    if code_object.is_a?(RDoc::AnyMethod) && (call_seq, = directives['call-seq']) && call_seq
+      code_object.call_seq = call_seq.lines.map(&:chomp).reject(&:empty?).join("\n")
+    end
+    format, = directives['markup']
+    [comment_text, format]
+  end
+
+  # Perform post preocesses to a code object
+
+  def run_post_processes(comment, code_object)
     self.class.post_processors.each do |handler|
       handler.call comment, code_object
     end
+  end
 
-    text
+  # Parse comment and return [normalized_comment_text, directives_hash]
+
+  def parse_comment(text, line_no, type)
+    RDoc::Comment.parse(text, @input_file_name, line_no, type) do |filename, prefix_indent|
+      include_file(filename, prefix_indent, text.encoding)
+    end
   end
 
   ##
@@ -151,7 +175,7 @@ def handle(text, code_object = nil, &block)
   # When 1.8.7 support is ditched prefix can be defaulted to ''
 
   def handle_directive(prefix, directive, param, code_object = nil,
-                       encoding = nil, line = nil)
+                       encoding = nil)
     blankline = "#{prefix.strip}\n"
     directive = directive.downcase
 
@@ -244,7 +268,7 @@ def handle_directive(prefix, directive, param, code_object = nil,
 
       blankline
     else
-      result = yield directive, param, line if block_given?
+      result = yield directive, param if block_given?
 
       case result
       when nil then