Backport #1648

Rational#div Raises NoMethodError for Invalid Argument

Added by runpaint (Run Paint Run Run) almost 3 years ago. Updated about 1 year ago.

[ruby-core:23903]
Status:Assigned Start date:06/18/2009
Priority:Low Due date:
Assignee:shyouhei (Shyouhei Urabe) % Done:

100%
Category:ext
Target version:-

Description

$ ruby8 -rrational -ve 'Rational(1,2).div(nil)'
    ruby 1.8.8dev (2009-06-17) [i686-linux]
    /usr/local/lib/ruby-1.8.8/ruby/1.8/rational.rb:206:in `/': undefined method `coerce' for nil:NilClass      
    (NoMethodError)
	from /usr/local/lib/ruby-1.8.8/ruby/1.8/rational.rb:244:in `div'
	from -e:1

RUBY-1648.patch (442 Bytes) calavera (David Calavera), 08/27/2009 10:50 pm

RUBY-1648_v2.patch (443 Bytes) calavera (David Calavera), 08/28/2009 03:21 pm

Associated revisions

Revision 25082
Added by marcandre over 2 years ago

* lib/rational.rb (#+, #-, #/, #**, #<=>): Return correct error message in case coercion fails. Based on a patch by Run Paint Run Run [ruby-core:23903]

History

#1
Updated by calavera (David Calavera) over 2 years ago

I include a patch that solves this bug. The rubyspec 'spec/ruby/library/rational/div_spec.rb' fails due to this bug so you can test it's solved running it.

#2
Updated by RickDeNatale (Rick DeNatale) over 2 years ago

-1  

I believe that the current behavior is correct.

The requirement on the argument is that it respond to :coerce, not that it be a kind of Numeric.  The current exception is fine.

The patch precludes ducktyping the argument.

I can't see a spec in rubyspecs which uses a nil argument to Rational#div  where is this failing spec.  If it's there I'd argue that it shouldn't be.

#3
Updated by runpaint (Run Paint Run Run) over 2 years ago

> I believe that the current behavior is correct.

It's misleading for a NoMethodError to be raised by a method which exists when called. If the argument it recieves is inappropriate either a TypeError or ArgumentError should be raised. This is in keeping with the stdlib APIs. 

> The patch precludes ducktyping the argument.

In which case #respond_to? can be used instead. Either way the current exception is wrong. :-)

#4
Updated by calavera (David Calavera) over 2 years ago

The code of the spec is in spec/ruby/shared/rational/div.rb, but that's what fails:

ruby_bug "#1648", "1.8.7" do
    it "raises a TypeError if passed a non-numeric argument" do
      lambda { Rational(3, 4).div([]) }.should raise_error(TypeError)
    end
  end

I updated the patch to use respond_to?.

#5
Updated by marcandre (Marc-Andre Lafortune) over 2 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100
Applied in changeset r25082.

#6
Updated by shyouhei (Shyouhei Urabe) over 2 years ago

  • Category set to ext
  • Status changed from Closed to Assigned
  • Assignee set to shyouhei (Shyouhei Urabe)
I'm not sure if it's a bug or a spec change.  I'm thinking.

#7
Updated by marcandre (Marc-Andre Lafortune) over 2 years ago

Maybe I should have justified further?

In any Ruby:
$ ruby -e '1 + nil'
-e:1:in `+': nil can't be coerced into Fixnum (TypeError)
$ ruby -e '1.2 + nil'
-e:1:in `+': nil can't be coerced into Float (TypeError)
$ ruby -e '(1<<66) + nil'
-e:1:in `+': nil can't be coerced into Bignum (TypeError)
$ ruby -r bigdecimal -e 'BigDecimal("1.2") + nil'
-e:1:in `+': nil can't be coerced into BigDecimal (TypeError)

In Ruby 1.9:
$ ruby -e 'Complex(1,2) + nil'
-e:1:in `+': nil can't be coerced into Complex (TypeError)
$ ruby -e 'Rational(1,2) + nil'
-e:1:in `+': nil can't be coerced into Rational (TypeError)

This change was thus made for consistency with other mathematical classes and for consistency with Rational class of Ruby 1.9.

#8
Updated by shyouhei (Shyouhei Urabe) over 2 years ago

Yes yes it's a good thing to have basically.  The problem I'm concerning is a slight backward-incompatibility this change introduces.

Also available in: Atom PDF