Reduce document difference between RDoc::Parser::Ruby and RDoc::Parser::PrismRuby

lib/rdoc/parser/prism_ruby.rb

@@ -31,10 +31,21 @@ def initialize(top_level, content, options, stats)
     @track_visibility = :nodoc != @options.visibility
     @encoding = @options.encoding
 
-    @module_nesting = [top_level]
+    @module_nesting = [[top_level, false]]
     @container = top_level
     @visibility = :public
     @singleton = false
+    @in_proc_block = false
+  end
+
+  # Suppress `extend` and `include` within block
+  # because they might be a metaprogramming block
+  # example: `Module.new { include M }` `M.module_eval { include N }`
+
+  def with_in_proc_block
+    @in_proc_block = true
+    yield
+    @in_proc_block = false
   end
 
   # Dive into another container
@@ -43,22 +54,24 @@ def with_container(container, singleton: false)
     old_container = @container
     old_visibility = @visibility
     old_singleton = @singleton
+    old_in_proc_block = @in_proc_block
     @visibility = :public
     @container = container
     @singleton = singleton
+    @in_proc_block = false
     unless singleton
-      @module_nesting.push container
-
       # Need to update module parent chain to emulate Module.nesting.
       # This mechanism is inaccurate and needs to be fixed.
       container.parent = old_container
     end
+    @module_nesting.push([container, singleton])
     yield container
   ensure
     @container = old_container
     @visibility = old_visibility
     @singleton = old_singleton
-    @module_nesting.pop unless singleton
+    @in_proc_block = old_in_proc_block
+    @module_nesting.pop
   end
 
   # Records the location of this +container+ in the file for this parser and
@@ -204,6 +217,10 @@ def parse_comment_tomdoc(container, comment, line_no, start_line)
     @stats.add_method meth
   end
 
+  def has_modifier_nodoc?(line_no) # :nodoc:
+    @modifier_comments[line_no]&.text&.match?(/\A#\s*:nodoc:/)
+  end
+
   def handle_modifier_directive(code_object, line_no) # :nodoc:
     comment = @modifier_comments[line_no]
     @preprocess.handle(comment.text, code_object) if comment
@@ -467,6 +484,7 @@ def add_attributes(names, rw, line_no)
   end
 
   def add_includes_extends(names, rdoc_class, line_no) # :nodoc:
+    return if @in_proc_block
     comment = consecutive_comment(line_no)
     handle_consecutive_comment_directive(@container, comment)
     names.each do |name|
@@ -492,7 +510,9 @@ def add_extends(names, line_no) # :nodoc:
 
   # Adds a method defined by `def` syntax
 
-  def add_method(name, receiver_name:, receiver_fallback_type:, visibility:, singleton:, params:, calls_super:, block_params:, tokens:, start_line:, end_line:)
+  def add_method(name, receiver_name:, receiver_fallback_type:, visibility:, singleton:, params:, calls_super:, block_params:, tokens:, start_line:, args_end_line:, end_line:)
+    return if @in_proc_block
+
     receiver = receiver_name ? find_or_create_module_path(receiver_name, receiver_fallback_type) : @container
     meth = RDoc::AnyMethod.new(nil, name)
     if (comment = consecutive_comment(start_line))
@@ -504,20 +524,10 @@ def add_method(name, receiver_name:, receiver_fallback_type:, visibility:, singl
       meth.comment = comment
     end
     handle_modifier_directive(meth, start_line)
+    handle_modifier_directive(meth, args_end_line)
     handle_modifier_directive(meth, end_line)
     return unless should_document?(meth)
 
-
-    if meth.name == 'initialize' && !singleton
-      if meth.dont_rename_initialize
-        visibility = :protected
-      else
-        meth.name = 'new'
-        singleton = true
-        visibility = :public
-      end
-    end
-
     internal_add_method(
       receiver,
       meth,
@@ -529,6 +539,18 @@ def add_method(name, receiver_name:, receiver_fallback_type:, visibility:, singl
       block_params: block_params,
       tokens: tokens
     )
+
+    # Rename after add_method to register duplicated 'new' and 'initialize'
+    # defined in c and ruby just like the old parser did.
+    if meth.name == 'initialize' && !singleton
+      if meth.dont_rename_initialize
+        meth.visibility = :protected
+      else
+        meth.name = 'new'
+        meth.singleton = true
+        meth.visibility = :public
+      end
+    end
   end
 
   private def internal_add_method(container, meth, line_no:, visibility:, singleton:, params:, calls_super:, block_params:, tokens:) # :nodoc:
@@ -565,12 +587,17 @@ def find_or_create_module_path(module_name, create_mode)
     if root_name.empty?
       mod = @top_level
     else
-      @module_nesting.reverse_each do |nesting|
+      @module_nesting.reverse_each do |nesting, singleton|
+        next if singleton
         mod = nesting.find_module_named(root_name)
         break if mod
+        # If a constant is found and it is not a module or class, RDoc can't document about it.
+        # Return an anonymous module to avoid wrong document creation.
+        return RDoc::NormalModule.new(nil) if nesting.find_constant_named(root_name)
       end
-      return mod || add_module.call(@top_level, root_name, create_mode) unless name
-      mod ||= add_module.call(@top_level, root_name, :module)
+      last_nesting, = @module_nesting.reverse_each.find { |_, singleton| !singleton }
+      return mod || add_module.call(last_nesting, root_name, create_mode) unless name
+      mod ||= add_module.call(last_nesting, root_name, :module)
     end
     path.each do |name|
       mod = mod.find_module_named(name) || add_module.call(mod, name, :module)
@@ -584,7 +611,8 @@ def resolve_constant_path(constant_path)
     owner_name, path = constant_path.split('::', 2)
     return constant_path if owner_name.empty? # ::Foo, ::Foo::Bar
     mod = nil
-    @module_nesting.reverse_each do |nesting|
+    @module_nesting.reverse_each do |nesting, singleton|
+      next if singleton
       mod = nesting.find_module_named(owner_name)
       break if mod
     end
@@ -598,7 +626,10 @@ def resolve_constant_path(constant_path)
   def find_or_create_constant_owner_name(constant_path)
     const_path, colon, name = constant_path.rpartition('::')
     if colon.empty? # class Foo
-      [@container, name]
+      # Within `class C` or `module C`, owner is C(== current container)
+      # Within `class <<C`, owner is C.singleton_class
+      # but RDoc don't track constants of a singleton class of module
+      [(@singleton ? nil : @container), name]
     elsif const_path.empty? # class ::Foo
       [@top_level, name]
     else # `class Foo::Bar` or `class ::Foo::Bar`
@@ -612,6 +643,8 @@ def add_constant(constant_name, rhs_name, start_line, end_line)
     comment = consecutive_comment(start_line)
     handle_consecutive_comment_directive(@container, comment)
     owner, name = find_or_create_constant_owner_name(constant_name)
+    return unless owner
+
     constant = RDoc::Constant.new(name, rhs_name, comment)
     constant.store = @store
     constant.line = start_line
@@ -635,26 +668,29 @@ def add_constant(constant_name, rhs_name, start_line, end_line)
 
   # Adds module or class
 
-  def add_module_or_class(module_name, start_line, end_line, is_class: false, superclass_name: nil)
+  def add_module_or_class(module_name, start_line, end_line, is_class: false, superclass_name: nil, superclass_expr: nil)
     comment = consecutive_comment(start_line)
     handle_consecutive_comment_directive(@container, comment)
     return unless @container.document_children
 
     owner, name = find_or_create_constant_owner_name(module_name)
+    return unless owner
+
     if is_class
       # RDoc::NormalClass resolves superclass name despite of the lack of module nesting information.
       # We need to fix it when RDoc::NormalClass resolved to a wrong constant name
       if superclass_name
         superclass_full_path = resolve_constant_path(superclass_name)
         superclass = @store.find_class_or_module(superclass_full_path) if superclass_full_path
         superclass_full_path ||= superclass_name
+        superclass_full_path = superclass_full_path.sub(/^::/, '')
       end
       # add_class should be done after resolving superclass
-      mod = owner.classes_hash[name] || owner.add_class(RDoc::NormalClass, name, superclass_name || '::Object')
+      mod = owner.classes_hash[name] || owner.add_class(RDoc::NormalClass, name, superclass_name || superclass_expr || '::Object')
       if superclass_name
         if superclass
           mod.superclass = superclass
-        elsif mod.superclass.is_a?(String) && mod.superclass != superclass_full_path
+        elsif (mod.superclass.is_a?(String) || mod.superclass.name == 'Object') && mod.superclass != superclass_full_path
           mod.superclass = superclass_full_path
         end
       end
@@ -678,6 +714,20 @@ def initialize(scanner, top_level, store)
       @store = store
     end
 
+    def visit_if_node(node)
+      if node.end_keyword
+        super
+      else
+        # Visit with the order in text representation to handle this method comment
+        # # comment
+        # def f
+        # end if call_node
+        node.statements.accept(self)
+        node.predicate.accept(self)
+      end
+    end
+    alias visit_unless_node visit_if_node
+
     def visit_call_node(node)
       @scanner.process_comments_until(node.location.start_line - 1)
       if node.receiver.nil?
@@ -715,26 +765,35 @@ def visit_call_node(node)
         when :private_class_method
           _visit_call_public_private_class_method(node, :private) { super }
         else
+          node.arguments&.accept(self)
           super
         end
       else
         super
       end
     end
 
+    def visit_block_node(node)
+      @scanner.with_in_proc_block do
+        # include, extend and method definition inside block are not documentable
+        super
+      end
+    end
+
     def visit_alias_method_node(node)
       @scanner.process_comments_until(node.location.start_line - 1)
       return unless node.old_name.is_a?(Prism::SymbolNode) && node.new_name.is_a?(Prism::SymbolNode)
       @scanner.add_alias_method(node.old_name.value.to_s, node.new_name.value.to_s, node.location.start_line)
     end
 
     def visit_module_node(node)
+      node.constant_path.accept(self)
       @scanner.process_comments_until(node.location.start_line - 1)
       module_name = constant_path_string(node.constant_path)
       mod = @scanner.add_module_or_class(module_name, node.location.start_line, node.location.end_line) if module_name
       if mod
         @scanner.with_container(mod) do
-          super
+          node.body&.accept(self)
           @scanner.process_comments_until(node.location.end_line)
         end
       else
@@ -743,13 +802,16 @@ def visit_module_node(node)
     end
 
     def visit_class_node(node)
+      node.constant_path.accept(self)
+      node.superclass&.accept(self)
       @scanner.process_comments_until(node.location.start_line - 1)
       superclass_name = constant_path_string(node.superclass) if node.superclass
+      superclass_expr = node.superclass.slice if node.superclass && !superclass_name
       class_name = constant_path_string(node.constant_path)
-      klass = @scanner.add_module_or_class(class_name, node.location.start_line, node.location.end_line, is_class: true, superclass_name: superclass_name) if class_name
+      klass = @scanner.add_module_or_class(class_name, node.location.start_line, node.location.end_line, is_class: true, superclass_name: superclass_name, superclass_expr: superclass_expr) if class_name
       if klass
         @scanner.with_container(klass) do
-          super
+          node.body&.accept(self)
           @scanner.process_comments_until(node.location.end_line)
         end
       else
@@ -760,6 +822,12 @@ def visit_class_node(node)
     def visit_singleton_class_node(node)
       @scanner.process_comments_until(node.location.start_line - 1)
 
+      if @scanner.has_modifier_nodoc?(node.location.start_line)
+        # Skip visiting inside the singleton class. Also skips creation of node.expression as a module
+        @scanner.skip_comments_until(node.location.end_line)
+        return
+      end
+
       expression = node.expression
       expression = expression.body.body.first if expression.is_a?(Prism::ParenthesesNode) && expression.body&.body&.size == 1
 
@@ -774,9 +842,10 @@ def visit_singleton_class_node(node)
       when Prism::SelfNode
         mod = @scanner.container if @scanner.container != @top_level
       end
+      expression.accept(self)
       if mod
         @scanner.with_container(mod, singleton: true) do
-          super
+          node.body&.accept(self)
           @scanner.process_comments_until(node.location.end_line)
         end
       else
@@ -786,6 +855,7 @@ def visit_singleton_class_node(node)
 
     def visit_def_node(node)
       start_line = node.location.start_line
+      args_end_line = node.parameters&.location&.end_line || start_line
       end_line = node.location.end_line
       @scanner.process_comments_until(start_line - 1)
 
@@ -836,6 +906,7 @@ def visit_def_node(node)
         calls_super: calls_super,
         tokens: tokens,
         start_line: start_line,
+        args_end_line: args_end_line,
         end_line: end_line
       )
     ensure
@@ -944,7 +1015,7 @@ def _visit_call_public_private_protected(call_node, visibility)
         @scanner.visibility = visibility
       else # `public :foo, :bar`, `private def foo; end`
         yield
-        names = visibility_method_arguments(call_node, singleton: @scanner.singleton)
+        names = visibility_method_arguments(call_node, singleton: false)
         @scanner.change_method_visibility(names, visibility) if names
       end
     end

test/rdoc/test_rdoc_parser_prism_ruby.rb

@@ -173,6 +173,50 @@ def m2; end
     assert_equal ['m1', 'm2'], c.method_list.map(&:name)
   end
 
+  def test_open_class_with_superclass_specified_later
+    util_parser <<~RUBY
+      # file_2
+      require 'file_1'
+      class A; end
+      class B; end
+      class C; end
+    RUBY
+    _a, b, c = @top_level.classes
+    assert_equal 'Object', b.superclass
+    assert_equal 'Object', c.superclass
+
+    util_parser <<~RUBY
+      # file_1
+      class B < A; end
+      class C < Unknown; end
+    RUBY
+    assert_equal 'A', b.superclass.full_name
+    assert_equal 'Unknown', c.superclass
+  end
+
+  def test_open_class_with_superclass_specified_later_with_object_defined
+    util_parser <<~RUBY
+      # file_2
+      require 'file_1'
+      class Object; end
+      class A; end
+      class B; end
+      class C; end
+    RUBY
+    _object, _a, b, c = @top_level.classes
+    # If Object exists, superclass will be a NormalClass(Object) instead of string "Object"
+    assert_equal 'Object', b.superclass.full_name
+    assert_equal 'Object', c.superclass.full_name
+
+    util_parser <<~RUBY
+      # file_1
+      class B < A; end
+      class C < Unknown; end
+    RUBY
+    assert_equal 'A', b.superclass.full_name
+    assert_equal 'Unknown', c.superclass
+  end
+
   def test_confusing_superclass
     util_parser <<~RUBY
       module A
@@ -281,7 +325,7 @@ class Baz < (any expression)
     assert_equal ['Foo', 'Bar', 'Baz'], @top_level.classes.map(&:full_name)
     foo, bar, baz = @top_level.classes
     assert_equal foo, bar.superclass
-    assert_equal 'Object', baz.superclass unless accept_legacy_bug?
+    assert_equal '(any expression)', baz.superclass
   end
 
   def test_class_new_notnew
@@ -358,7 +402,6 @@ def new; end
     assert_equal ['DidYouMean::NameErrorCheckers::new'], mod.method_list.map(&:full_name)
   end
 
-
   def test_ghost_method
     util_parser <<~RUBY
       class Foo
@@ -534,6 +577,27 @@ def three x; end
     assert_equal @top_level, three.file
   end
 
+  def test_method_with_modifier_if_unless
+    util_parser <<~RUBY
+      class Foo
+        # my method one
+        def one
+        end if foo
+
+        # my method two
+        def two
+        end unless foo
+      end
+    RUBY
+
+    klass = @store.find_class_named 'Foo'
+    one, two = klass.method_list
+    assert_equal 'Foo#one', one.full_name
+    assert_equal 'my method one', one.comment.text.strip
+    assert_equal 'Foo#two', two.full_name
+    assert_equal 'my method two', two.comment.text.strip
+  end
+
   def test_method_toplevel
     util_parser <<~RUBY
       # comment
@@ -643,25 +707,22 @@ module A
           add_my_method :bar
         end
 
-        tap do
-          # comment baz1
+        metaprogramming do
+          # class that defines this method is unknown
           def baz1; end
         end
 
-        self.tap do
-          # comment baz2
-          def baz2; end
-        end
-
-        my_decorator def self.baz3; end
+        my_decorator def self.baz2; end
 
-        self.my_decorator def baz4; end
+        self.my_decorator def baz3; end
       end
     RUBY
     mod = @store.find_module_named 'A'
     methods = mod.method_list
-    assert_equal ['A::foo', 'A#bar', 'A#baz1', 'A#baz2', 'A::baz3', 'A#baz4'], methods.map(&:full_name)
-    assert_equal ['comment foo', 'comment bar', 'comment baz1', 'comment baz2'], methods.take(4).map { |m| m.comment.text.strip }
+    unless accept_legacy_bug?
+      assert_equal ['A::foo', 'A#bar', 'A::baz2', 'A#baz3'], methods.map(&:full_name)
+    end
+    assert_equal ['comment foo', 'comment bar'], methods.take(2).map { |m| m.comment.text.strip }
   end
 
   def test_method_yields_directive
@@ -769,14 +830,22 @@ def baz; end
 
   def test_undefined_singleton_class_defines_module
     util_parser <<~RUBY
-      class << Foo
-      end
-      class << ::Bar
+      module A
+        class << Foo
+        end
+        class << ::Bar
+        end
+        Baz1 = ''
+        class << Baz1
+        end
+        class << Baz2 # :nodoc:
+        end
       end
     RUBY
 
     modules = @store.all_modules
-    assert_equal ['Foo', 'Bar'], modules.map(&:name)
+    modules = modules.take(3) if accept_legacy_bug?
+    assert_equal ['A', 'A::Foo', 'Bar'], modules.map(&:full_name)
   end
 
   def test_singleton_class
@@ -1022,7 +1091,7 @@ def m5; end
   end
 
   def test_undocumentable_change_visibility
-    pend if accept_legacy_bug?
+    omit if accept_legacy_bug?
     util_parser <<~RUBY
       class A
         def m1; end
@@ -1039,8 +1108,25 @@ def self.m2; end
     assert_equal [:public] * 4, klass.method_list.map(&:visibility)
   end
 
+  def test_singleton_class_def_with_visibility
+    util_parser <<~RUBY
+      class A
+        class <<self
+          private def m1; end
+        end
+        private def self.m2; end
+      end
+      class <<A
+        private def m3; end
+      end
+    RUBY
+    klass = @store.find_class_named 'A'
+    assert_equal [true, true, true], klass.method_list.map(&:singleton)
+    assert_equal [:private, :public, :private], klass.method_list.map(&:visibility)
+  end
+
   def test_method_visibility_change_in_subclass
-    pend 'not implemented' if accept_legacy_bug?
+    omit 'not implemented' if accept_legacy_bug?
     util_parser <<~RUBY
       class A
         def m1; end
@@ -1151,7 +1237,7 @@ def bar; end
   end
 
   def test_invalid_alias_method
-    pend if accept_legacy_bug?
+    omit if accept_legacy_bug?
     util_parser <<~RUBY
       class Foo
         def foo; end
@@ -1305,9 +1391,12 @@ def nodoc2 # :nodoc:
         def doc3; end
         def nodoc3
         end # :nodoc:
+        def nodoc4(arg1,
+                   arg2) # :nodoc:
+        end
         def doc4; end
         # :stopdoc:
-        def nodoc4; end
+        def nodoc5; end
       end
     RUBY
     klass = @store.find_class_named 'Foo'
@@ -1549,6 +1638,37 @@ class Bar; end
     assert_equal 'Foo::C', klass.find_module_named('C').full_name
   end
 
+  def test_constant_with_singleton_class
+    omit if accept_legacy_bug?
+    util_parser <<~RUBY
+      class Foo
+        class Bar; end
+        A = 1
+        class <<Bar
+          B = 1
+          Foo::Bar::B2 = 1
+        end
+        class Bar
+          C = 1
+        end
+        class <<(p(D = 1))
+          E = 1
+        end
+        class (F = 1)::Baz
+          G = 1
+        end
+        module (H = 1)::Baz
+          I = 1
+        end
+      end
+      J = 1
+    RUBY
+    foo, bar = @store.all_classes
+    assert_equal ['J'], @store.find_class_named('Object').constants.map(&:name)
+    assert_equal ['A', 'D', 'F', 'H'], foo.constants.map(&:name)
+    assert_equal ['B2', 'C'], bar.constants.map(&:name)
+  end
+
   def test_constant_method
     util_parser <<~RUBY
       def Object.foo; end
@@ -1608,7 +1728,7 @@ module M
   end
 
   def test_include_extend_to_singleton_class
-    pend 'not implemented' if accept_legacy_bug?
+    omit 'not implemented' if accept_legacy_bug?
     util_parser <<~RUBY
       class Foo
         class << self
@@ -1657,7 +1777,7 @@ class C::D::Foo
   end
 
   def test_various_argument_include
-    pend 'not implemented' if accept_legacy_bug?
+    omit 'not implemented' if accept_legacy_bug?
     util_parser <<~RUBY
       module A; end
       module B; end
@@ -1881,6 +2001,35 @@ def d; end
     assert_equal ['a', 'f'], mod.method_list.map(&:name)
   end
 
+  def test_include_extend_suppressed_within_block
+    util_parser <<~RUBY
+      module M; end
+      module N; end
+      module O: end
+      class A
+        metaprogramming do
+          include M
+          extend N
+          class B
+            include M
+            extend N
+          end
+          include M
+          extend N
+        end
+        include O
+        extend O
+      end
+    RUBY
+    a, b = @store.all_classes
+    unless accept_legacy_bug?
+      assert_equal ['O'], a.includes.map(&:name)
+      assert_equal ['O'], a.extends.map(&:name)
+    end
+    assert_equal ['M'], b.includes.map(&:name)
+    assert_equal ['N'], b.extends.map(&:name)
+  end
+
   def test_multibyte_method_name
     content = <<~RUBY
       class Foo

test/rdoc/test_rdoc_store.rb

@@ -66,6 +66,10 @@ def setup
     @mod.record_location @top_level
   end
 
+  def using_prism_ruby_parser?
+    RDoc::Parser::Ruby.name == 'RDoc::Parser::PrismRuby'
+  end
+
   def teardown
     super
 
@@ -161,13 +165,17 @@ def test_add_file_relative
 
   def test_all_classes_and_modules
     expected = %w[
-      C1 C10 C10::C11 C11 C2 C2::C3 C2::C3::H1 C3 C3::H1 C3::H2 C4 C4::C4 C5 C5::C1 C6 C7 C8 C8::S1 C9 C9::A C9::B
+      C1 C10 C10::C11 C11 C2 C2::C3 C2::C3::H1 C3 C3::H1 C3::H2 C4 C4::C4 C5 C5::C1 C6 C7 C8 C9 C9::A C9::B
       Child
       M1 M1::M2
       Object
       Parent
     ]
 
+    # C8::S1 does not exist. It should not be in the list.
+    # class C8; class << something; class S1; end; end; end
+    expected = (expected + ['C8::S1']).sort unless using_prism_ruby_parser?
+
     assert_equal expected,
                  @store.all_classes_and_modules.map { |m| m.full_name }.sort
   end
@@ -213,12 +221,16 @@ def test_class_path
 
   def test_classes
     expected = %w[
-      C1 C10 C10::C11 C11 C2 C2::C3 C2::C3::H1 C3 C3::H1 C3::H2 C4 C4::C4 C5 C5::C1 C6 C7 C8 C8::S1 C9 C9::A C9::B
+      C1 C10 C10::C11 C11 C2 C2::C3 C2::C3::H1 C3 C3::H1 C3::H2 C4 C4::C4 C5 C5::C1 C6 C7 C8 C9 C9::A C9::B
       Child
       Object
       Parent
     ]
 
+    # C8::S1 does not exist. It should not be in the list.
+    # class C8; class << something; class S1; end; end; end
+    expected = (expected + ['C8::S1']).sort unless using_prism_ruby_parser?
+
     assert_equal expected, @store.all_classes.map { |m| m.full_name }.sort
   end
 
@@ -550,6 +562,13 @@ def test_load_class
   end
 
   def test_load_single_class
+    if using_prism_ruby_parser?
+      # Class defined inside singleton class is not documentable.
+      # @c8_s1 should be nil because C8::S1 does not exist.
+      assert_nil @c8_s1
+      return
+    end
+
     @s.save_class @c8_s1
     @s.classes_hash.clear
 

test/rdoc/xref_data.rb

@@ -110,6 +110,7 @@ class C7
 
 class C8
   class << self
+    # This is C8.singleton_class::S1. C8::S1 does not exist.
     class S1
     end
   end