From b426663e089d7b6dea9c1318285f408936815458 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 28 Aug 2026 10:23:52 -0400 Subject: [PATCH 1/4] Less strict parsing of visibility method arguments This updates method name argument parsing for the visibility methods: * `private`, `public`, `protected` * `private_class_method`, `public_class_method` * `private_constant`, `public_constant` * `module_function` Prior to this commit, the parser is stricter about visibility method name arguments than it should be: it only parses method names when _all_ arguments are symbols. This updates the prism parser to behave more like the rdoc 7.2, so every (non-interpolated) string and symbol in the argument list is used (`call_node_name_arguments` is used to parse the arguments list). --- lib/rdoc/parser/ruby.rb | 16 ++++++++-------- test/rdoc/parser/ruby_test.rb | 32 +++++++++++++++++++------------- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index a4d363082c..5e47dc14bd 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -1229,10 +1229,10 @@ 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 - # module_function :foo, :bar - return symbols.map(&:to_s) + names = call_node_name_arguments(call_node) + if names + # module_function :foo, "bar" + return names else return unless arguments_node.arguments.size == 1 arg = arguments_node.arguments.first @@ -1322,14 +1322,14 @@ 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 + names = call_node_name_arguments(call_node) + @scanner.container.set_constant_visibility_for(names, :public) if names 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 + names = call_node_name_arguments(call_node) + @scanner.container.set_constant_visibility_for(names, :private) if names end def _visit_call_attr_reader_writer_accessor(call_node, rw) diff --git a/test/rdoc/parser/ruby_test.rb b/test/rdoc/parser/ruby_test.rb index e3e7fd712f..81a3e36dfc 100644 --- a/test/rdoc/parser/ruby_test.rb +++ b/test/rdoc/parser/ruby_test.rb @@ -1372,7 +1372,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 @@ -1391,8 +1391,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 @@ -1410,8 +1410,8 @@ 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 @@ -1419,8 +1419,8 @@ 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' @@ -1435,7 +1435,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 @@ -1444,7 +1445,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 @@ -1490,10 +1494,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 @@ -1943,8 +1948,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' @@ -2706,6 +2711,7 @@ def foo; end tap do end def foo; end module_function :foo + def foo; end end def bar; end module_function :bar From 5d3c8cf442d86a7d854a2829166d9ace2a25618b Mon Sep 17 00:00:00 2001 From: nick evans Date: Sun, 30 Aug 2026 14:12:01 -0400 Subject: [PATCH 2/4] Fix Parser::Ruby docs for `*_class_method` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There aren't any `private_class_function` and `public_class_function` methods. 😉 --- lib/rdoc/parser/ruby.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index 5e47dc14bd..e6f83f9157 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -18,7 +18,7 @@ class Parser # * 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 From e30f6d651eb49ac604de0bd47195d070b27e7e39 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sun, 30 Aug 2026 19:59:49 -0400 Subject: [PATCH 3/4] Refactor Parser::Ruby#call_node_name_arguments Arguably, these methods belong more to the visitor than the "scanner". Since they simply process prism nodes without any ivar references, they should probably be converted into module functions on a utility module. --- lib/rdoc/parser/ruby.rb | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index e6f83f9157..2298e27f5a 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -388,15 +388,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 @@ -412,7 +420,7 @@ def handle_meta_method_comment(comment, directives, node) case directive when 'attr', 'attr_reader', 'attr_writer', 'attr_accessor' attributes = [param] if param - attributes ||= call_node_name_arguments(node).compact if is_call_node + attributes ||= call_node_name_arguments(node) if is_call_node rw = directive == 'attr_writer' ? 'W' : directive == 'attr_accessor' ? 'RW' : 'R' when 'method' method_name = param if param @@ -438,7 +446,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 line_no = node.location.start_line if node internal_add_method( method_name, @@ -1216,8 +1224,7 @@ def constant_arguments_names(call_node) end def call_node_name_arguments(call_node) - names = @scanner.call_node_name_arguments(call_node).compact - names unless names.empty? + @scanner.call_node_name_arguments(call_node) end def symbol_arguments(call_node) From 2c3972c93bacca385dc446d7f1f6b590cff42fa6 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 11 Sep 2026 13:50:11 -0400 Subject: [PATCH 4/4] Address review comment This addresses @tompng's comment here, and uses his suggested fix: https://github.com/ruby/rdoc/pull/1803#discussion_r3989476271 I'd previously reasoned that the add method clause would only be entered when a `method` or `singleton-method` directive is seen. I missed that `node` is sent in as a parameter, so it can _also_ also trigger the other clause. The issue this seems to cause is that unparseable attributes directives will be interpreted as ghost methods, and ghost methods with no known name behave differently from ghost attributes with no known name in that they are still created with an "unknown" name. I've added some examples to `test_invalid_meta_method` which demonstrated this bug. Co-authored-by: tomoya ishida --- lib/rdoc/parser/ruby.rb | 2 +- test/rdoc/parser/ruby_test.rb | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index 2298e27f5a..ab0544e0b4 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -420,7 +420,7 @@ def handle_meta_method_comment(comment, directives, node) case directive when 'attr', 'attr_reader', 'attr_writer', 'attr_accessor' attributes = [param] if param - attributes ||= call_node_name_arguments(node) if is_call_node + attributes ||= call_node_name_arguments(node) || [] if is_call_node rw = directive == 'attr_writer' ? 'W' : directive == 'attr_accessor' ? 'RW' : 'R' when 'method' method_name = param if param diff --git a/test/rdoc/parser/ruby_test.rb b/test/rdoc/parser/ruby_test.rb index 81a3e36dfc..4ac663abb4 100644 --- a/test/rdoc/parser/ruby_test.rb +++ b/test/rdoc/parser/ruby_test.rb @@ -865,6 +865,24 @@ class Foo # :attr-accessor: arw3 add_my_attribute_arw3 + + # Unparsable attributes should not parse as "unknown" methods + + ## + # :attr: + add_my_attribute_a4("foo" + "bar") + + ## + # :attr_reader: + add_my_attribute_ar4("foo" + "bar") + + ## + # :attr_writer: + add_my_attribute_aw4("foo" + "bar") + + ## + # :attr_accessor: + add_my_attribute_arw4("foo" + "bar") end RUBY