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

Author: tompng

lib/rdoc/parser/prism_ruby.rb

@@ -204,6 +204,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
@@ -568,9 +572,12 @@ def find_or_create_module_path(module_name, create_mode)
       @module_nesting.reverse_each do |nesting|
         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)
+      return mod || add_module.call(@module_nesting.last, root_name, create_mode) unless name
+      mod ||= add_module.call(@module_nesting.last, root_name, :module)
     end
     path.each do |name|
       mod = mod.find_module_named(name) || add_module.call(mod, name, :module)
@@ -635,7 +642,7 @@ 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
@@ -650,7 +657,7 @@ def add_module_or_class(module_name, start_line, end_line, is_class: false, supe
         superclass_full_path ||= superclass_name
       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
@@ -678,6 +685,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?
@@ -745,8 +766,9 @@ def visit_module_node(node)
     def visit_class_node(node)
       @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
@@ -760,6 +782,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
 
@@ -944,7 +972,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

@@ -281,7 +281,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
@@ -534,6 +534,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
@@ -769,14 +790,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
@@ -1039,6 +1068,23 @@ 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?
     util_parser <<~RUBY