Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Returned Proc of Symbol#to_proc should raise an ArgumentError when calling #call without receiver parameter #2312

Merged
merged 2 commits into from Apr 24, 2013

Conversation

Projects
None yet
2 participants
Member

kachick commented Apr 24, 2013

MRI

  • ruby 1.8.7 (2012-10-12 patchlevel 371)
  • ruby 1.9.3p392 (2013-02-22 revision 39386)
:object_id.to_proc.call #=> ArgumentError: no receiver given
:DUMMY.to_proc.call     #=> ArgumentError: no receiver given

Rubinius

  • rubinius 2.0.0.rc1 (1.8.7 aacddf7 yyyy-mm-dd JI)
  • rubinius 2.0.0.rc1 (1.9.3 aacddf7 yyyy-mm-dd JI)
:object_id.to_proc.call #=> 26
:DUMMY.to_proc.call     #=> NoMethodError: undefined method `DUMMY' on nil:NilClass.

@dbussink dbussink commented on the diff Apr 24, 2013

kernel/common/symbol18.rb
@@ -26,6 +26,9 @@ def to_proc
# we leave the symbol in sym and use it in the block.
#
sym = self
- Proc.new { |*args| args.shift.__send__(sym, *args) }
+ Proc.new do |*args|
+ raise ArgumentError, "no receiver given" if args.empty?
+ args.shift.__send__(sym, *args)
@dbussink

dbussink Apr 24, 2013

Owner

Since we always shift here, would it be better to do a nil check against that?

recv = args.shift
raise ArgumentError, "no receiver given" unless recv
recv.__send__(sym, *args)

Or perhaps already extract the recv in the block arguments?

@dbussink

dbussink Apr 24, 2013

Owner

Something like this:

    Proc.new do |recv, *args|
      raise ArgumentError, "no receiver given" unless recv
      recv.__send__(sym, *args)
    end
@kachick

kachick Apr 24, 2013

Member

Thank you.

recv = args.shift
raise ArgumentError, "no receiver given" unless recv

I'm afraid when receiver is nil.

nil.tap(&:display)

My firstd idea is |recv, *args| .
But that is modified arity number.

@dbussink

dbussink Apr 24, 2013

Owner

Hmm, the arity argument is right yeah. Probably best to with this approach for now then yeah.

@kachick

kachick Apr 24, 2013

Member

Thank you for the catch!

@dbussink

dbussink Apr 24, 2013

Owner

Thank you for having this thought through better than I did when commenting :).

@dbussink dbussink added a commit that referenced this pull request Apr 24, 2013

@dbussink dbussink Merge pull request #2312 from kachick/fix-symbol-to_proc
Returned Proc of Symbol#to_proc should raise an ArgumentError when calling #call without receiver parameter
118d7e2

@dbussink dbussink merged commit 118d7e2 into rubinius:master Apr 24, 2013

1 check was pending

default The Travis build is in progress
Details

@kachick kachick deleted the kachick:fix-symbol-to_proc branch Apr 24, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment