From 6833182b71f0dbef0c742738581c8b9eebb537c4 Mon Sep 17 00:00:00 2001 From: tompng Date: Sat, 1 Aug 2026 15:50:43 +0900 Subject: [PATCH 1/2] Make :stopdoc:/:startdoc:/:enddoc: lexically scoped in the Ruby parser These directives previously mutated shared CodeObject state (document_self/document_children/done_documenting), so a :stopdoc: without :startdoc: leaked into later reopenings of the class in the same file and in other files (#398, #1591). Track the region state in the parser instead and restore it when the enclosing class/module scope is closed. Containers created inside a suppressed region are still created as namespaces marked as ignored, and become documentable when they receive contents outside the region. Such containers that never receive contents are removed from the store on Store#complete via Context#remove_from_documentation?. Compatibility notes: - :nodoc: semantics are unchanged. :startdoc: additionally calls start_doc on the current container because `module Net #:nodoc:` followed by :startdoc: is a common pattern that expects the module to be documented. - A comment linked to a `class << self` line is now processed in the enclosing scope, so a :startdoc: there no longer expires at the end of the singleton class scope. - Region directives in modifier position (e.g. `class Foo # :stopdoc:`), which are unused in ruby/ruby, rails and the bundled gems, are no longer applied. Generated documentation for ruby/ruby lib and rails differs only in intended ways: suppressed namespaces no longer produce pages, module aliases written inside :stopdoc: regions (Ripper, HashWithIndifferentAccess shims) are no longer documented nor used for cross-reference resolution, and `include` after a :stopdoc: inside `class << self` is no longer lost (net/http/response.rb). Co-Authored-By: Claude Fable 5 --- lib/rdoc/code_object/context.rb | 4 +- lib/rdoc/parser/ruby.rb | 134 +++++++++++++++-- test/rdoc/generator/darkfish_test.rb | 3 +- test/rdoc/parser/ruby_test.rb | 212 +++++++++++++++++++++++++++ 4 files changed, 338 insertions(+), 15 deletions(-) diff --git a/lib/rdoc/code_object/context.rb b/lib/rdoc/code_object/context.rb index dc6f6be4af..7de435ee38 100644 --- a/lib/rdoc/code_object/context.rb +++ b/lib/rdoc/code_object/context.rb @@ -1066,8 +1066,10 @@ def record_location(top_level) # * All classes and modules have #remove_from_documentation? == true def remove_from_documentation? + # Contexts that are still ignored here were created inside a :stopdoc: + # region and never received documentable contents afterwards @remove_from_documentation ||= - @received_nodoc && + (@received_nodoc || @ignored) && !any_content(false) && @includes.all? { |i| !i.module.is_a?(String) && i.module.remove_from_documentation? } && classes_and_modules.all? { |cm| cm.remove_from_documentation? } diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index 55bfefae1e..e1d574ec02 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -152,6 +152,50 @@ def initialize(top_level, content, options, stats) @visibility = :public @singleton = false @in_proc_block = false + @doc_state = :startdoc + end + + # Applies document control directives (:startdoc:, :stopdoc: and :enddoc:) + # to the current lexical scope. The state is restored when the enclosing + # class/module scope is closed. + + def apply_document_control_directive(directives) + directives.each do |directive, _| + case directive + when 'startdoc', 'stopdoc' + # :enddoc: cannot be cancelled within the scope, even by :startdoc: + next if @doc_state == :enddoc + @doc_state = directive.to_sym + if directive == 'startdoc' && !@container.ignored? + # Compatibility: `module Net #:nodoc:` followed by :stopdoc:/:startdoc: + # regions is a common pattern that expects :startdoc: to make the + # container documentable again. Containers ignored here were created + # in a suppressed region and need documentable contents to revive. + @container.start_doc + @container.force_documentation = true + end + when 'enddoc' + @doc_state = :enddoc + end + end + end + + # Returns true if code objects at the current position should not be + # documented, that is, inside a :stopdoc: or :enddoc: region. + + def document_suppressed? + @track_visibility && @doc_state != :startdoc + end + + # Makes a container that was created inside a :stopdoc:/:enddoc: region + # (thus ignored) documentable again when it receives documentable contents + # outside the region, possibly from another file. + + def mark_container_documentable(container) + return if container.received_nodoc || !container.ignored? + record_location(container) + container.start_doc + mark_container_documentable(container.parent) if container.parent.is_a?(RDoc::ClassModule) end # Suppress `extend` and `include` within block @@ -172,6 +216,7 @@ def with_container(container, singleton: false) old_visibility = @visibility old_singleton = @singleton old_in_proc_block = @in_proc_block + old_doc_state = @doc_state @visibility = :public @container = container @singleton = singleton @@ -183,6 +228,7 @@ def with_container(container, singleton: false) @visibility = old_visibility @singleton = old_singleton @in_proc_block = old_in_proc_block + @doc_state = old_doc_state @module_nesting.pop end @@ -353,6 +399,7 @@ def call_node_name_arguments(call_node) # :nodoc: # Handles meta method comments def handle_meta_method_comment(comment, directives, node) + apply_document_control_directive(directives) handle_code_object_directives(@container, directives) is_call_node = node.is_a?(Prism::CallNode) singleton_method = false @@ -375,6 +422,8 @@ def handle_meta_method_comment(comment, directives, node) end end + return if document_suppressed? + if attributes attributes.each do |attr| a = RDoc::Attr.new(attr, rw, comment, singleton: @singleton) @@ -382,6 +431,7 @@ def handle_meta_method_comment(comment, directives, node) a.line = line_no record_location(a) @container.add_attribute(a) + mark_container_documentable(@container) a.visibility = visibility end elsif line_no || node @@ -427,6 +477,7 @@ def handle_standalone_consecutive_comment_directive(comment, directives, start_w elsif normal_comment_treat_as_ghost_method_for_now?(directives, line_no) && start_line != @first_non_meta_comment_start_line handle_meta_method_comment(comment, directives, nil) else + apply_document_control_directive(directives) handle_code_object_directives(@container, directives) end end @@ -552,6 +603,9 @@ def change_method_to_module_function(names) def handle_code_object_directives(code_object, directives) # :nodoc: directives.each do |directive, (param)| + # startdoc/stopdoc/enddoc are handled by apply_document_control_directive. + # They control the lexical scope of the parser, not the code object. + next if directive == 'startdoc' || directive == 'stopdoc' || directive == 'enddoc' @preprocess.handle_directive('', directive, param, code_object) end end @@ -560,7 +614,10 @@ def handle_code_object_directives(code_object, directives) # :nodoc: def add_alias_method(old_name, new_name, line_no) comment, directives = consecutive_comment(line_no) + apply_document_control_directive(directives) if directives handle_code_object_directives(@container, directives) if directives + return if document_suppressed? + visibility = @container.find_method(old_name, @singleton)&.visibility || :public a = RDoc::Alias.new(old_name, new_name, comment, singleton: @singleton) handle_modifier_directive(a, line_no) @@ -568,6 +625,7 @@ def add_alias_method(old_name, new_name, line_no) a.line = line_no record_location(a) if should_document?(a) + mark_container_documentable(@container) @container.add_alias(a) @container.find_method(new_name, @singleton)&.visibility = visibility end @@ -577,7 +635,9 @@ def add_alias_method(old_name, new_name, line_no) def add_attributes(names, rw, line_no) comment, directives, type_signature_lines = consecutive_comment(line_no) + apply_document_control_directive(directives) if directives handle_code_object_directives(@container, directives) if directives + return if document_suppressed? return unless @container.document_children names.each do |symbol| @@ -587,7 +647,10 @@ def add_attributes(names, rw, line_no) a.type_signature_lines = type_signature_lines record_location(a) handle_modifier_directive(a, line_no) - @container.add_attribute(a) if should_document?(a) + if should_document?(a) + @container.add_attribute(a) + mark_container_documentable(@container) + end a.visibility = visibility # should set after adding to container end end @@ -596,7 +659,11 @@ def add_attributes(names, rw, line_no) def add_includes_extends(names, rdoc_class, line_no) # :nodoc: comment, directives = consecutive_comment(line_no) + apply_document_control_directive(directives) if directives handle_code_object_directives(@container, directives) if directives + return if document_suppressed? + + mark_container_documentable(@container) names.each do |name| resolved_name = resolve_constant_path(name) ie = @container.add(rdoc_class, resolved_name || name, '') @@ -622,9 +689,12 @@ def add_extends(names, line_no) # :nodoc: # Adds a method defined by `def` syntax def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:, singleton:, params:, calls_super:, block_params:, tokens:, start_line:, args_end_line:, end_line:) - receiver = receiver_name ? find_or_create_lexical_module_path(receiver_name, receiver_fallback_type) : @container comment, directives, type_signature_lines = consecutive_comment(start_line) + apply_document_control_directive(directives) if directives handle_code_object_directives(@container, directives) if directives + # Resolve receiver after applying directives so that a namespace created + # here is marked as ignored when the comment starts a :stopdoc: region + receiver = receiver_name ? find_or_create_lexical_module_path(receiver_name, receiver_fallback_type) : @container internal_add_method( method_name, @@ -650,8 +720,11 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility: modifier_comment_lines&.each do |line| handle_modifier_directive(meth, line) end + return if document_suppressed? return unless should_document?(meth) + mark_container_documentable(container) + if directives && (call_seq, = directives['call-seq']) meth.call_seq = call_seq.lines.map(&:chomp).reject(&:empty?).join("\n") if call_seq end @@ -691,12 +764,18 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility: def find_or_create_lexical_module_path(module_name, create_mode) root_name, *path, name = module_name.split('::') add_module = ->(mod, name, mode) { - case mode - when :class - mod.add_class(RDoc::NormalClass, name, 'Object').tap { |m| m.store = @store } - when :module - mod.add_module(RDoc::NormalModule, name).tap { |m| m.store = @store } - end + created = + case mode + when :class + mod.add_class(RDoc::NormalClass, name, 'Object').tap { |m| m.store = @store } + when :module + mod.add_module(RDoc::NormalModule, name).tap { |m| m.store = @store } + end + # add_class/add_module may return an existing object created by another + # file (in_files is not empty then), which must not be ignored here. + # Documentable again when reopened or receiving contents outside the region. + created.ignore if document_suppressed? && created.in_files.empty? + created } if root_name.empty? mod = @top_level @@ -755,7 +834,10 @@ def find_or_create_lexical_constant_owner_name(constant_path) def add_constant(constant_name, rhs_name, start_line, end_line, alias_path: nil) comment, directives = consecutive_comment(start_line) + apply_document_control_directive(directives) if directives handle_code_object_directives(@container, directives) if directives + return if document_suppressed? + owner, name = find_or_create_lexical_constant_owner_name(constant_name) return unless owner @@ -763,6 +845,7 @@ def add_constant(constant_name, rhs_name, start_line, end_line, alias_path: nil) constant.store = @store constant.line = start_line constant.is_alias_for_path = alias_path + mark_container_documentable(owner) if owner.is_a?(RDoc::ClassModule) record_location(constant) handle_modifier_directive(constant, start_line) handle_modifier_directive(constant, end_line) @@ -787,6 +870,7 @@ def add_constant(constant_name, rhs_name, start_line, end_line, alias_path: nil) def add_module_or_class(module_name, start_line, end_line, is_class: false, superclass_name: nil, superclass_expr: nil) comment, directives = consecutive_comment(start_line) + apply_document_control_directive(directives) if directives handle_code_object_directives(@container, directives) if directives return unless @container.document_children @@ -803,7 +887,13 @@ def add_module_or_class(module_name, start_line, end_line, is_class: false, supe superclass_full_path = superclass_full_path.sub(/^::/, '') end # add_class should be done after resolving superclass - mod = owner.classes_hash[name] || owner.add_class(RDoc::NormalClass, name, superclass_name || superclass_expr || '::Object') + mod = owner.classes_hash[name] + unless mod + # add_class may return an existing class created by another file + # (in_files is not empty then), which must not be ignored here + mod = owner.add_class(RDoc::NormalClass, name, superclass_name || superclass_expr || '::Object') + mod.ignore if document_suppressed? && mod.in_files.empty? + end if superclass_name if superclass mod.superclass = superclass @@ -812,15 +902,29 @@ def add_module_or_class(module_name, start_line, end_line, is_class: false, supe end end else - mod = owner.modules_hash[name] || owner.add_module(RDoc::NormalModule, name) + mod = owner.modules_hash[name] + unless mod + mod = owner.add_module(RDoc::NormalModule, name) + mod.ignore if document_suppressed? && mod.in_files.empty? + end end mod.store = @store mod.line = start_line - record_location(mod) handle_modifier_directive(mod, start_line) handle_modifier_directive(mod, end_line) - mod.add_comment(comment, @top_level) if comment + unless document_suppressed? + # In a :stopdoc:/:enddoc: region, the container is still created as a + # namespace (the body is visited so that an inner :startdoc: works) + # but is not recorded to this file nor documented + mark_container_documentable(owner) if owner.is_a?(RDoc::ClassModule) + if mod.ignored? + mark_container_documentable(mod) + else + record_location(mod) + end + mod.add_comment(comment, @top_level) if comment + end mod end @@ -975,7 +1079,9 @@ def visit_class_node(node) end def visit_singleton_class_node(node) - @scanner.process_comments_until(node.location.start_line - 1) + # A comment linked to the `class << ...` line (e.g. a document control + # directive) belongs to the enclosing scope, not to the singleton scope + @scanner.process_comments_until(node.location.start_line) if @scanner.has_modifier_nodoc?(node.location.start_line) # Skip visiting inside the singleton class. Also skips creation of node.expression as a module @@ -990,6 +1096,7 @@ def visit_singleton_class_node(node) when Prism::ConstantWriteNode # Accept `class << (NameErrorCheckers = Object.new)` as a module which is not actually a module mod = @scanner.container.add_module(RDoc::NormalModule, expression.name.to_s) + mod.ignore if @scanner.document_suppressed? && mod.in_files.empty? when Prism::ConstantPathNode, Prism::ConstantReadNode expression_name = constant_path_string(expression) # If a constant_path does not exist, RDoc creates a module @@ -1150,6 +1257,7 @@ def constant_path_string(node) end def _visit_call_require(call_node) + return if @scanner.document_suppressed? return unless call_node.arguments&.arguments&.size == 1 arg = call_node.arguments.arguments.first return unless arg.is_a?(Prism::StringNode) diff --git a/test/rdoc/generator/darkfish_test.rb b/test/rdoc/generator/darkfish_test.rb index 2f43ea0a4c..34c8431219 100644 --- a/test/rdoc/generator/darkfish_test.rb +++ b/test/rdoc/generator/darkfish_test.rb @@ -355,7 +355,8 @@ def test_setup assert_equal %w[Ignored Klass Klass::A Object], [@ignored, @klass, @klass_alias, @object].map(&:full_name) - assert_equal [@ignored, @klass, @klass_alias, @object], + # @ignored is removed from the store on Store#complete + assert_equal [@klass, @klass_alias, @object], @g.classes.sort_by { |klass| klass.full_name } assert_equal [@top_level], @g.files assert_equal [@meth, @meth, @meth_bang, @meth_bang, @meth_with_html_tag_yield, @meth_with_html_tag_yield], @g.methods diff --git a/test/rdoc/parser/ruby_test.rb b/test/rdoc/parser/ruby_test.rb index 99b99e5065..8f6a671ff9 100644 --- a/test/rdoc/parser/ruby_test.rb +++ b/test/rdoc/parser/ruby_test.rb @@ -349,6 +349,218 @@ module B; end assert_equal ['Bar::A'], mod.modules.select(&:document_self).map(&:full_name) end + def test_stopdoc_closed_at_end_of_scope + util_parser <<~RUBY + class A + # :stopdoc: + HIDDEN = 1 + end + + class B; end + + class A + VISIBLE = 1 + end + RUBY + a, b = @top_level.classes + assert_equal 'B', b.full_name + refute b.ignored? + assert_equal ['VISIBLE'], a.constants.map(&:name) + end + + def test_stopdoc_does_not_leak_to_another_file + util_parser <<~RUBY + class A + # :stopdoc: + def hidden; end + end + RUBY + other_top_level = @store.add_file 'other.rb' + parser = RDoc::Parser::Ruby.new other_top_level, <<~RUBY, @options, @stats + class A + def visible; end + end + RUBY + parser.scan + a = @store.find_class_or_module('A') + assert a.document_self + assert_equal ['visible'], a.method_list.map(&:name) + end + + def test_startdoc_in_nested_scope_of_stopdoc + util_parser <<~RUBY + class A + # :stopdoc: + class B + # :startdoc: + def visible; end + end + HIDDEN = 1 + end + RUBY + a = @top_level.classes.first + b = @store.find_class_or_module('A::B') + refute b.ignored? + assert_equal ['visible'], b.method_list.map(&:name) + # :stopdoc: state is restored after class B's end + assert_empty a.constants + end + + def test_class_created_in_stopdoc_region + util_parser <<~RUBY + # :stopdoc: + class Hidden + def f; end + end + # :startdoc: + class Visible; end + RUBY + assert_equal ['Visible'], @top_level.classes.reject(&:ignored?).map(&:full_name) + assert_empty @store.find_class_or_module('Hidden').method_list + end + + def test_class_created_in_stopdoc_region_reopened + util_parser <<~RUBY + # :stopdoc: + class Foo + def hidden; end + end + # :startdoc: + class Foo + def visible; end + end + RUBY + foo = @store.find_class_or_module('Foo') + refute foo.ignored? + assert_equal ['visible'], foo.method_list.map(&:name) + end + + def test_class_created_in_stopdoc_region_documented_by_singleton_method_def + util_parser <<~RUBY + # :stopdoc: + class Foo; end + # :startdoc: + def Foo.f; end + RUBY + foo = @store.find_class_or_module('Foo') + refute foo.ignored? + assert_equal ['f'], foo.method_list.map(&:name) + end + + def test_class_created_in_stopdoc_region_documented_by_constant + util_parser <<~RUBY + # :stopdoc: + class Foo; end + # :startdoc: + Foo::X = 1 + RUBY + foo = @store.find_class_or_module('Foo') + refute foo.ignored? + assert_equal ['X'], foo.constants.map(&:name) + end + + def test_require_in_stopdoc_region + util_parser <<~RUBY + require 'a' + # :stopdoc: + class Hidden; end + require 'b' + RUBY + assert_equal ['a'], @top_level.requires.map(&:name) + end + + # action_dispatch/http/rack_cache.rb pattern: reopening a module documented + # in another file inside an :enddoc: file must not hide the module itself + def test_reopen_documented_module_in_enddoc_file + util_parser <<~RUBY + module Foo + def visible; end + end + RUBY + other_top_level = @store.add_file 'other.rb' + parser = RDoc::Parser::Ruby.new other_top_level, <<~RUBY, @options, @stats + # :enddoc: + + module Foo + class Hidden; end + end + RUBY + parser.scan + foo = @store.find_class_or_module('Foo') + refute foo.ignored? + assert_equal ['visible'], foo.method_list.map(&:name) + assert foo.classes.all?(&:ignored?) + end + + # net/http.rb pattern: `module Net #:nodoc:` expects :startdoc: to make + # Net documentable again + def test_startdoc_in_nodoc_module + util_parser <<~RUBY + module Net # :nodoc: + # :stopdoc: + class Hidden; end + # :startdoc: + class Visible; end + CONST = 1 + end + RUBY + net = @top_level.modules.first + assert net.document_self + assert_equal ['Net::Visible'], net.classes.reject(&:ignored?).map(&:full_name) + assert_equal ['CONST'], net.constants.map(&:name) + end + + def test_document_control_directive_attached_to_singleton_class + util_parser <<~RUBY + class Foo + # :stopdoc: + def hidden; end + + # :startdoc: + class << self + def smethod; end + end + + def visible; end + end + RUBY + foo = @top_level.classes.first + assert_equal ['smethod', 'visible'], foo.method_list.map(&:name).sort + end + + # prism/translation/ripper/shim.rb pattern + def test_constant_alias_in_stopdoc_region + util_parser <<~RUBY + class Real; end + # :stopdoc: + RealAlias = Real + # :startdoc: + RUBY + assert_empty @top_level.constants + assert_equal ['Real'], @store.all_classes_and_modules.map(&:full_name) + end + + # net/http/response.rb pattern: :stopdoc: in `class << self` scope should not + # suppress the rest of the class body + def test_stopdoc_closed_at_end_of_singleton_class_scope + util_parser <<~RUBY + module Bar; end + class Foo + class << self + # :stopdoc: + def hidden; end + end + + include Bar + + def visible; end + end + RUBY + foo = @top_level.classes.first + assert_equal ['Bar'], foo.includes.map(&:name) + assert_equal ['visible'], foo.method_list.map(&:name) + end + def test_class_superclass util_parser <<~RUBY class Foo; end From 5aec0e48ae3b910ad81bde8710f2188c8416cb87 Mon Sep 17 00:00:00 2001 From: tompng Date: Tue, 4 Aug 2026 01:40:34 +0900 Subject: [PATCH 2/2] Clarify that :enddoc: is final for the whole scope subtree The comment in add_module_or_class implied an inner :startdoc: also works in an :enddoc: region. It does not: :enddoc: is inherited by nested scopes and cannot be cancelled there, matching the previous behavior where nested class bodies under :enddoc: were not visited at all. Pin this semantics with a test. Co-Authored-By: Claude Fable 5 --- lib/rdoc/parser/ruby.rb | 6 ++++-- test/rdoc/parser/ruby_test.rb | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/rdoc/parser/ruby.rb b/lib/rdoc/parser/ruby.rb index e1d574ec02..3d2f30c577 100644 --- a/lib/rdoc/parser/ruby.rb +++ b/lib/rdoc/parser/ruby.rb @@ -915,8 +915,10 @@ def add_module_or_class(module_name, start_line, end_line, is_class: false, supe handle_modifier_directive(mod, end_line) unless document_suppressed? # In a :stopdoc:/:enddoc: region, the container is still created as a - # namespace (the body is visited so that an inner :startdoc: works) - # but is not recorded to this file nor documented + # namespace but is not recorded to this file nor documented. + # The body is also visited: an inner :startdoc: re-enables documentation + # in a :stopdoc: region (not in an :enddoc: region), and nested + # namespaces need to be created for later promotion from other files mark_container_documentable(owner) if owner.is_a?(RDoc::ClassModule) if mod.ignored? mark_container_documentable(mod) diff --git a/test/rdoc/parser/ruby_test.rb b/test/rdoc/parser/ruby_test.rb index 8f6a671ff9..769c8e52e6 100644 --- a/test/rdoc/parser/ruby_test.rb +++ b/test/rdoc/parser/ruby_test.rb @@ -492,6 +492,24 @@ class Hidden; end assert foo.classes.all?(&:ignored?) end + # :enddoc: is final for the whole scope subtree: unlike :stopdoc:, + # a :startdoc: in a nested scope cannot re-enable documentation + def test_enddoc_cannot_be_cancelled_by_startdoc_in_nested_scope + util_parser <<~RUBY + class A + # :enddoc: + class B + # :startdoc: + def hidden; end + end + end + RUBY + a = @top_level.classes.first + assert_equal [], a.classes.reject(&:ignored?).map(&:full_name) + b = @store.find_class_or_module('A::B') + assert_empty b.method_list + end + # net/http.rb pattern: `module Net #:nodoc:` expects :startdoc: to make # Net documentable again def test_startdoc_in_nodoc_module