Skip to content

Commit d13636a

Browse files
tompngclaude
andauthored
Make :stopdoc:/:startdoc:/:enddoc: lexically scoped in the Ruby parser (#1763)
Part of #1591. This PR only changes the region directives (`:stopdoc:`/`:startdoc:`/`:enddoc:`). `:nodoc:` semantics are unchanged and will be addressed separately. ## Problem These directives are currently implemented as mutations of shared `CodeObject` state (`document_self`/`document_children`/`done_documenting`). Since classes are open and shared across files in the store, a `:stopdoc:` without a matching `:startdoc:` leaks into later reopenings of the class within the same file. The actual behavior is a product of mutation order, and neither the documentation nor the implementation defines what these directives mean. ## New semantics The parser now tracks the document control state (`:startdoc:`/`:stopdoc:`/`:enddoc:`) as a lexical region: - The state is inherited when entering a class/module scope and automatically restored at its `end`. A `:stopdoc:` can no longer leak out of the scope it was written in. - `:startdoc:` inside a nested scope re-enables documentation for that scope only. - `:enddoc:` disables documentation for the rest of the current scope and cannot be cancelled within it, even by `:startdoc:`. It no longer marks the container object as `done_documenting` globally. - Classes/modules defined inside a suppressed region are still created as namespaces (marked as ignored) and their bodies are visited, so an inner `:startdoc:` works. They become documentable when they receive documentable contents outside the region, possibly from another file. Namespaces that never receive contents are removed from the store on `Store#complete` (`Context#remove_from_documentation?` now also covers ignored contexts, which is what `CodeObject#ignore` was documented to do in GH #55). - Document control scopes are the syntactic units that change the definition context (`class`/`module` bodies, `class << self`, and definition-creating blocks). `if`/`while`/`begin` do not create scopes, consistent with Ruby's own scoping rules. ## Bug fixes On master, these directives mutate the state of whatever object they happen to be applied to, so their effect escapes the scope they were written in. Some examples of what this PR fixes: **A `:stopdoc:` inside `class << self` no longer leaks out of the singleton scope:** ```ruby class Net::HTTPResponse class << self # :stopdoc: def internal; end end # master: NOT documented / this PR: documented attr_reader :http_version end ``` This is exactly what happened to [the Ruby 4.0 documentation](https://docs.ruby-lang.org/en/4.0/Net/HTTPResponse.html) **A `:stopdoc:` no longer leaks into later reopenings of the class.** Note that on master the two ways of reopening `A::B` below do not even agree with each other: `f` is documented but `g` is not, because the leaked state only affects the nested-class form. The behavior was an artifact of the implementation, not a specification: ```ruby class A # :stopdoc: def internal; end end class A::B # master: documented / this PR: documented def f; end end class A class B # master: NOT documented / this PR: documented def g; end end end ``` ## Compatibility notes - `:nodoc:` is untouched. However, `:startdoc:` additionally calls `start_doc` on the current container, because `module Net #:nodoc:` followed by `:stopdoc:`/`:startdoc:` regions is a common pattern that expects `:startdoc:` to make the container documentable again. This is a transitional measure until `:nodoc:` gets lexical semantics, at which point the pattern itself becomes unnecessary. Containers created inside a suppressed region are excluded, so a bare `:startdoc:` cannot revive an empty hidden class. - A comment linked to a `class << X` line is now processed in the enclosing scope. Nothing consumes such a comment as documentation, and processing it inside the singleton scope would make a `:startdoc:` written there expire at the singleton's `end` (this pattern appears in net/http). - Region directives in modifier position (e.g. `class Foo # :stopdoc:` at the end of a line) are no longer applied. There are no occurrences of this form in ruby/ruby, rails, or the bundled gems. ## Validation Compared generated HTML for ruby/ruby `lib` and six Rails frameworks between master and this branch. The sets of generated files are identical, and every remaining difference is the directives working as their authors intended: - Module aliases written inside `:stopdoc:` regions (`Ripper = Prism::Translation::Ripper` in prism's shim.rb, `HashWithIndifferentAccess = ...` in activesupport) are no longer documented. This also removes a duplicated "Ripper" entry from the navigation and stops bare "Ripper" mentions (which refer to the stdlib Ripper) from being cross-referenced to `Prism::Translation::Ripper`. Note: the old accidental link on `HashWithIndifferentAccess` in Hash.html was arguably useful; writing the full name in the comment restores it. - `Net::HTTPResponse` regains its `include Net::HTTPHeader`. The old implementation lost it because a `:stopdoc:` inside `class << self` leaked out of the singleton scope; net/http.rb contains a "this is to fix bug in RDoc" workaround comment for this exact class of leak. - `ActionDispatch::Journey` (`# :nodoc:`) no longer appears in the navigation. Its page is still generated because it has documented children. - Namespaces fully contained in suppressed regions (e.g. `Net::HTTPBadResponse` region in net/http.rb, `Timeout::Request`) no longer produce empty pages. ## Follow-ups (out of scope here) - Lexical/shallow semantics for `:nodoc:` (the hard part of #1591); the `start_doc` compatibility shim and the `received_nodoc` guard in promotion can be removed then. - Warnings for degenerate usages (redundant `:stopdoc:`, directives after `:enddoc:`, standalone `:nodoc:` in the middle of a class body). - A visibility trace output (`file:line: stopdoc applied to X`) as proposed in #1591 (comment); the parser now has the lexical information to make this cheap. - The C parser still uses the old CodeObject-state mechanism and is intentionally untouched. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent c162c6b commit d13636a

4 files changed

Lines changed: 434 additions & 16 deletions

File tree

‎lib/rdoc/code_object/context.rb‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1066,8 +1066,10 @@ def record_location(top_level)
10661066
# * All classes and modules have <tt>#remove_from_documentation? == true</tt>
10671067

10681068
def remove_from_documentation?
1069+
# Contexts that are still ignored here were created inside a :stopdoc:
1070+
# region and never received documentable contents afterwards
10691071
@remove_from_documentation ||=
1070-
@received_nodoc &&
1072+
(@received_nodoc || @ignored) &&
10711073
!any_content(false) &&
10721074
@includes.all? { |i| !i.module.is_a?(String) && i.module.remove_from_documentation? } &&
10731075
classes_and_modules.all? { |cm| cm.remove_from_documentation? }

‎lib/rdoc/parser/ruby.rb‎

Lines changed: 138 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,53 @@ def initialize(top_level, content, options, stats)
152152
@visibility = :public
153153
@singleton = false
154154
@in_proc_block = false
155+
@doc_state = :startdoc
156+
end
157+
158+
# Applies document control directives (:startdoc:, :stopdoc: and :enddoc:)
159+
# to the current lexical scope. The state is restored when the enclosing
160+
# class/module scope is closed.
161+
162+
def apply_document_control_directive(directives)
163+
directives.each do |directive, (_param, line)|
164+
case directive
165+
when 'startdoc', 'stopdoc'
166+
# :enddoc: cannot be cancelled within the scope, even by :startdoc:
167+
if @doc_state == :enddoc
168+
@options.warn "#{@top_level.relative_name}:#{line}: :startdoc: is ignored after :enddoc:" if directive == 'startdoc'
169+
next
170+
end
171+
@doc_state = directive.to_sym
172+
if directive == 'startdoc' && !@container.ignored?
173+
# Compatibility: `module Net #:nodoc:` followed by :stopdoc:/:startdoc:
174+
# regions is a common pattern that expects :startdoc: to make the
175+
# container documentable again. Containers ignored here were created
176+
# in a suppressed region and need documentable contents to revive.
177+
@container.start_doc
178+
@container.force_documentation = true
179+
end
180+
when 'enddoc'
181+
@doc_state = :enddoc
182+
end
183+
end
184+
end
185+
186+
# Returns true if code objects at the current position should not be
187+
# documented, that is, inside a :stopdoc: or :enddoc: region.
188+
189+
def document_suppressed?
190+
@track_visibility && @doc_state != :startdoc
191+
end
192+
193+
# Makes a container that was created inside a :stopdoc:/:enddoc: region
194+
# (thus ignored) documentable again when it receives documentable contents
195+
# outside the region, possibly from another file.
196+
197+
def mark_container_documentable(container)
198+
return if container.received_nodoc || !container.ignored?
199+
record_location(container)
200+
container.start_doc
201+
mark_container_documentable(container.parent) if container.parent.is_a?(RDoc::ClassModule)
155202
end
156203

157204
# Suppress `extend` and `include` within block
@@ -172,6 +219,7 @@ def with_container(container, singleton: false)
172219
old_visibility = @visibility
173220
old_singleton = @singleton
174221
old_in_proc_block = @in_proc_block
222+
old_doc_state = @doc_state
175223
@visibility = :public
176224
@container = container
177225
@singleton = singleton
@@ -183,6 +231,7 @@ def with_container(container, singleton: false)
183231
@visibility = old_visibility
184232
@singleton = old_singleton
185233
@in_proc_block = old_in_proc_block
234+
@doc_state = old_doc_state
186235
@module_nesting.pop
187236
end
188237

@@ -307,6 +356,7 @@ def prepare_comments(comments)
307356
# Signature section in the comment
308357

309358
def parse_comment_tomdoc(container, comment, line_no, start_line)
359+
return if document_suppressed?
310360
return unless signature = RDoc::TomDoc.signature(comment)
311361

312362
name, = signature.split %r%[ \(]%, 2
@@ -353,6 +403,7 @@ def call_node_name_arguments(call_node) # :nodoc:
353403
# Handles meta method comments
354404

355405
def handle_meta_method_comment(comment, directives, node)
406+
apply_document_control_directive(directives)
356407
handle_code_object_directives(@container, directives)
357408
is_call_node = node.is_a?(Prism::CallNode)
358409
singleton_method = false
@@ -375,13 +426,16 @@ def handle_meta_method_comment(comment, directives, node)
375426
end
376427
end
377428

429+
return if document_suppressed?
430+
378431
if attributes
379432
attributes.each do |attr|
380433
a = RDoc::Attr.new(attr, rw, comment, singleton: @singleton)
381434
a.store = @store
382435
a.line = line_no
383436
record_location(a)
384437
@container.add_attribute(a)
438+
mark_container_documentable(@container)
385439
a.visibility = visibility
386440
end
387441
elsif line_no || node
@@ -427,6 +481,7 @@ def handle_standalone_consecutive_comment_directive(comment, directives, start_w
427481
elsif normal_comment_treat_as_ghost_method_for_now?(directives, line_no) && start_line != @first_non_meta_comment_start_line
428482
handle_meta_method_comment(comment, directives, nil)
429483
else
484+
apply_document_control_directive(directives)
430485
handle_code_object_directives(@container, directives)
431486
end
432487
end
@@ -510,6 +565,9 @@ def change_method_visibility(names, visibility, singleton: @singleton)
510565
new_methods = []
511566
@container.methods_matching(names, singleton) do |m|
512567
if m.parent != @container
568+
# A copy of an ancestor's method must not be documented
569+
# in a :stopdoc:/:enddoc: region
570+
next if document_suppressed?
513571
m = m.dup
514572
record_location(m)
515573
new_methods << m
@@ -532,6 +590,10 @@ def change_method_visibility(names, visibility, singleton: @singleton)
532590

533591
def change_method_to_module_function(names)
534592
@container.set_visibility_for(names, :private, false)
593+
# In a :stopdoc:/:enddoc: region, the visibility of instance methods still
594+
# changes but the singleton method copies must not be documented
595+
return if document_suppressed?
596+
535597
new_methods = []
536598
@container.methods_matching(names) do |m|
537599
s_m = m.dup
@@ -552,6 +614,9 @@ def change_method_to_module_function(names)
552614

553615
def handle_code_object_directives(code_object, directives) # :nodoc:
554616
directives.each do |directive, (param)|
617+
# startdoc/stopdoc/enddoc are handled by apply_document_control_directive.
618+
# They control the lexical scope of the parser, not the code object.
619+
next if directive == 'startdoc' || directive == 'stopdoc' || directive == 'enddoc'
555620
@preprocess.handle_directive('', directive, param, code_object)
556621
end
557622
end
@@ -560,14 +625,18 @@ def handle_code_object_directives(code_object, directives) # :nodoc:
560625

561626
def add_alias_method(old_name, new_name, line_no)
562627
comment, directives = consecutive_comment(line_no)
628+
apply_document_control_directive(directives) if directives
563629
handle_code_object_directives(@container, directives) if directives
630+
return if document_suppressed?
631+
564632
visibility = @container.find_method(old_name, @singleton)&.visibility || :public
565633
a = RDoc::Alias.new(old_name, new_name, comment, singleton: @singleton)
566634
handle_modifier_directive(a, line_no)
567635
a.store = @store
568636
a.line = line_no
569637
record_location(a)
570638
if should_document?(a)
639+
mark_container_documentable(@container)
571640
@container.add_alias(a)
572641
@container.find_method(new_name, @singleton)&.visibility = visibility
573642
end
@@ -577,7 +646,9 @@ def add_alias_method(old_name, new_name, line_no)
577646

578647
def add_attributes(names, rw, line_no)
579648
comment, directives, type_signature_lines = consecutive_comment(line_no)
649+
apply_document_control_directive(directives) if directives
580650
handle_code_object_directives(@container, directives) if directives
651+
return if document_suppressed?
581652
return unless @container.document_children
582653

583654
names.each do |symbol|
@@ -587,7 +658,10 @@ def add_attributes(names, rw, line_no)
587658
a.type_signature_lines = type_signature_lines
588659
record_location(a)
589660
handle_modifier_directive(a, line_no)
590-
@container.add_attribute(a) if should_document?(a)
661+
if should_document?(a)
662+
@container.add_attribute(a)
663+
mark_container_documentable(@container)
664+
end
591665
a.visibility = visibility # should set after adding to container
592666
end
593667
end
@@ -596,7 +670,11 @@ def add_attributes(names, rw, line_no)
596670

597671
def add_includes_extends(names, rdoc_class, line_no) # :nodoc:
598672
comment, directives = consecutive_comment(line_no)
673+
apply_document_control_directive(directives) if directives
599674
handle_code_object_directives(@container, directives) if directives
675+
return if document_suppressed?
676+
677+
mark_container_documentable(@container)
600678
names.each do |name|
601679
resolved_name = resolve_constant_path(name)
602680
ie = @container.add(rdoc_class, resolved_name || name, '')
@@ -622,9 +700,12 @@ def add_extends(names, line_no) # :nodoc:
622700
# Adds a method defined by `def` syntax
623701

624702
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:)
625-
receiver = receiver_name ? find_or_create_lexical_module_path(receiver_name, receiver_fallback_type) : @container
626703
comment, directives, type_signature_lines = consecutive_comment(start_line)
704+
apply_document_control_directive(directives) if directives
627705
handle_code_object_directives(@container, directives) if directives
706+
# Resolve receiver after applying directives so that a namespace created
707+
# here is marked as ignored when the comment starts a :stopdoc: region
708+
receiver = receiver_name ? find_or_create_lexical_module_path(receiver_name, receiver_fallback_type) : @container
628709

629710
internal_add_method(
630711
method_name,
@@ -650,8 +731,11 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:
650731
modifier_comment_lines&.each do |line|
651732
handle_modifier_directive(meth, line)
652733
end
734+
return if document_suppressed?
653735
return unless should_document?(meth)
654736

737+
mark_container_documentable(container)
738+
655739
if directives && (call_seq, = directives['call-seq'])
656740
meth.call_seq = call_seq.lines.map(&:chomp).reject(&:empty?).join("\n") if call_seq
657741
end
@@ -691,12 +775,18 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:
691775
def find_or_create_lexical_module_path(module_name, create_mode)
692776
root_name, *path, name = module_name.split('::')
693777
add_module = ->(mod, name, mode) {
694-
case mode
695-
when :class
696-
mod.add_class(RDoc::NormalClass, name, 'Object').tap { |m| m.store = @store }
697-
when :module
698-
mod.add_module(RDoc::NormalModule, name).tap { |m| m.store = @store }
699-
end
778+
created =
779+
case mode
780+
when :class
781+
mod.add_class(RDoc::NormalClass, name, 'Object').tap { |m| m.store = @store }
782+
when :module
783+
mod.add_module(RDoc::NormalModule, name).tap { |m| m.store = @store }
784+
end
785+
# add_class/add_module may return an existing object created by another
786+
# file (in_files is not empty then), which must not be ignored here.
787+
# Documentable again when reopened or receiving contents outside the region.
788+
created.ignore if document_suppressed? && created.in_files.empty?
789+
created
700790
}
701791
if root_name.empty?
702792
mod = @top_level
@@ -755,17 +845,22 @@ def find_or_create_lexical_constant_owner_name(constant_path)
755845

756846
def add_constant(constant_name, rhs_name, start_line, end_line, alias_path: nil)
757847
comment, directives = consecutive_comment(start_line)
848+
apply_document_control_directive(directives) if directives
758849
handle_code_object_directives(@container, directives) if directives
850+
return if document_suppressed?
851+
759852
owner, name = find_or_create_lexical_constant_owner_name(constant_name)
760853
return unless owner
761854

762855
constant = RDoc::Constant.new(name, rhs_name, comment)
763856
constant.store = @store
764857
constant.line = start_line
765858
constant.is_alias_for_path = alias_path
766-
record_location(constant)
767859
handle_modifier_directive(constant, start_line)
768860
handle_modifier_directive(constant, end_line)
861+
# A constant marked :nodoc: must not make an ignored owner documentable
862+
mark_container_documentable(owner) if constant.document_self && owner.is_a?(RDoc::ClassModule)
863+
record_location(constant)
769864
owner.add_constant(constant)
770865
return unless alias_path
771866
mod =
@@ -787,6 +882,7 @@ def add_constant(constant_name, rhs_name, start_line, end_line, alias_path: nil)
787882

788883
def add_module_or_class(module_name, start_line, end_line, is_class: false, superclass_name: nil, superclass_expr: nil)
789884
comment, directives = consecutive_comment(start_line)
885+
apply_document_control_directive(directives) if directives
790886
handle_code_object_directives(@container, directives) if directives
791887
return unless @container.document_children
792888

@@ -803,7 +899,13 @@ def add_module_or_class(module_name, start_line, end_line, is_class: false, supe
803899
superclass_full_path = superclass_full_path.sub(/^::/, '')
804900
end
805901
# add_class should be done after resolving superclass
806-
mod = owner.classes_hash[name] || owner.add_class(RDoc::NormalClass, name, superclass_name || superclass_expr || '::Object')
902+
mod = owner.classes_hash[name]
903+
unless mod
904+
# add_class may return an existing class created by another file
905+
# (in_files is not empty then), which must not be ignored here
906+
mod = owner.add_class(RDoc::NormalClass, name, superclass_name || superclass_expr || '::Object')
907+
mod.ignore if document_suppressed? && mod.in_files.empty?
908+
end
807909
if superclass_name
808910
if superclass
809911
mod.superclass = superclass
@@ -812,15 +914,33 @@ def add_module_or_class(module_name, start_line, end_line, is_class: false, supe
812914
end
813915
end
814916
else
815-
mod = owner.modules_hash[name] || owner.add_module(RDoc::NormalModule, name)
917+
mod = owner.modules_hash[name]
918+
unless mod
919+
mod = owner.add_module(RDoc::NormalModule, name)
920+
mod.ignore if document_suppressed? && mod.in_files.empty?
921+
end
816922
end
817923

818924
mod.store = @store
819925
mod.line = start_line
820-
record_location(mod)
821926
handle_modifier_directive(mod, start_line)
822927
handle_modifier_directive(mod, end_line)
823-
mod.add_comment(comment, @top_level) if comment
928+
unless document_suppressed?
929+
# In a :stopdoc:/:enddoc: region, the container is still created as a
930+
# namespace but is not recorded to this file nor documented.
931+
# The body is also visited: an inner :startdoc: re-enables documentation
932+
# in a :stopdoc: region (not in an :enddoc: region), and nested
933+
# namespaces need to be created for later promotion from other files
934+
if mod.ignored?
935+
# Promotes the owner chain too, unless mod received :nodoc:
936+
mark_container_documentable(mod)
937+
else
938+
# A class/module marked :nodoc: must not make an ignored owner documentable
939+
mark_container_documentable(owner) if mod.document_self && owner.is_a?(RDoc::ClassModule)
940+
record_location(mod)
941+
end
942+
mod.add_comment(comment, @top_level) if comment
943+
end
824944
mod
825945
end
826946

@@ -975,7 +1095,9 @@ def visit_class_node(node)
9751095
end
9761096

9771097
def visit_singleton_class_node(node)
978-
@scanner.process_comments_until(node.location.start_line - 1)
1098+
# A comment linked to the `class << ...` line (e.g. a document control
1099+
# directive) belongs to the enclosing scope, not to the singleton scope
1100+
@scanner.process_comments_until(node.location.start_line)
9791101

9801102
if @scanner.has_modifier_nodoc?(node.location.start_line)
9811103
# Skip visiting inside the singleton class. Also skips creation of node.expression as a module
@@ -990,6 +1112,7 @@ def visit_singleton_class_node(node)
9901112
when Prism::ConstantWriteNode
9911113
# Accept `class << (NameErrorCheckers = Object.new)` as a module which is not actually a module
9921114
mod = @scanner.container.add_module(RDoc::NormalModule, expression.name.to_s)
1115+
mod.ignore if @scanner.document_suppressed? && mod.in_files.empty?
9931116
when Prism::ConstantPathNode, Prism::ConstantReadNode
9941117
expression_name = constant_path_string(expression)
9951118
# If a constant_path does not exist, RDoc creates a module
@@ -1150,6 +1273,7 @@ def constant_path_string(node)
11501273
end
11511274

11521275
def _visit_call_require(call_node)
1276+
return if @scanner.document_suppressed?
11531277
return unless call_node.arguments&.arguments&.size == 1
11541278
arg = call_node.arguments.arguments.first
11551279
return unless arg.is_a?(Prism::StringNode)

‎test/rdoc/generator/darkfish_test.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,8 @@ def test_setup
355355

356356
assert_equal %w[Ignored Klass Klass::A Object],
357357
[@ignored, @klass, @klass_alias, @object].map(&:full_name)
358-
assert_equal [@ignored, @klass, @klass_alias, @object],
358+
# @ignored is removed from the store on Store#complete
359+
assert_equal [@klass, @klass_alias, @object],
359360
@g.classes.sort_by { |klass| klass.full_name }
360361
assert_equal [@top_level], @g.files
361362
assert_equal [@meth, @meth, @meth_bang, @meth_bang, @meth_with_html_tag_yield, @meth_with_html_tag_yield], @g.methods

0 commit comments

Comments
 (0)