Drop Ripper-based parser#1690
Merged
st0012 merged 5 commits intoruby:masterfrom May 4, 2026
Merged
Conversation
The Prism-based Ruby parser is the default and has been the only non-opt-in parser for some time. Drop the legacy Ripper parser, its tests, and the RDOC_USE_RIPPER_PARSER opt-in path: - Delete lib/rdoc/parser/ripper_ruby.rb and test/rdoc/parser/ruby_test.rb - Simplify lib/rdoc/parser.rb to always require the Prism parser - Drop the env-gated parser_files_matching guard from PrismRuby - Remove the parallel RDocParserRipperRubyWithPrismRubyTestCasesTest test class and the accept_legacy_bug? branches it required - Drop the Ripper test step from .github/workflows/test.yml and the second documentation pass from .github/workflows/ruby-core.yml - Update AGENTS.md to reflect the single Ruby parser RipperStateLex is kept: it is a tokenizer used by the Prism parser and HTML markup output, not the deprecated parser itself.
With the legacy Ripper-based parser gone, the Prism-based parser is just "the Ruby parser". Drop the "Prism" prefix everywhere: - lib/rdoc/parser/prism_ruby.rb -> lib/rdoc/parser/ruby.rb - test/rdoc/parser/prism_ruby_test.rb -> test/rdoc/parser/ruby_test.rb - RDoc::Parser::PrismRuby -> RDoc::Parser::Ruby (drop the alias in lib/rdoc/parser.rb) - RDocParserPrismTestCases -> RDocParserRubyTestCases - RDocParserPrismRubyTest -> RDocParserRubyTest Also drop test/rdoc/rdoc_store_test.rb's using_prism_ruby_parser? helper -- it's always true now -- and the Ripper-only branches it gated. Update PrismRuby references in code comments and docs (AGENTS.md, CONTRIBUTING.md, mixin.rb, context.rb).
- Delete lib/rdoc/parser/ruby_tools.rb (and its autoload). The
RubyTools mixin -- get_tk / unget_tk / token_listener / etc. -- was
only included by the Ripper parser; the Ruby parser scans tokens
via Prism directly.
- Drop the "When mixin is created from RDoc::Parser::Ruby, module
name is already a resolved full-path name" note in mixin.rb. It
only existed to contrast against the Ripper parser's unresolved
names; with one parser left, it just mis-describes the lookup.
- Reword the find_enclosing_module_named comment in context.rb
("stopped representing module nesting" -> "does not represent
module nesting"); the past tense was relative to Ripper.
- Drop "just like the old parser did" tail from the
initialize/new rename comment in ruby.rb.
Earlopain split section-comment extraction out of Section#extract_comment in ruby#1639 and added the TODO "Remove when the ripper parser has been removed". With ripper gone, the Ruby parser's own extract_section_comment is the only path that fires; every production caller hands Section.add_comment a comment that has already been stripped of the ":section:" header. - Drop Section#extract_comment, and simplify Section#add_comment to just validate and append. - Update test_add_comment / test_description to pass the empty, pre-extracted comments the parser actually produces today. - Drop test_extract_comment. Also restore the explanatory note above Mixin#module that the prior commit removed too aggressively. The method's resolution dance is still live for the C parser (which passes unresolved local names); only the Ruby parser's mixins arrive pre-resolved.
st0012
requested a deployment
to
fork-preview-protection
GitHub Actions
Waiting
There was a problem hiding this comment.
Pull request overview
Removes the legacy Ripper-based Ruby parser path from RDoc, consolidating on the Prism-based Ruby parser (RDoc::Parser::Ruby) and deleting associated legacy tooling/tests/CI.
Changes:
- Remove Ripper parser implementation and token tooling (
ripper_ruby.rb,ruby_tools.rb) and theRDOC_USE_RIPPER_PARSERswitch. - Rename/standardize the Prism parser class to
RDoc::Parser::Rubyand update section/comment handling accordingly. - Update tests, docs, and CI workflows to reflect the single-parser world (including deleting the old Prism/Ripper shared test suite file and removing the Ripper CI runs).
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/rdoc/rdoc_store_test.rb | Removes parser-conditional expectations; adjusts singleton-class nested constant expectations |
| test/rdoc/rdoc_context_section_test.rb | Updates section comment tests to match new section-comment behavior |
| test/rdoc/parser/prism_ruby_test.rb | Deletes legacy Prism/Ripper shared parser test suite file |
| lib/rdoc/parser/ruby_tools.rb | Deletes legacy token-stream helper module used by Ripper parser |
| lib/rdoc/parser/ruby.rb | Renames Prism parser class to RDoc::Parser::Ruby and removes env-gating |
| lib/rdoc/parser/ripper_ruby.rb | Deletes legacy Ripper-based Ruby parser implementation |
| lib/rdoc/parser.rb | Removes env-based Ruby parser selection + deprecation banner; requires Ruby parser directly |
| lib/rdoc/code_object/mixin.rb | Updates docs to reflect a single Ruby parser behavior |
| lib/rdoc/code_object/context/section.rb | Simplifies section comment handling; removes legacy extraction path |
| lib/rdoc/code_object/context.rb | Updates comments to reflect new parser naming/behavior |
| CONTRIBUTING.md | Updates repository layout docs to remove Prism/Ripper split |
| AGENTS.md | Updates parser/test documentation to match single parser approach |
| .github/workflows/test.yml | Removes the Ripper-parser CI run |
| .github/workflows/ruby-core.yml | Removes the secondary “generate docs with Ripper parser” step |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
571
to
575
| def test_load_single_class | ||
| if using_prism_ruby_parser? | ||
| # Class defined inside singleton class is not documentable. | ||
| # @c8_s1 should be nil because C8::S1 does not exist. | ||
| assert_nil @c8_s1 | ||
| return | ||
| end | ||
|
|
||
| @s.save_class @c8_s1 | ||
| @s.classes_hash.clear | ||
|
|
||
| assert_equal @c8_s1, @s.load_class('C8::S1') | ||
|
|
||
| assert_includes @s.classes_hash, 'C8::S1' | ||
| # Class defined inside singleton class is not documentable. | ||
| # @c8_s1 should be nil because C8::S1 does not exist. | ||
| assert_nil @c8_s1 | ||
| end |
tompng
reviewed
May 4, 2026
| @@ -172,7 +168,6 @@ def test_all_classes_and_modules | |||
|
|
|||
| # C8::S1 does not exist. It should not be in the list. | |||
| # class C8; class << something; class S1; end; end; end | |||
Member
There was a problem hiding this comment.
This comment (and the same one on line 228) is a comment for the workaround in line 175.
I think we can remove this comment along with the workaround code below.
tompng
approved these changes
May 4, 2026
tompng pointed out on ruby#1690 that the "C8::S1 does not exist. It should not be in the list." comments in test_all_classes_and_modules and test_all_classes were anchored to the Ripper-only workaround ("expected = (expected + ['C8::S1']).sort unless using_prism_ruby_parser?") that the previous commit removed. With no workaround for them to explain, the comments are orphaned -- drop them. The matching comment in test_load_single_class stays; it explains the remaining `assert_nil @c8_s1` assertion.
st0012
requested a deployment
to
fork-preview-protection
GitHub Actions
Waiting
st0012
pushed a commit
to st0012/rdoc
that referenced
this pull request
May 4, 2026
Following up on tompng's review on ruby#1690: the C8 fixture (`class C8; class << self; class S1; end; end; end`) only existed to verify that the Ripper parser wrongly treated `S1` as `C8::S1`. The matching `expected = (expected + ['C8::S1']).sort unless using_prism_ruby_parser?` workaround in test_all_classes_and_modules / test_all_classes is gone, and test_load_single_class only asserted `assert_nil @c8_s1` -- which is just a restatement of "we removed the workaround". The whole apparatus is dead. - Delete the `class C8` fixture from test/rdoc/xref_data.rb. - Drop `@c8` and `@c8_s1` lookups from XrefTestCase. - Drop `C8` from the expected lists in test_all_classes_and_modules and test_all_classes. - Remove test_load_single_class.
st0012
requested a deployment
to
fork-preview-protection
GitHub Actions
Waiting
st0012
force-pushed
the
claude/remove-ripper-parser-jAXdK
branch
from
c1cfc6a to
fffd7f4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
|
🚀 Preview deployment available at: https://7271beb5.rdoc-6cd.pages.dev (commit: fffd7f4) |
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.
No description provided.