Less strict parsing of visibility method arguments

lib/rdoc/parser/ruby.rb

@@ -16,7 +16,7 @@
 # * constants
 # * aliases
 # * private, public, protected
-# * private_class_function, public_class_function
+# * private_class_method, public_class_method
 # * private_constant, public_constant
 # * module_function
 # * attr, attr_reader, attr_writer, attr_accessor
@@ -389,15 +389,23 @@ def handle_modifier_directive(code_object, line_no) # :nodoc:
   end
 
   def call_node_name_arguments(call_node) # :nodoc:
-    return [] unless call_node.arguments
-    call_node.arguments.arguments.map do |arg|
-      case arg
-      when Prism::SymbolNode
-        arg.value
-      when Prism::StringNode
-        arg.unescaped
-      end
-    end || []
+    return unless arguments_node = call_node.arguments
+    names = arguments_node.arguments.filter_map { |arg| argument_name(arg) }
+    names unless names.empty?
+  end
+
+  def call_node_name_argument(call_node) # :nodoc:
+    return unless call_node.arguments
+    argument_name(call_node.arguments.arguments.first)
+  end
+
+  def argument_name(argument_node) # :nodoc:
+    case argument_node
+    when Prism::SymbolNode
+      argument_node.value
+    when Prism::StringNode
+      argument_node.unescaped
+    end
   end
 
   # Handles meta method comments
@@ -439,7 +447,7 @@ def handle_meta_method_comment(comment, directives, node)
         mark_container_documentable(@container)
       end
     elsif line_no || node
-      method_name ||= call_node_name_arguments(node).first if is_call_node
+      method_name ||= call_node_name_argument(node) if is_call_node
       if node
         tokens = syntax_highlighted_tokens(node)
         line_no = node.location.start_line
@@ -1232,6 +1240,10 @@ def constant_arguments_names(call_node)
       names.all? ? names : nil
     end
 
+    def call_node_name_arguments(call_node)
+      @scanner.call_node_name_arguments(call_node)
+    end
+
     def symbol_arguments(call_node)
       arguments_node = call_node.arguments
       return unless arguments_node && arguments_node.arguments.all? { |arg| arg.is_a?(Prism::SymbolNode)}
@@ -1241,10 +1253,9 @@ def symbol_arguments(call_node)
     def visibility_method_arguments(call_node, singleton:)
       arguments_node = call_node.arguments
       return unless arguments_node
-      symbols = symbol_arguments(call_node)
-      if symbols
+      if (names = call_node_name_arguments(call_node))
         # module_function :foo, :bar
-        return symbols.map(&:to_s)
+        return names
       else
         return unless arguments_node.arguments.size == 1
         arg = arguments_node.arguments.first
@@ -1334,20 +1345,20 @@ def _visit_call_extend(call_node)
 
     def _visit_call_public_constant(call_node)
       return if @scanner.in_proc_block || @scanner.singleton
-      names = symbol_arguments(call_node)
-      @scanner.container.set_constant_visibility_for(names.map(&:to_s), :public) if names
+      return unless names = call_node_name_arguments(call_node)
+      @scanner.container.set_constant_visibility_for(names, :public)
     end
 
     def _visit_call_private_constant(call_node)
       return if @scanner.in_proc_block || @scanner.singleton
-      names = symbol_arguments(call_node)
-      @scanner.container.set_constant_visibility_for(names.map(&:to_s), :private) if names
+      return unless names = call_node_name_arguments(call_node)
+      @scanner.container.set_constant_visibility_for(names, :private)
     end
 
     def _visit_call_attr_reader_writer_accessor(call_node, rw)
       return if @scanner.in_proc_block
-      names = symbol_arguments(call_node)
-      @scanner.add_attributes(names.map(&:to_s), rw, call_node.location.start_line) if names
+      return unless names = call_node_name_arguments(call_node)
+      @scanner.add_attributes(names, rw, call_node.location.start_line)
     end
 
     class MethodSignatureVisitor < Prism::Visitor # :nodoc:

test/rdoc/parser/ruby_test.rb

@@ -1347,7 +1347,7 @@ module A
         def m1; end
         def m2; end
         def m3; end
-        module_function :m1, :m3
+        module_function :m1, "m3", kwarg: ignored
         module_function def m4; end
       end
     RUBY
@@ -1366,8 +1366,8 @@ class A
         def self.m1; end
         def self.m2; end
         def self.m3; end
-        private_class_method :m1, :m2
-        public_class_method :m1, :m3
+        private_class_method ignored, :m1, "m2"
+        public_class_method :m1, ignored, :m3
         private_class_method def self.m4; end
         public_class_method def self.m5; end
       end
@@ -1385,17 +1385,17 @@ def m2; end
         def m3; end
         def m4; end
         def m5; end
-        private :m2, :m3, :m4
-        public :m1, :m3
+        private :m2, :m3, "m4", kwarg: :ignored
+        public :m1, ignored, :m3
       end
       class << A
         def m1; end
         def m2; end
         def m3; end
         def m4; end
         def m5; end
-        private :m1, :m2, :m3
-        public :m2, :m4
+        private :m1, :m2, "m3"
+        public ignored, :m2, :m4
       end
     RUBY
     klass = @store.find_class_named 'A'
@@ -1410,7 +1410,8 @@ def test_undocumentable_change_visibility
       class A
         def m1; end
         def self.m2; end
-        private 42, :m # maybe not Module#private
+        def m3; end
+        private 42, "m3"
         # ignore all non-standard `private def` and `private_class_method def`
         private def self.m1; end
         private_class_method def m2; end
@@ -1419,7 +1420,10 @@ def self.m2; end
       end
     RUBY
     klass = @store.find_class_named 'A'
-    assert_equal [:public] * 4, klass.method_list.map(&:visibility)
+    singleton_methods, instance_methods = klass.method_list.partition(&:singleton)
+      .map { |methods| methods.to_h { |m| [m.name, m.visibility] } }
+    assert_equal({'m1' => :public, 'm3' => :private, 'm2' => :public}, instance_methods)
+    assert_equal({'m2' => :public, 'm1' => :public}, singleton_methods)
   end
 
   def test_singleton_class_def_with_visibility
@@ -1465,10 +1469,11 @@ def test_singleton_method_visibility_change_in_subclass
       class A
         def self.m1; end
         def self.m2; end
-        private_class_method :m2
+        ignored = :m1
+        private_class_method ignored, "m2"
       end
       class B < A
-        private_class_method :m1
+        private_class_method :m1, ignored
         public_class_method :m2
       end
     RUBY
@@ -1580,14 +1585,14 @@ class Foo
         # attrs
         attr :attr1, :attr2
         # readers
-        attr_reader :reader1, :reader2
+        attr_reader :reader1, "reader2"
         # writers
-        attr_writer :writer1, :writer2
+        attr_writer "writer1", :writer2
         # accessors
         attr_accessor :accessor1, :accessor2
         # :stopdoc:
         attr :attr3, :attr4
-        attr_reader :reader3, :reader4
+        attr_reader :reader3, "reader4"
         attr_writer :write3, :writer4
         attr_accessor :accessor3, :accessor4
       end
@@ -1614,16 +1619,44 @@ class Foo
     assert_equal [@top_level] * 8, [a1, a2, r1, r2, w1, w2, rw1, rw2].map(&:file)
   end
 
-  def test_undocumentable_attributes
+  def test_ignored_undocumentable_attributes
     util_parser <<~RUBY
       class Foo
-        attr
-        attr 42, :foo
+        # attrs
+        attr :attr1, *ignored1, :attr2, (ignored2), kwarg: :ignored3
+        # readers
+        attr_reader ignored3, :reader1, ignored4, :reader2, kw: ignored5
+        # writers
+        attr_writer :writer1, *%i[ignored6], :writer2, kwarg: :ignored7
+        # accessors
+        attr_accessor ignored8, :accessor1, (:ignored9), :accessor2, kw: :ignored10
+        # ignored
+        attr          ignored11
+        attr_reader   ignored12
+        attr_writer   ignored13
+        attr_accessor ignored14
       end
     RUBY
     klass = @store.find_class_named 'Foo'
-    assert_empty klass.method_list
-    assert_empty klass.attributes
+    assert_equal 8, klass.attributes.size
+    a1, a2, r1, r2, w1, w2, rw1, rw2 = klass.attributes
+    assert_equal ['attr1', 'attr2'], [a1.name, a2.name]
+    assert_equal ['reader1', 'reader2'], [r1.name, r2.name]
+    assert_equal ['writer1', 'writer2'], [w1.name, w2.name]
+    assert_equal ['accessor1', 'accessor2'], [rw1.name, rw2.name]
+    assert_equal ['R', 'R'], [a1.rw, a2.rw]
+    assert_equal ['R', 'R'], [r1.rw, r2.rw]
+    assert_equal ['W', 'W'], [w1.rw, w2.rw]
+    assert_equal ['RW', 'RW'], [rw1.rw, rw2.rw]
+    assert_equal ['attrs', 'attrs'], [a1.comment.text, a2.comment.text]
+    assert_equal ['readers', 'readers'], [r1.comment.text, r2.comment.text]
+    assert_equal ['writers', 'writers'], [w1.comment.text, w2.comment.text]
+    assert_equal ['accessors', 'accessors'], [rw1.comment.text, rw2.comment.text]
+    assert_equal [3, 3], [a1.line, a2.line]
+    assert_equal [5, 5], [r1.line, r2.line]
+    assert_equal [7, 7], [w1.line, w2.line]
+    assert_equal [9, 9], [rw1.line, rw2.line]
+    assert_equal [@top_level] * 8, [a1, a2, r1, r2, w1, w2, rw1, rw2].map(&:file)
   end
 
   def test_singleton_class_attributes
@@ -1727,19 +1760,19 @@ class Foo
         ##
         # :attr:
         # attrs
-        add_my_method :attr1, :attr2
+        add_my_method :attr1, "attr2", (ignored)
         ##
         # :attr_reader:
         # readers
-        add_my_method :reader1, :reader2
+        add_my_method :reader1, ignored, "reader2"
         ##
         # :attr_writer:
         # writers
-        add_my_method :writer1, :writer2
+        add_my_method :writer1, :writer2, kwarg: ignored
         ##
         # :attr_accessor:
         # accessors
-        add_my_method :accessor1, :accessor2
+        add_my_method ignored, :accessor1, :accessor2
 
         # :stopdoc:
 
@@ -1890,8 +1923,8 @@ class C
         private_constant
         private_constant foo
         private_constant :A
-        private_constant :B, :C
-        public_constant :B
+        private_constant :B, bar, :C
+        public_constant baz, "B"
       end
     RUBY
     klass = @store.find_class_named 'C'
@@ -2653,6 +2686,7 @@ def foo; end
           tap do end
           def foo; end
           module_function :foo
+          def foo; end
         end
         def bar; end
         module_function :bar