Feature #22205
openDeprecate ruby2_keywords
Description
I believe ruby2_keywords should be removed in the future, but removing it now would cause compatibility issues. As a first step, how about adding deprecation warnings to ruby2_keywords?
At least the following methods should emit a warning:
- Module#ruby2_keywords
- main.ruby2_keywords
- Proc#ruby2_keywords
The following methods are relatively harmless, so it might be better not to warn on them in the first step:
- Hash.ruby2_keywords_hash?
- Hash.ruby2_keywords_hash
Background and Motivation¶
While implementing Proc#refined, I noticed that the ruby2_keywords flag on an iseq (param.flags.ruby2_keywords) can be mutated by Proc#ruby2_keywords even when the iseq is shared among multiple Ractors, which is not Ractor-safe.
I consider ruby2_keywords a negative legacy, and I would rather deprecate it than fix such behavior.
Schedule¶
For Module#ruby2_keywords, main.ruby2_keywords, and Proc#ruby2_keywords¶
- 4.1: Documentation-only deprecation
- 4.2: Verbose-mode deprecation warning
- 4.3: Non-verbose-mode deprecation warning
- 4.4: Removal
For Hash.ruby2_keywords_hash?, Hash.ruby2_keywords_hash¶
- 4.1: Documentation-only deprecation
- 4.2: No warnings, still working as migration tools
- 4.3: Verbose-mode deprecation warning
- 4.4: Non-verbose-mode deprecation warning. Hash.ruby2_keywords_hash? always returns false, and Hash.ruby2_keywords_hash returns a plain duplicate of the given hash
- 4.5: Removal
Updated by shugo (Shugo Maeda) 4 days ago
- Description updated (diff)
Updated by mame (Yusuke Endoh) 4 days ago
- Related to Feature #14183: "Real" keyword argument added
Updated by mame (Yusuke Endoh) 4 days ago
I agree with this proposal. ruby2_keywords was a compromise we reluctantly introduced so that a single code base could behave exactly the same on Rubies before and after 3.0. Now that all of those versions have reached EOL, explicit delegation with foo(*args, **opts) or foo(...) should be all we need. In fact, its documentation has stated from the beginning that "This method will probably be removed at some point, as it exists only for backwards compatibility", so deprecating it now is just following through on that long-standing announcement.
When we were designing the keyword argument separation, @jeremyevans0 (Jeremy Evans), @Eregon (Benoit Daloze), and I had a very long discussion searching for a migration path that would need only a single rewrite, but we could never find one. I still regret that. On the other hand, the deliberately awkward name was chosen precisely so that nobody would ever want to keep it forever. I'm glad to see it finally serving that purpose.
Updated by Earlopain (Earlopain _) 4 days ago
ruby2_keywords still sees quite a bit of usage in the ecosystem.
I think many can easily be migrated now that ruby 3.0 is long EOL already and gems have their required_ruby_version set higher (like https://github.com/rails/bootsnap/pull/559).
Some use it for performance I believe (and to not break their API contract) but I'm not sure how true that is anymore these days.
Often happens when serializing to other places like a database etc.
Updated by jeremyevans0 (Jeremy Evans) 4 days ago
I figured this day would come eventually. :) There is no doubt that ruby2_keywords significantly complicates the internals, and enough time has passed that I'm not against this change.
That said, removing ruby2_keywords will make it more difficult to continue to support Ruby versions before 3.0 (it's still possible, but it requires ugly code duplication). Certainly in my libraries (most of which currently still support Ruby 1.9.2+), if the library currently uses ruby2_keywords, I'll drop compatibility for Ruby 1.9 and 2.x.
I request an extended deprecation cycle for this:
4.1: Documentation-only deprecation
4.2: Verbose-mode deprecation warning
4.3: Non-verbose-mode deprecation warning
4.4: Removal
If that is too long, maybe:
4.1: Documentation-only deprecation
4.2: Non-verbose-mode deprecation warning
4.3: Removal
I'd like at least Ruby 4.1 to be documentation-only deprecation so the community (myself included) has sufficient time (17 months instead of 5 months) to replace use of ruby2_keywords before it starts emitting deprecation warnings.
Updated by byroot (Jean Boussier) 4 days ago
If ruby2_keywords is deprecated, I'll also likely drop support for 2.x rubies in a bunch of gems.
I suppose there isn't much reason to keep it, for a while ruby2_keywords was the most efficient way to do blind delegation, but since the introduction and improvement of ..., it's now only really useful in very rare cases.
What I'd request though, but perhaps could be better discussed in a dedicated Misc ticket, is that if it's deprecated, we agree upfront on a timeline and include it in the deprecation message. We have a bad habit of deprecating things without a well understood timeline and it makes managing deprecations harder.
Updated by shugo (Shugo Maeda) 3 days ago
Thank you all for the feedback.
I think the timeline for ruby2_keywords itself should be decided in this ticket, as we usually do for each deprecation. If we want to discuss a general policy for deprecation timelines, a dedicated Misc ticket would be welcome.
I agree with @jeremyevans0 (Jeremy Evans) that we should start with a documentation-only deprecation, so that people have enough time to migrate before the warnings begin. I like this schedule as a starting point:
- 4.1: Documentation-only deprecation
- 4.2: Verbose-mode deprecation warning
- 4.3: Non-verbose-mode deprecation warning
- 4.4: Removal
I agree with @byroot (Jean Boussier) that the timeline should be clear from the beginning, and we can include the planned removal version in the warning message.
Updated by shugo (Shugo Maeda) 3 days ago
I forgot to consider Hash.ruby2_keywords_hash? and Hash.ruby2_keywords_hash in the above timeline. How about handling them one phase later, because they are needed to deal with flagged hashes during the migration period?
- 4.1: Documentation-only deprecation
- 4.2: No warnings, still working as migration tools
- 4.3: Verbose-mode deprecation warning
- 4.4: Non-verbose-mode deprecation warning. Hash.ruby2_keywords_hash? always returns false, and Hash.ruby2_keywords_hash returns a plain duplicate of the given hash
- 4.5: Removal
The compatibility shims in 4.4 are for gems that support both 4.3 and 4.4, so that unconditional calls to these methods do not raise NoMethodError.
Updated by jeremyevans0 (Jeremy Evans) 3 days ago
I like @shugo (Shugo Maeda) 's proposed plan to deprecate Hash.ruby2_keywords_hash? and Hash.ruby2_keywords_hash.
Updated by shugo (Shugo Maeda) 2 days ago
- Description updated (diff)
@jeremyevans0 (Jeremy Evans) Thanks for your confirmation. I've added the schedule to the description.
Updated by shugo (Shugo Maeda) 2 days ago
- Subject changed from Add deprecation warnings for ruby2_keywords to Deprecate ruby2_keywords
Updated by shugo (Shugo Maeda) 2 days ago
I've created a pull request: https://github.com/ruby/ruby/pull/18044
Updated by Eregon (Benoit Daloze) about 11 hours ago
Big +1 from me, and agreed it's definitely worth deprecating them but also there is no immediate need so the proposed schedule sounds good to me.
The main thing I'm interested to remove is the special dispatch on foo(*args) when args.last is a ruby2_keywords-marked Hash.
For that we need all these methods to be removed (or become noop, so 4.4 in the current proposed schedule).
My only question would be whether aligning both schedules for the methods might make sense, so it's more "consistent/simpler" for users.
shugo (Shugo Maeda) wrote in #note-8:
they are needed to deal with flagged hashes during the migration period?
Could you give an example? I'm not sure to follow.
Updated by Eregon (Benoit Daloze) about 11 hours ago
Ah one more thing I wanted to mention is it might still be possible to support Ruby 2.7 in gems without ruby2_keywords by using def foo(...); bar(...); end which was introduced in Ruby 2.7.2 (although that does not cover all delegation cases). But I wouldn't recommend it, having to reason about "keywords arguments in Ruby < 3" and "keywords arguments in Ruby >= 3" is very hard as they are quite different concepts.
Updated by shugo (Shugo Maeda) about 7 hours ago
Eregon (Benoit Daloze) wrote in #note-13:
My only question would be whether aligning both schedules for the methods might make sense, so it's more "consistent/simpler" for users.
shugo (Shugo Maeda) wrote in #note-8:
they are needed to deal with flagged hashes during the migration period?
Could you give an example? I'm not sure to follow.
Here is an example of Hash.ruby2_keywords_hash? in rspec-mocks: https://github.com/rspec/rspec/blob/8098cb121a440cbc1f9dc4581ac65edbd9f001c8/rspec-mocks/lib/rspec/mocks/argument_list_matcher.rb#L66-L70
Hash.ruby2_keywords_hash is used in ActiveJob to restore the flag lost by serialization: https://github.com/rails/rails/blob/be7436208a9eecd82e9d52c960bfe692c1c3263b/activejob/lib/active_job/arguments.rb#L174
The creator of flagged hashes and the code handling them can also be separate libraries. For example, ActiveRecord::Migration::CommandRecorder records arguments with ruby2_keywords-marked methods, and plugins such as scenic use Hash.ruby2_keywords_hash to restore the flag when they rebuild the recorded arguments for revert: https://github.com/scenic-views/scenic/blob/f2162dbddb0fb0eb5d4b04d640dcec5303a387e8/lib/scenic/command_recorder/statement_arguments.rb#L39-L58
In such cases, a plugin cannot finish the migration alone; it needs the Hash methods until the framework stops passing flagged hashes, so the migration requires coordination across libraries.
Such code needs the Hash methods to keep working, as long as flagged hashes can exist. That is why their warning phases are one version later in the schedule that I proposed. That said, if we want to push the migration more aggressively, it may also be possible to align both schedules and remove the Hash methods together with the marking methods.