Project

General

Profile

Actions

Feature #16697

closed

Hash.ruby2_keywords_hash?(value) should support any object

Added by Eregon (Benoit Daloze) about 4 years ago. Updated over 3 years ago.

Status:
Rejected
Assignee:
-
Target version:
-
[ruby-core:97554]

Description

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.

Actions #1

Updated by hsbt (Hiroshi SHIBATA) over 3 years ago

  • Target version changed from 36 to 3.0

Updated by jbeschi (jacopo beschi) over 3 years ago

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 ?

Updated by Eregon (Benoit Daloze) over 3 years ago

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.

Actions #4

Updated by Eregon (Benoit Daloze) over 3 years ago

  • 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)

Updated by jbeschi (jacopo beschi) over 3 years ago

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

Updated by mame (Yusuke Endoh) over 3 years ago

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?

Updated by Dan0042 (Daniel DeLorme) over 3 years ago

Maybe return nil if the object is not a Hash?

Updated by jbeschi (jacopo beschi) over 3 years ago

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.

Updated by mame (Yusuke Endoh) over 3 years ago

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.

Updated by jbeschi (jacopo beschi) over 3 years ago

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

Updated by mame (Yusuke Endoh) over 3 years ago

  • 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.

Updated by Eregon (Benoit Daloze) over 3 years ago

  • Status changed from Feedback to Closed

OK, let's reject this then.

Actions #14

Updated by Eregon (Benoit Daloze) over 3 years ago

  • Status changed from Closed to Rejected
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0