Fix Ruby parser to not create a class whose superclass references itself - #1784
Merged
Conversation
Context#add_class will create a class with nil superclass in some case: BasicObject, superclass referencing itself. Ruby parser should support handling class with nil superclass, and also superclass overwrite mechanism should also follow this rule.
tompng
requested a deployment
to
fork-preview-protection
August 19, 2026 06:44 — with 
GitHub Actions
Waiting
GitHub Actions
Waiting
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts the Prism-based Ruby parser’s class-superclass handling to avoid creating/overwriting superclasses in cases that should result in a nil superclass (notably BasicObject and self-referential superclass paths), preventing crashes when later code assumes a non-nil superclass object.
Changes:
- Tighten superclass overwrite rules in
RDoc::Parser::Ruby#add_module_or_classto avoid invalid/self-referential superclass assignments and toleratenilsuperclasses. - Add a regression test covering
BasicObjectand self-superclass edge cases inContext#add_class/ parser overwrite behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/rdoc/parser/ruby.rb | Updates superclass overwrite logic to avoid nil crashes and prevent invalid overwrites for BasicObject and self-referential superclass paths. |
| test/rdoc/parser/ruby_test.rb | Adds coverage for nil superclass edge cases and (with suggested update) should include the #1781 nested-class repro. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if superclass | ||
| mod.superclass = superclass | ||
| elsif (mod.superclass.is_a?(String) || mod.superclass.name == 'Object') && mod.superclass != superclass_full_path | ||
| elsif mod.superclass.nil? || (mod.superclass.is_a?(String) || mod.superclass.name == 'Object') && mod.superclass != superclass_full_path |
Comment on lines
+910
to
+914
| # Superclass with the same full path and superclass for BasicObject are not allowed | ||
| if superclass_name && mod.full_name != superclass_full_path && mod.full_name != 'BasicObject' | ||
| if superclass | ||
| mod.superclass = superclass | ||
| elsif (mod.superclass.is_a?(String) || mod.superclass.name == 'Object') && mod.superclass != superclass_full_path | ||
| elsif mod.superclass.nil? || (mod.superclass.is_a?(String) || mod.superclass.name == 'Object') && mod.superclass != superclass_full_path |
Comment on lines
+3026
to
+3047
| def test_nil_superclass_case | ||
| # Context#add_class will create a class with nil superclass when | ||
| # the class is a BasicObject, or a superclass has the same full_name as the class itself. | ||
| # Superclass overwrite should follow this rule. | ||
| util_parser <<~RUBY | ||
| class BasicObject < Super; end | ||
| class BasicObject < Super; end | ||
| class Foo < Foo; end | ||
| class Bar < Bar; end | ||
| class Bar < Super; end | ||
| module M | ||
| class Baz < Super; end | ||
| class Baz < Baz; end | ||
| class Baz < M::Baz; end | ||
| end | ||
| RUBY | ||
|
|
||
| assert_nil @store.find_class_named('BasicObject').superclass | ||
| assert_nil @store.find_class_named('Foo').superclass | ||
| assert_equal 'Super', @store.find_class_named('Bar').superclass | ||
| assert_equal 'Super', @store.find_class_named('M::Baz').superclass | ||
| end |
st0012
approved these changes
Aug 23, 2026
tompng
added a commit
that referenced
this pull request
Aug 24, 2026
Related to #1781 and #1782 Fix crash when a class has a superclass cycle. Superclass chain can have a cycle. Some cycle can be rejected in parse phase(#1784), but I think some manually crafted case aren't possible to reject in parse phase. So `ancestors` `superclasses` must handle cyclic chain for now. Even after #1784, superclass cycle can be created by: ```ruby class A < B; end class B < C; end class C < B; end ``` (Perhaps there's an option to add validation phase that detects and fixes cyclic chain in the future) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Fixes #1781
Context#add_classwill create a class with nil superclass in some case: BasicObject, superclass referencing itself.Ruby parser should support handling class with nil superclass, and also superclass overwrite mechanism should also follow this rule.