https://redmine.ruby-lang.org/https://redmine.ruby-lang.org/favicon.ico?17113305112016-06-12T01:25:59ZRuby Issue Tracking SystemRuby master - Feature #12482: ArgumentError.new(nil) should give a better descriptionhttps://redmine.ruby-lang.org/issues/12482?journal_id=591522016-06-12T01:25:59Znobu (Nobuyoshi Nakada)nobu@ruby-lang.org
<ul><li><strong>Tracker</strong> changed from <i>Bug</i> to <i>Feature</i></li><li><strong>Description</strong> updated (<a title="View differences" href="/journals/59152/diff?detail_id=41602">diff</a>)</li></ul><p>It breaks ruby/spec.</p>
<p><a href="https://github.com/ruby/ruby/compare/trunk...nobu:feature/12482-Exception-nil-mesg" class="external">https://github.com/ruby/ruby/compare/trunk...nobu:feature/12482-Exception-nil-mesg</a></p>
<pre><code class="diff syntaxhl" data-language="diff"><span class="gh">diff --git i/core/marshal/dump_spec.rb w/core/marshal/dump_spec.rb
index 88ff871..64bf49b 100644
</span><span class="gd">--- i/core/marshal/dump_spec.rb
</span><span class="gi">+++ w/core/marshal/dump_spec.rb
</span><span class="p">@@ -470,17 +470,17 @@</span> describe "Marshal.dump" do
describe "with an Exception" do
it "dumps an empty Exception" do
<span class="gd">- Marshal.dump(Exception.new).should == "\x04\bo:\x0EException\a:\tmesg0:\abt0"
</span><span class="gi">+ Marshal.dump(Exception.new).should_not =~ /mesg(?!0)/
</span> end
it "dumps the message for the exception" do
<span class="gd">- Marshal.dump(Exception.new("foo")).should == "\x04\bo:\x0EException\a:\tmesgI\"\bfoo\x06:\x06EF:\abt0"
</span><span class="gi">+ Marshal.dump(Exception.new("foo")).should =~ /foo/
</span> end
it "contains the filename in the backtrace" do
obj = Exception.new("foo")
obj.set_backtrace(["foo/bar.rb:10"])
<span class="gd">- Marshal.dump(obj).should == "\x04\bo:\x0EException\a:\tmesgI\"\bfoo\x06:\x06EF:\abt[\x06I\"\x12foo/bar.rb:10\x06;\aF"
</span><span class="gi">+ Marshal.dump(obj).should =~ /foo\/bar.rb:10/
</span> end
end
<span class="gh">diff --git i/core/marshal/shared/load.rb w/core/marshal/shared/load.rb
index 8bf5f33..5a92437 100644
</span><span class="gd">--- i/core/marshal/shared/load.rb
</span><span class="gi">+++ w/core/marshal/shared/load.rb
</span><span class="p">@@ -441,10 +441,7 @@</span> describe :marshal_load, shared: true do
describe "for an Exception" do
it "loads a marshalled exception with no message" do
obj = Exception.new
<span class="gd">- 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")
</span><span class="gi">+ loaded = Marshal.send(@method, "\004\bo:\016Exception\6:\abt0")
</span> loaded.message.should == obj.message
loaded.backtrace.should == obj.backtrace
end
</code></pre> Ruby master - Feature #12482: ArgumentError.new(nil) should give a better descriptionhttps://redmine.ruby-lang.org/issues/12482?journal_id=592452016-06-15T20:19:42Zeike.rb (Eike Dierks)eike@inter.net
<ul></ul><p>OK, that change would break the spec.<br>
I do understand that breaking the spec something that we rarely want to do.</p>
<p>I found a workaround:<br>
ArgumentError.new([arg]) # wrap arg in Array to give a better error message when arg is nil<br>
works for me.</p>
<p>But I still hold on with my request.<br>
I believe the spec should be changed in this case.</p>
<p>The spec checks the stringification of the exception.<br>
To my believe, no serious code should rely on this.</p>
<p>However there might be some existing code that relies on the stringification in localizations.</p>
<p>But changing the stringification would help there as well.</p>
<p>We can currently not diffferentiate between ArgumentError.new() and ArgumentError.new(nil),<br>
as both result in the same error message.</p>
<p>I'd like to suggest to change the spec.<br>
(it should not be graved in stone on this)</p> Ruby master - Feature #12482: ArgumentError.new(nil) should give a better descriptionhttps://redmine.ruby-lang.org/issues/12482?journal_id=592482016-06-16T04:18:29Zduerst (Martin Dürst)duerst@it.aoyama.ac.jp
<ul></ul><p>Eike Dierks wrote:</p>
<blockquote>
<p>OK, that change would break the spec.<br>
I do understand that breaking the spec something that we rarely want to do.</p>
</blockquote>
<p>It's not just that it formally breaks the spec. The documentation says that Exceptions take a <em>message</em> as an argument. This essentially means that it's the caller's responsibility to make sure the output is understandable.</p>
<p>Nil is not a message. Ruby is so kind as to stringify 'messages' that are not strings. Stringification of <em>nil</em> is "", so that is what you end up with.</p>
<blockquote>
<p>I found a workaround:<br>
ArgumentError.new([arg]) # wrap arg in Array to give a better error message when arg is nil<br>
works for me.</p>
</blockquote>
<p>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.</p>