Feature #12482
openArgumentError.new(nil) should give a better description
Description
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.
Updated by nobu (Nobuyoshi Nakada) over 8 years ago
- 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
Updated by eike.rb (Eike Dierks) over 8 years ago
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)
Updated by duerst (Martin Dürst) over 8 years ago
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.