Backport #1648: Rational#div Raises NoMethodError for Invalid Argument

• Status: Closed
• Assignee: shyouhei (Shyouhei Urabe)

[runpaint (Run Paint Run Run)]

=begin
$ 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
=end

[calavera (David Calavera)]

• File RUBY-1648.patch RUBY-1648.patch added

=begin
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.
=end

[RickDeNatale (Rick DeNatale)]

=begin
-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.
=end

[runpaint (Run Paint Run Run)]

=begin

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. :-)

=end

[calavera (David Calavera)]

• File RUBY-1648_v2.patch RUBY-1648_v2.patch added

=begin
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?.
=end

[marcandre (Marc-Andre Lafortune)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

=begin
Applied in changeset r25082.
=end

[shyouhei (Shyouhei Urabe)]

• Category set to ext
• Status changed from Closed to Assigned
• Assignee set to shyouhei (Shyouhei Urabe)

=begin
I'm not sure if it's a bug or a spec change. I'm thinking.
=end

[marcandre (Marc-Andre Lafortune)]

=begin
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.

=end

[shyouhei (Shyouhei Urabe)]

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

[shyouhei (Shyouhei Urabe)]

• Status changed from Assigned to Closed