Handle old method signature in Alias/AnyMethod/Attr#1711
Conversation
`rbs` for example uses them.
Earlopain
had a problem deploying
to
fork-preview-protection
GitHub Actions
Failure
There was a problem hiding this comment.
Pull request overview
Restores backward compatibility for callers (e.g. rbs) that still invoke RDoc::Alias, RDoc::AnyMethod, and RDoc::Attr with the old method signature carrying one extra leading positional argument. Each constructor now accepts a variadic positional list, emits a :deprecated warning when the legacy arity is detected, drops the obsolete leading argument, and raises ArgumentError for any other arity.
Changes:
- Change
RDoc::Alias#initialize,RDoc::AnyMethod#initialize, andRDoc::Attr#initializeto accept*argsand dispatch onargs.size. - Emit
warn(..., uplevel: 1, category: :deprecated)when the old (extra-arg) signature is used, then shift off the leading argument. - Raise
ArgumentErrorwith a "given X, expected Y" message when the arity matches neither the new nor the legacy form.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/rdoc/code_object/alias.rb | Accept legacy 4-arg form with deprecation warning; validate arity. |
| lib/rdoc/code_object/any_method.rb | Accept legacy 2-arg form with deprecation warning; validate arity. |
| lib/rdoc/code_object/attr.rb | Accept legacy 4-arg form with deprecation warning; validate arity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| raise ArgumentError, "wrong number of arguments (given #{args.size}, expected 1)" | ||
| end | ||
|
|
||
| super(name = args[0], singleton: singleton) |
There was a problem hiding this comment.
For clarity this shows what this actually is. The other two assign it to named locals, here there is no such thing
|
Is it possible to solve the problem by:
|
|
Currently rbs has no dependency on rdoc it's just I don't know much about what rbs does (I think it creates docs from external rbs files). I plan to make a PR to rbs to change it over, but maybe the plugin should live in rdoc itself now? It would be a much better place now that it actually has a proper dependency on it. |
## Problem RDoc already supports parser classes that register handled file names through [`parse_files_matching`](https://ruby.github.io/rdoc/RDoc/Parser.html#method-c-parse_files_matching), but `.rbs` documentation parsing currently comes from the `rbs` gem's `rdoc/discover` hook, which defines `RDoc::Parser::RBS` and delegates to `RBS::RDocPlugin::Parser` at runtime ([discovery hook](https://github.com/ruby/rbs/blob/master/lib/rdoc/discover.rb), [plugin parser](https://github.com/ruby/rbs/blob/master/lib/rdoc_plugin/parser.rb)). That leaves RBS documentation generation coupled to a separately released plugin even though RDoc already uses RBS for type-signature display ([#1665](#1665)), and internal RDoc API changes can break the plugin independently of RDoc releases ([#1711](#1711)). ## Solution - Add `RDoc::Parser::RBS` inside RDoc, adapted from the existing RBS plugin parser, so `.rbs` files are registered as RDoc input and common RBS declarations/members become RDoc code objects ([RBS plugin parser](https://github.com/ruby/rbs/blob/master/lib/rdoc_plugin/parser.rb), [PR diff](https://github.com/ruby/rdoc/pull/1728/files)). - Extend existing Ruby-source documentation when matching RBS declarations are parsed, while still generating documentation for classes, modules, methods, attributes, and constants that only appear in RBS ([PR diff](https://github.com/ruby/rdoc/pull/1728/files)). - Keep RDoc's existing `sig/**/*.rbs` type-signature loading path and skip the released `rbs` discovery hook when RDoc's parser is available, so RubyGems discovery does not replace the in-tree parser ([#1665](#1665), [discovery hook](https://github.com/ruby/rbs/blob/master/lib/rdoc/discover.rb), [PR diff](https://github.com/ruby/rdoc/pull/1728/files)).
rbsfor example uses them. This is not so pretty but can be reverted eventually.rbstests pass with this.Should I mention rdoc 8 in the warning? Not sure when you'd want to actually remove this.