Remove RDoc::MethodAttr#text#1710
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the text token-stream field from RDoc::MethodAttr (and related code objects) and updates RDoc internals/tests to construct AnyMethod, Attr, and Alias objects without passing/reading that field.
Changes:
- Remove
RDoc::MethodAttr#text(and associated initialization / pretty-printing support). - Update constructors and call sites for
RDoc::AnyMethod,RDoc::Attr, andRDoc::Aliasto no longer accept atextargument. - Update parsers and extensive test fixtures to use the new constructor signatures.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/rdoc/code_object/alias.rb | Drop text storage/access from RDoc::Alias and simplify initializer. |
| lib/rdoc/code_object/any_method.rb | Update RDoc::AnyMethod initializer and alias construction to no longer use text. |
| lib/rdoc/code_object/attr.rb | Update RDoc::Attr initializer and alias creation to no longer use text. |
| lib/rdoc/code_object/class_module.rb | Update marshal-load reconstruction to use new Attr/AnyMethod constructors. |
| lib/rdoc/code_object/method_attr.rb | Remove text reader/ivar and update base initializer + pretty_print output. |
| lib/rdoc/parser/c.rb | Update alias/attr/method object creation to use new constructors. |
| lib/rdoc/parser/ruby.rb | Update meta-method, alias, attribute, and TomDoc method creation to use new constructors. |
| lib/rdoc/ri/driver.rb | Update “missing documentation” method creation to use new AnyMethod constructor. |
| test/rdoc/code_object/alias_test.rb | Update alias test setup to new RDoc::Alias constructor. |
| test/rdoc/code_object/any_method_test.rb | Update AnyMethod construction and alias creation in tests. |
| test/rdoc/code_object/attr_test.rb | Update Attr/Alias construction in tests. |
| test/rdoc/code_object/class_module_test.rb | Update marshal/merge tests to new Attr/AnyMethod constructors. |
| test/rdoc/code_object/method_attr_test.rb | Update MethodAttr/AnyMethod construction in tests. |
| test/rdoc/generator/aliki/search_index_test.rb | Update method construction in generator search index tests. |
| test/rdoc/generator/aliki_test.rb | Update generator test fixtures to new method constructor. |
| test/rdoc/generator/darkfish_test.rb | Update generator test fixtures for methods/attrs. |
| test/rdoc/generator/json_index_test.rb | Update JSON index generator test fixtures for methods. |
| test/rdoc/generator/pot_test.rb | Update POT generator test fixtures for method/attr creation. |
| test/rdoc/generator/ri_test.rb | Update RI generator test fixtures for method/attr creation. |
| test/rdoc/markup/pre_process_test.rb | Update preprocessor tests to new method constructor. |
| test/rdoc/markup/to_html_crossref_test.rb | Update crossref HTML tests to new method constructor. |
| test/rdoc/markup/to_html_snippet_test.rb | Update HTML snippet tests to new method constructor. |
| test/rdoc/markup/to_html_test.rb | Update HTML formatter tests to new method constructor. |
| test/rdoc/parser/c_test.rb | Update C parser tests for attrs/methods with new constructors. |
| test/rdoc/rdoc_context_test.rb | Update context tests for alias/method/attr creation with new constructors. |
| test/rdoc/rdoc_cross_reference_test.rb | Update cross-reference tests to new method constructor. |
| test/rdoc/rdoc_rdoc_test.rb | Update RDoc integration test to new method constructor. |
| test/rdoc/rdoc_stats_test.rb | Update stats tests for attrs/methods created without text. |
| test/rdoc/rdoc_store_test.rb | Update store tests for method/attr/alias creation with new constructors. |
| test/rdoc/rdoc_tom_doc_test.rb | Update TomDoc tests to new method constructor. |
| test/rdoc/rdoc_top_level_test.rb | Update top-level tests for alias/method creation with new constructors. |
| test/rdoc/ri/driver_test.rb | Update RI driver tests for method/attr/alias creation with new constructors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Creates a new MethodAttr with method or attribute | ||
| # name +name+. | ||
| # | ||
| # Usually this is called by super from a subclass. | ||
|
|
||
| def initialize(text, name, singleton: false) | ||
| def initialize(name, singleton: false) | ||
| super() |
It is unused and in majority of the cases just set to nil or the empty string
5695443 to
93d32e4
Compare
|
🚀 Preview deployment available at: https://56b225c6.rdoc-6cd.pages.dev (commit: 93d32e4) |
|
I think this will fail rbs' rdoc plugin at lib/rdoc_plugin/parser.rb. I think we may need to revert this until we find a way to keep both sides in sync. |
|
Right, I thought this was internal only. How about accepting Or just revert if you don't think that is worth the effort. Let me know and I can do a PR |
|
I’m in support of cleaning up dead attributes, so if there’s a way to avoid crash without reverting then it’s worth a try |
It is unused and in majority of the cases just set to nil or the empty string