From 562f8f2776deb117dbff316ac1677edeb027f4b8 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Tue, 20 Oct 2026 00:24:25 +0200 Subject: [PATCH 1/3] Fix bare module_function handling in the Ruby parser The Prism-based Ruby parser ignored module_function without arguments, documenting subsequent methods only as public instance methods. This affected APIs such as JSON.parse, whose public singleton methods were missing from generated documentation and downstream API lookups. --- lib/rdoc/parser/ruby.rb | 26 ++++- test/rdoc/parser/ruby_test.rb | 186 ++++++++++++++++++++++++++++++++++ 2 files changed, 208 insertions(+), 4 deletions(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index ab0544e0b4..1ba585f8e5 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -133,7 +133,7 @@ class Ruby < Parser # Matches an RBS inline type annotation line: #: followed by whitespace RBS_SIG_LINE = /\A#:\s/ # :nodoc: - attr_accessor :visibility + attr_accessor :visibility, :module_function_mode attr_reader :container, :singleton, :in_proc_block def initialize(top_level, content, options, stats) @@ -153,6 +153,7 @@ def initialize(top_level, content, options, stats) @module_nesting = [[top_level, false]] @container = top_level @visibility = :public + @module_function_mode = false @singleton = false @in_proc_block = false @doc_state = :startdoc @@ -220,10 +221,12 @@ def with_in_proc_block def with_container(container, singleton: false) old_container = @container old_visibility = @visibility + old_module_function_mode = @module_function_mode old_singleton = @singleton old_in_proc_block = @in_proc_block old_doc_state = @doc_state @visibility = :public + @module_function_mode = false @container = container @singleton = singleton @in_proc_block = false @@ -232,6 +235,7 @@ def with_container(container, singleton: false) ensure @container = old_container @visibility = old_visibility + @module_function_mode = old_module_function_mode @singleton = old_singleton @in_proc_block = old_in_proc_block @doc_state = old_doc_state @@ -718,6 +722,7 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility: end private def internal_add_method(method_name, container, comment:, directives:, modifier_comment_lines: nil, line_no:, visibility:, singleton:, params:, calls_super:, block_params:, node_id:, type_signature_lines: nil) # :nodoc: + module_function = @module_function_mode && !singleton && container.equal?(@container) meth = AnyMethod.new(method_name, singleton: singleton) meth.comment = comment handle_code_object_directives(meth, directives) if directives @@ -743,7 +748,7 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility: meth.type_signature_lines = type_signature_lines # An instance method `initialize` is documented as `::new` unless the # :notnew: directive is given - if method_name == 'initialize' && !singleton + if method_name == 'initialize' && !singleton && !module_function if meth.dont_rename_initialize meth.visibility = :protected else @@ -757,6 +762,13 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility: container.add_method(meth) token_stream_loader = @colorizer_context.token_stream_loader(node_id) if node_id meth.start_collecting_tokens(:ruby, loader: token_stream_loader) + + if module_function + singleton_method = meth.dup + singleton_method.singleton = true + singleton_method.visibility = :public + container.add_method(singleton_method) + end end # Find or create module or class from a given module name using Ruby lexical @@ -1277,8 +1289,13 @@ def _visit_call_require(call_node) def _visit_call_module_function(call_node) return if @scanner.in_proc_block || @scanner.singleton - names = visibility_method_arguments(call_node, singleton: false)&.map(&:to_s) - @scanner.change_method_to_module_function(names) if names + if call_node.arguments.nil? + @scanner.visibility = :private + @scanner.module_function_mode = true + else + names = visibility_method_arguments(call_node, singleton: false)&.map(&:to_s) + @scanner.change_method_to_module_function(names) if names + end end def _visit_call_public_private_class_method(call_node, visibility) @@ -1292,6 +1309,7 @@ def _visit_call_public_private_protected(call_node, visibility) arguments_node = call_node.arguments if arguments_node.nil? # `public` `private` @scanner.visibility = visibility + @scanner.module_function_mode = false else # `public :foo, :bar`, `private def foo; end` names = visibility_method_arguments(call_node, singleton: false) @scanner.change_method_visibility(names, visibility) if names diff --git a/test/rdoc/parser/ruby_test.rb b/test/rdoc/parser/ruby_test.rb index 4ac663abb4..2ec26ff9f7 100644 --- a/test/rdoc/parser/ruby_test.rb +++ b/test/rdoc/parser/ruby_test.rb @@ -1403,6 +1403,191 @@ def m3; end assert_equal [:public, :public, :public], singleton_methods.map(&:visibility) end + def test_module_function_without_arguments + util_parser <<~RUBY + module JSON + def before; end + module_function + + # :call-seq: + # JSON.parse(source, opts = {}) -> object + # + # Parses a source. + def parse(source, opts = {}); end + def generate(object); end + end + RUBY + mod = @store.find_module_named 'JSON' + instance_methods = mod.method_list.reject(&:singleton) + singleton_methods = mod.method_list.select(&:singleton) + + assert_equal ['before', 'parse', 'generate'], instance_methods.map(&:name) + assert_equal [:public, :private, :private], instance_methods.map(&:visibility) + assert_equal ['parse', 'generate'], singleton_methods.map(&:name) + assert_equal [:public, :public], singleton_methods.map(&:visibility) + + parse = mod.find_method('parse', false) + singleton_parse = mod.find_method('parse', true) + assert_not_same parse, singleton_parse + assert_equal '(source, opts = {})', singleton_parse.params + assert_equal 'Parses a source.', singleton_parse.comment.text.strip + assert_equal 'JSON.parse(source, opts = {}) -> object', singleton_parse.call_seq + assert_equal 'JSON::parse', singleton_parse.full_name + assert_equal 'method-c-parse', singleton_parse.aref + assert_equal parse.token_stream, singleton_parse.token_stream + end + + def test_module_function_without_arguments_reset_by_visibility + util_parser <<~RUBY + module M + module_function + def first; end + public + def pub; end + module_function + def second; end + private + def pri; end + module_function + def third; end + protected + def pro; end + end + RUBY + mod = @store.find_module_named 'M' + instance_methods = mod.method_list.reject(&:singleton) + + assert_equal ['first', 'pub', 'second', 'pri', 'third', 'pro'], instance_methods.map(&:name) + assert_equal [:private, :public, :private, :private, :private, :protected], instance_methods.map(&:visibility) + assert_equal ['first', 'second', 'third'], mod.class_method_list.map(&:name) + end + + def test_module_function_without_arguments_preserved_by_named_visibility + util_parser <<~RUBY + module M + def before; end + module_function + def first; end + public :first + private :before + def second; end + module_function :before + public def third; end + def fourth; end + end + RUBY + mod = @store.find_module_named 'M' + instance_methods = mod.method_list.reject(&:singleton) + + assert_equal ['before', 'first', 'second', 'third', 'fourth'], instance_methods.map(&:name) + assert_equal [:private, :public, :private, :public, :private], instance_methods.map(&:visibility) + assert_equal ['first', 'second', 'before', 'third', 'fourth'], mod.class_method_list.map(&:name) + assert_equal [:public] * 5, mod.class_method_list.map(&:visibility) + end + + def test_module_function_without_arguments_scoped + util_parser <<~RUBY + module M + module_function + module Nested + def ordinary; end + module_function + def nested; end + end + class C + def ordinary; end + end + class << self + private + def singleton; end + end + def outer; end + end + module M + def reopened; end + end + module Other + def ordinary; end + end + RUBY + mod = @store.find_module_named 'M' + nested = @store.find_module_named 'M::Nested' + klass = @store.find_class_named 'M::C' + other = @store.find_module_named 'Other' + + assert_equal ['singleton', 'outer'], mod.class_method_list.map(&:name) + assert_equal [:private, :public], mod.class_method_list.map(&:visibility) + assert_equal :private, mod.find_method('outer', false).visibility + assert_equal :public, mod.find_method('reopened', false).visibility + assert_equal ['nested'], nested.class_method_list.map(&:name) + assert_equal :public, nested.find_method('ordinary', false).visibility + assert_equal :private, nested.find_method('nested', false).visibility + assert_equal [false], klass.method_list.map(&:singleton) + assert_equal [:public], klass.method_list.map(&:visibility) + assert_equal [false], other.method_list.map(&:singleton) + assert_equal [:public], other.method_list.map(&:visibility) + end + + def test_module_function_without_arguments_explicit_receivers_and_attributes + util_parser <<~RUBY + class NilClass; end + module M + module_function + def self.explicit; end + def M.named; end + def nil.literal; end + attr_reader :reader + def implicit; end + end + RUBY + mod = @store.find_module_named 'M' + nil_class = @store.find_class_named 'NilClass' + + assert_equal ['explicit', 'named', 'implicit'], mod.class_method_list.map(&:name) + assert_equal [:public] * 3, mod.class_method_list.map(&:visibility) + assert_equal ['implicit'], mod.method_list.reject(&:singleton).map(&:name) + assert_equal ['reader'], mod.attributes.map(&:name) + assert_equal [false], mod.attributes.map(&:singleton) + assert_equal [:private], mod.attributes.map(&:visibility) + assert_equal [false], nil_class.method_list.map(&:singleton) + assert_equal [:public], nil_class.method_list.map(&:visibility) + end + + def test_module_function_without_arguments_initialize + util_parser <<~RUBY + module M + module_function + def initialize(value); end + end + RUBY + mod = @store.find_module_named 'M' + + assert_equal ['initialize', 'initialize'], mod.method_list.map(&:name) + assert_equal [false, true], mod.method_list.map(&:singleton) + assert_equal [:private, :public], mod.method_list.map(&:visibility) + end + + def test_module_function_without_arguments_respects_documentation_directives + util_parser <<~RUBY + module M + module_function + def visible; end + def omitted; end # :nodoc: + # :stopdoc: + def hidden; end + # :startdoc: + def resumed; end + # :enddoc: + def ended; end + end + RUBY + mod = @store.find_module_named 'M' + + assert_equal ['visible', 'visible', 'resumed', 'resumed'], mod.method_list.map(&:name) + assert_equal [false, true, false, true], mod.method_list.map(&:singleton) + assert_equal [:private, :public, :private, :public], mod.method_list.map(&:visibility) + end + def test_class_method_visibility util_parser <<~RUBY class A @@ -2729,6 +2914,7 @@ def foo; end tap do end def foo; end module_function :foo + module_function def foo; end end def bar; end From 106e3d45b3a71f4038b55cbd1eca85eb831db72e Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Tue, 20 Oct 2026 00:39:29 +0200 Subject: [PATCH 2/3] simplify --- lib/rdoc/parser/ruby.rb | 56 ++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index 1ba585f8e5..7b33d0aad0 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -133,9 +133,13 @@ class Ruby < Parser # Matches an RBS inline type annotation line: #: followed by whitespace RBS_SIG_LINE = /\A#:\s/ # :nodoc: - attr_accessor :visibility, :module_function_mode + attr_writer :visibility attr_reader :container, :singleton, :in_proc_block + def visibility + @visibility == :module_function ? :private : @visibility + end + def initialize(top_level, content, options, stats) super @@ -153,7 +157,6 @@ def initialize(top_level, content, options, stats) @module_nesting = [[top_level, false]] @container = top_level @visibility = :public - @module_function_mode = false @singleton = false @in_proc_block = false @doc_state = :startdoc @@ -221,12 +224,10 @@ def with_in_proc_block def with_container(container, singleton: false) old_container = @container old_visibility = @visibility - old_module_function_mode = @module_function_mode old_singleton = @singleton old_in_proc_block = @in_proc_block old_doc_state = @doc_state @visibility = :public - @module_function_mode = false @container = container @singleton = singleton @in_proc_block = false @@ -235,7 +236,6 @@ def with_container(container, singleton: false) ensure @container = old_container @visibility = old_visibility - @module_function_mode = old_module_function_mode @singleton = old_singleton @in_proc_block = old_in_proc_block @doc_state = old_doc_state @@ -418,7 +418,7 @@ def handle_meta_method_comment(comment, directives, node) handle_code_object_directives(@container, directives) is_call_node = node.is_a?(Prism::CallNode) singleton_method = false - visibility = @visibility + visibility = self.visibility attributes = rw = line_no = method_name = nil directives.each do |directive, (param, line)| case directive @@ -593,24 +593,23 @@ def change_method_to_module_function(names) # changes but the singleton method copies must not be documented return if document_suppressed? - new_methods = [] - @container.methods_matching(names) do |m| - s_m = m.dup - record_location(s_m) - s_m.singleton = true - new_methods << s_m - end - new_methods.each do |method| - method.visibility = :public - case method - when AnyMethod - @container.add_method(method) - when Attr - @container.add_attribute(method) - end - end + methods = @container.to_enum(:methods_matching, names).to_a + methods.each { |method| add_module_function_copy(method) } end + private def add_module_function_copy(method) # :nodoc: + method = method.dup + record_location(method) + method.singleton = true + method.visibility = :public + case method + when AnyMethod + @container.add_method(method) + when Attr + @container.add_attribute(method) + end + end + def handle_code_object_directives(code_object, directives) # :nodoc: directives.each do |directive, (param)| # startdoc/stopdoc/enddoc are handled by apply_document_control_directive. @@ -722,7 +721,7 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility: end private def internal_add_method(method_name, container, comment:, directives:, modifier_comment_lines: nil, line_no:, visibility:, singleton:, params:, calls_super:, block_params:, node_id:, type_signature_lines: nil) # :nodoc: - module_function = @module_function_mode && !singleton && container.equal?(@container) + module_function = @visibility == :module_function && !singleton && container.equal?(@container) meth = AnyMethod.new(method_name, singleton: singleton) meth.comment = comment handle_code_object_directives(meth, directives) if directives @@ -763,12 +762,7 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility: token_stream_loader = @colorizer_context.token_stream_loader(node_id) if node_id meth.start_collecting_tokens(:ruby, loader: token_stream_loader) - if module_function - singleton_method = meth.dup - singleton_method.singleton = true - singleton_method.visibility = :public - container.add_method(singleton_method) - end + add_module_function_copy(meth) if module_function end # Find or create module or class from a given module name using Ruby lexical @@ -1290,8 +1284,7 @@ def _visit_call_require(call_node) def _visit_call_module_function(call_node) return if @scanner.in_proc_block || @scanner.singleton if call_node.arguments.nil? - @scanner.visibility = :private - @scanner.module_function_mode = true + @scanner.visibility = :module_function else names = visibility_method_arguments(call_node, singleton: false)&.map(&:to_s) @scanner.change_method_to_module_function(names) if names @@ -1309,7 +1302,6 @@ def _visit_call_public_private_protected(call_node, visibility) arguments_node = call_node.arguments if arguments_node.nil? # `public` `private` @scanner.visibility = visibility - @scanner.module_function_mode = false else # `public :foo, :bar`, `private def foo; end` names = visibility_method_arguments(call_node, singleton: false) @scanner.change_method_visibility(names, visibility) if names From 3d13a8fedfdcf66f78e2bab12aeb03c765fdc146 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Tue, 20 Oct 2026 00:45:43 +0200 Subject: [PATCH 3/3] even more simplification --- lib/rdoc/parser/ruby.rb | 3 +-- test/rdoc/parser/ruby_test.rb | 21 +++------------------ 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index 7b33d0aad0..d79e72cfee 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -593,8 +593,7 @@ def change_method_to_module_function(names) # changes but the singleton method copies must not be documented return if document_suppressed? - methods = @container.to_enum(:methods_matching, names).to_a - methods.each { |method| add_module_function_copy(method) } + @container.methods_matching(names) { |method| add_module_function_copy(method) } end private def add_module_function_copy(method) # :nodoc: diff --git a/test/rdoc/parser/ruby_test.rb b/test/rdoc/parser/ruby_test.rb index 2ec26ff9f7..660aeae3d9 100644 --- a/test/rdoc/parser/ruby_test.rb +++ b/test/rdoc/parser/ruby_test.rb @@ -1418,8 +1418,7 @@ def generate(object); end end RUBY mod = @store.find_module_named 'JSON' - instance_methods = mod.method_list.reject(&:singleton) - singleton_methods = mod.method_list.select(&:singleton) + singleton_methods, instance_methods = mod.method_list.partition(&:singleton) assert_equal ['before', 'parse', 'generate'], instance_methods.map(&:name) assert_equal [:public, :private, :private], instance_methods.map(&:visibility) @@ -1553,25 +1552,11 @@ def implicit; end assert_equal [:public], nil_class.method_list.map(&:visibility) end - def test_module_function_without_arguments_initialize + def test_module_function_without_arguments_initialize_and_documentation_directives util_parser <<~RUBY module M module_function def initialize(value); end - end - RUBY - mod = @store.find_module_named 'M' - - assert_equal ['initialize', 'initialize'], mod.method_list.map(&:name) - assert_equal [false, true], mod.method_list.map(&:singleton) - assert_equal [:private, :public], mod.method_list.map(&:visibility) - end - - def test_module_function_without_arguments_respects_documentation_directives - util_parser <<~RUBY - module M - module_function - def visible; end def omitted; end # :nodoc: # :stopdoc: def hidden; end @@ -1583,7 +1568,7 @@ def ended; end RUBY mod = @store.find_module_named 'M' - assert_equal ['visible', 'visible', 'resumed', 'resumed'], mod.method_list.map(&:name) + assert_equal ['initialize', 'initialize', 'resumed', 'resumed'], mod.method_list.map(&:name) assert_equal [false, true, false, true], mod.method_list.map(&:singleton) assert_equal [:private, :public, :private, :public], mod.method_list.map(&:visibility) end