Feature #16697: Hash.ruby2_keywords_hash?(value) should support any object

• Status: Rejected
• Assignee: -
• Target version: -

[Eregon (Benoit Daloze)]

But currently it raises, which makes it needlessly inconvenient to use:

> Hash.ruby2_keywords_hash?({})
=> false
> Hash.ruby2_keywords_hash?("hello")
Traceback (most recent call last):
        5: from /home/eregon/.rubies/ruby-trunk/bin/irb:23:in `<main>'
        4: from /home/eregon/.rubies/ruby-trunk/bin/irb:23:in `load'
        3: from /home/eregon/prefix/ruby-trunk/lib/ruby/gems/2.8.0/gems/irb-1.2.3/exe/irb:11:in `<top (required)>'
        2: from (irb):4
        1: from (irb):4:in `ruby2_keywords_hash?'
TypeError (wrong argument type String (expected Hash))

See https://github.com/ruby/ruby/pull/2966/files#r394741112 for a motivating example.

I'd like to suggest backporting this to 2.7 too.

[hsbt (Hiroshi SHIBATA)]

• Target version changed from 36 to 3.0

[jbeschi (jacopo beschi)]

I'd like to contribute here but I'm new to this so please forgive me if this is a stupid question: we basically just need to change ruby2_keywords_hash? to return false if the argument is not an Hash instead of raising a TypeError ?

[Eregon (Benoit Daloze)]

I think we need to decide if this change is accepted, I'll add it to the dev meeting agenda.

Indeed, the concrete change would just be that, feel free to give it a try.

[Eregon (Benoit Daloze)]

• Tracker changed from Bug to Feature
• ruby -v deleted (ruby 2.8.0dev (2020-02-28T14:32:56Z master f7be85a2b7) [x86_64-linux])
• Backport deleted (2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN)

[jbeschi (jacopo beschi)]

Thanks! I'll give it a try :-)

[jbeschi (jacopo beschi)]

I've created a PR for the proposal https://github.com/ruby/ruby/pull/3839

[mame (Yusuke Endoh)]

I'm a bit afraid if changing the behavior would bring confusion rather than trivial usability. It is easy to workaround the issue.

Anyway, if it is changed, we need to backport the patch to ruby_2_7. If ruby_2_7 and master are inconsistent, it is more confusing. @nagachika (Tomoyuki Chikanaga) -san, will you backport it if it is accepted?

[Dan0042 (Daniel DeLorme)]

Maybe return nil if the object is not a Hash?

[jbeschi (jacopo beschi)]

Honestly, I don't think returning nil would be an improvement: using nil as a type check looks a bit hacky to me.

Anyways I'd wait for the devs meeting outcome and see if we can move this forward.

[mame (Yusuke Endoh)]

jbeschi (jacopo beschi) wrote in #note-9:

Anyways I'd wait for the devs meeting outcome and see if we can move this forward.

My comment #7 is the meeting outcome. Some committers including matz discussed this issue, but we were not sure if the change is really needed. Even if we change it in Ruby 2.7.3 and 3.0.0, people cannot depend on the new behavior if they care users of Ruby 2.7.2. Also, while it is trivial to workaround the issue, changing the behavior may bring new confusion.

Anyway, this issue must be handled not only in master but also in ruby 2.7 branch. @nagachika (Tomoyuki Chikanaga) , who is Ruby 2.7 branch maintainer, didn't attend the meeting. So we first ask @nagachika (Tomoyuki Chikanaga) 's opinion.

[jbeschi (jacopo beschi)]

@mame (Yusuke Endoh) Thanks for clarifying! Ok let's see :)

[mame (Yusuke Endoh)]

• Status changed from Open to Feedback
• Target version deleted (3.0)

I don't think we will be able to decide this issue by Ruby 3.0.0. But changing it after Ruby 3.0.0 seems more confusing. I vote for WONTFIX if there is no strong reason.

[Eregon (Benoit Daloze)]

• Status changed from Feedback to Closed

OK, let's reject this then.

[Eregon (Benoit Daloze)]

• Status changed from Closed to Rejected