Make :stopdoc:/:startdoc:/:enddoc: lexically scoped in the Ruby parser - #1763
Open
tompng wants to merge 1 commit into
Open
Make :stopdoc:/:startdoc:/:enddoc: lexically scoped in the Ruby parser#1763tompng wants to merge 1 commit into
:stopdoc:/:startdoc:/:enddoc: lexically scoped in the Ruby parser#1763tompng wants to merge 1 commit into
Conversation
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 (ruby#398, ruby#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 <noreply@anthropic.com>
tompng
requested a deployment
to
fork-preview-protection
August 1, 2026 16:43 — with
GitHub Actions
Waiting
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Prism-based Ruby parser to make region directives (:stopdoc:, :startdoc:, :enddoc:) lexically scoped to the current syntactic container, preventing directive state from leaking across class/module reopenings and across files. It also aligns store cleanup behavior with the documented meaning of CodeObject#ignore and adds targeted regression tests for the new semantics.
Changes:
- Track document-control state in the Ruby parser (
@doc_state) and restore it on scope exit (with_container) so region directives are lexical rather than mutating sharedCodeObjectstate. - Ensure namespaces created in suppressed regions are created as ignored placeholders, and promote them to documentable when they later receive documentable contents outside the suppressed region.
- Add extensive parser tests covering leakage prevention, nested overrides, singleton-class behavior, and interactions with
:nodoc:; adjust Darkfish generator expectations for ignored contexts being removed onStore#complete.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/rdoc/parser/ruby.rb |
Implements lexically-scoped startdoc/stopdoc/enddoc via parser state, suppression checks, and namespace promotion logic. |
lib/rdoc/code_object/context.rb |
Extends remove_from_documentation? to also prune ignored contexts that never gain documentable contents. |
test/rdoc/parser/ruby_test.rb |
Adds regression tests to validate lexical scoping and non-leaking behavior across scopes, singleton classes, and files. |
test/rdoc/generator/darkfish_test.rb |
Updates expectations to reflect ignored contexts being removed during store completion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+917
to
+919
| # 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
CodeObjectstate (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 in the same file and into other files (#398). 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: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 asdone_documentingglobally.: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 onStore#complete(Context#remove_from_documentation?now also covers ignored contexts, which is whatCodeObject#ignorewas documented to do in GH :stopdoc: directive produces Object reference #55).class/modulebodies,class << self, and definition-creating blocks).if/while/begindo not create scopes, consistent with Ruby's own scoping rules.Compatibility notes
:nodoc:is untouched. However,:startdoc:additionally callsstart_docon the current container, becausemodule 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.class << Xline 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'send(this pattern appears in net/http).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
liband 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::stopdoc:regions (Ripper = Prism::Translation::Ripperin 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 toPrism::Translation::Ripper. Note: the old accidental link onHashWithIndifferentAccessin Hash.html was arguably useful; writing the full name in the comment restores it.Net::HTTPResponseregains itsinclude Net::HTTPHeader. The old implementation lost it because a:stopdoc:insideclass << selfleaked 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.Net::HTTPBadResponseregion in net/http.rb,Timeout::Request) no longer produce empty pages.Follow-ups (out of scope here)
:nodoc:(the hard part of Lexical scope document control directives #1591); thestart_doccompatibility shim and thereceived_nodocguard in promotion can be removed then.:stopdoc:, directives after:enddoc:, standalone:nodoc:in the middle of a class body).file:line: stopdoc applied to X) as proposed in Lexical scope document control directives #1591 (comment); the parser now has the lexical information to make this cheap.