From cc11a609ee997007a51cc86c0a9174e78a29c6c2 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 28 Aug 2026 10:23:52 -0400 Subject: [PATCH 1/5] Allow less strict attribute argument parsing This updates argument parsing for the `attr`, `attr_reader`, `attr_writer`, and `attr_accessor` methods, so they behave more like rdoc 7.2's parser. The prism parser is strict about attribute arguments: it only parses as an attribute when _all_ arguments are symbols. rdoc 7.2's parser allowed all symbol or string arguments and ignored the rest. As an example, the rdoc for `Net::IMAP::Config` intentionally took advantage of the looser parsing done by rdoc 7.2. That class redefines `attr_reader`, `attr_writer` and `attr_accessor` to add keyword arguments for type validation/coercion and defaults: ```ruby # Seconds to wait until a connection is opened. # # Applied separately for establishing TCP connection and starting a TLS # connection. # # If the IMAP object cannot open a connection within this time, # it raises a Net::OpenTimeout exception. # # See Net::IMAP.new and Net::IMAP#starttls. # # The default value is +30+ seconds. attr_accessor :open_timeout, type: Integer, default: 30 ``` rdoc 7.2 simply ignored the unknown keyword args, and parses this no differently from `attr_accessor :open_timeout.` Fixes #1790. --- lib/rdoc/parser/ruby.rb | 9 +++++-- test/rdoc/parser/ruby_test.rb | 44 ++++++++++++++++++++++++++++------- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index 6e943e5d6d..0e7f616936 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -1232,6 +1232,11 @@ def constant_arguments_names(call_node) names.all? ? names : nil end + def call_node_name_arguments(call_node) + names = @scanner.call_node_name_arguments(call_node).compact + names unless names.empty? + 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)} @@ -1346,8 +1351,8 @@ def _visit_call_private_constant(call_node) 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: diff --git a/test/rdoc/parser/ruby_test.rb b/test/rdoc/parser/ruby_test.rb index 3ebfd5891b..b5ead1eb2e 100644 --- a/test/rdoc/parser/ruby_test.rb +++ b/test/rdoc/parser/ruby_test.rb @@ -1580,14 +1580,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 +1614,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 From f4607d26d8db4ca8d96b819028bf083e8ad5f5c4 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sun, 30 Aug 2026 14:32:16 -0400 Subject: [PATCH 2/5] Don't create meta attributes with no name This updates inferred attribute name parsing for the `:attr:`, `:attr_reader:`, `:attr_writer:`, and `:attr_accessor:` directives, to avoid creating unnamed attributes from unparsable arguments. Previously, an unnamed (nil) attribute would be created. Now, any non-string/non-symbol arguments are simply ignored. --- lib/rdoc/parser/ruby.rb | 2 +- test/rdoc/parser/ruby_test.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index 0e7f616936..c7a59a7cde 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -413,7 +413,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).compact 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 b5ead1eb2e..a57f77d398 100644 --- a/test/rdoc/parser/ruby_test.rb +++ b/test/rdoc/parser/ruby_test.rb @@ -1755,19 +1755,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: From 7e1a88db37cece5b99b0c8ee6d69955441cf49e5 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 28 Aug 2026 10:23:52 -0400 Subject: [PATCH 3/5] 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 | 13 ++++++------- test/rdoc/parser/ruby_test.rb | 32 +++++++++++++++++++------------- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index c7a59a7cde..76905556c4 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -1246,10 +1246,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 @@ -1339,14 +1338,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 + 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) diff --git a/test/rdoc/parser/ruby_test.rb b/test/rdoc/parser/ruby_test.rb index a57f77d398..90e7159689 100644 --- a/test/rdoc/parser/ruby_test.rb +++ b/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,8 +1385,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 @@ -1394,8 +1394,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' @@ -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 @@ -1918,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' @@ -2681,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 From a7f4df8db7a4272562c863e4e986ac225c61e558 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sun, 30 Aug 2026 14:12:01 -0400 Subject: [PATCH 4/5] 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 76905556c4..d368a35e78 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/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 From 242a1e9a28d2c5abdc261e7141bd26ba2999e202 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sun, 30 Aug 2026 19:59:49 -0400 Subject: [PATCH 5/5] 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 d368a35e78..7e321165ee 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -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 @@ -413,7 +421,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 @@ -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 @@ -1233,8 +1241,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)