Feature #12482: ArgumentError.new(nil) should give a better description

• Status: Open
• Assignee: -
• Target version: -

[eike.rb (Eike Dierks)]

Currently:

ArgumentError.new(nil)  =>  #<ArgumentError: ArgumentError>
ArgumentError.new(nil).to_s => "ArgumentError"
ArgumentError.new(nil).inspect => "#<ArgumentError: ArgumentError>"

I want to suggest to change this to "ArgumentError(nil)" instead.

Rational:

For testing arguments, I frequently use:

raise ArgumentError.new(arg) unless ExpectedClass === arg

However when arg is nil, this raises "ArgumentError" while raising "ArgumentError(nil)" would be much nicer.

This would differentiate between ArgumentError.new() and ArgumentError.new(nil)

Suggested changes

It looks like ArgumentError#initialize inherits Exception#initialize

I'd like to suggest to differentiate between Exception.new() and Exception.new(nil)

def Exception.new(msg = :__Exception_msg_arg_undefined___)
  case arg
  when :__Exception_msg_arg_undefined___
    # msg = "ClassName"
  when nil
    # msg = "ClassName(nil)" 
  end

Impact

I believe this should not break existing code as no code should rely on the string returned.

[nobu (Nobuyoshi Nakada)]

• Tracker changed from Bug to Feature
• Description updated (diff)

It breaks ruby/spec.

https://github.com/ruby/ruby/compare/trunk...nobu:feature/12482-Exception-nil-mesg

diff --git i/core/marshal/dump_spec.rb w/core/marshal/dump_spec.rb
index 88ff871..64bf49b 100644
--- i/core/marshal/dump_spec.rb
+++ w/core/marshal/dump_spec.rb
@@ -470,17 +470,17 @@ describe "Marshal.dump" do
 
   describe "with an Exception" do
     it "dumps an empty Exception" do
-      Marshal.dump(Exception.new).should == "\x04\bo:\x0EException\a:\tmesg0:\abt0"
+      Marshal.dump(Exception.new).should_not =~ /mesg(?!0)/
     end
 
     it "dumps the message for the exception" do
-      Marshal.dump(Exception.new("foo")).should == "\x04\bo:\x0EException\a:\tmesgI\"\bfoo\x06:\x06EF:\abt0"
+      Marshal.dump(Exception.new("foo")).should =~ /foo/
     end
 
     it "contains the filename in the backtrace" do
       obj = Exception.new("foo")
       obj.set_backtrace(["foo/bar.rb:10"])
-      Marshal.dump(obj).should == "\x04\bo:\x0EException\a:\tmesgI\"\bfoo\x06:\x06EF:\abt[\x06I\"\x12foo/bar.rb:10\x06;\aF"
+      Marshal.dump(obj).should =~ /foo\/bar.rb:10/
     end
   end
 
diff --git i/core/marshal/shared/load.rb w/core/marshal/shared/load.rb
index 8bf5f33..5a92437 100644
--- i/core/marshal/shared/load.rb
+++ w/core/marshal/shared/load.rb
@@ -441,10 +441,7 @@ describe :marshal_load, shared: true do
   describe "for an Exception" do
     it "loads a marshalled exception with no message" do
       obj = Exception.new
-      loaded = Marshal.send(@method, "\004\bo:\016Exception\a:\abt0:\tmesg0")
-      loaded.message.should == obj.message
-      loaded.backtrace.should == obj.backtrace
-      loaded = Marshal.send(@method, "\x04\bo:\x0EException\a:\tmesg0:\abt0")
+      loaded = Marshal.send(@method, "\004\bo:\016Exception\6:\abt0")
       loaded.message.should == obj.message
       loaded.backtrace.should == obj.backtrace
     end

[eike.rb (Eike Dierks)]

OK, that change would break the spec.
I do understand that breaking the spec something that we rarely want to do.

I found a workaround:
ArgumentError.new([arg]) # wrap arg in Array to give a better error message when arg is nil
works for me.

But I still hold on with my request.
I believe the spec should be changed in this case.

The spec checks the stringification of the exception.
To my believe, no serious code should rely on this.

However there might be some existing code that relies on the stringification in localizations.

But changing the stringification would help there as well.

We can currently not diffferentiate between ArgumentError.new() and ArgumentError.new(nil),
as both result in the same error message.

I'd like to suggest to change the spec.
(it should not be graved in stone on this)

[duerst (Martin Dürst)]

Eike Dierks wrote:

OK, that change would break the spec.
I do understand that breaking the spec something that we rarely want to do.

It's not just that it formally breaks the spec. The documentation says that Exceptions take a message as an argument. This essentially means that it's the caller's responsibility to make sure the output is understandable.

Nil is not a message. Ruby is so kind as to stringify 'messages' that are not strings. Stringification of nil is "", so that is what you end up with.

I found a workaround:
ArgumentError.new([arg]) # wrap arg in Array to give a better error message when arg is nil
works for me.

What about ArgumentError.new("nil") ? That would be clearer. But in general, even that isn't very helpful, something like ArgumentError.new("Got nil, but expected ...") would be much more helpful.