Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update NoMethodError message to not use object#inspect #3749

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Compatibility:
* Support `symbolize_names` argument to `MatchData#named_captures` (#3681, @rwstauner).
* Support `Proc#initialize_{dup,copy}` for subclasses (#3681, @rwstauner).
* Remove deprecated `Encoding#replicate` method (#3681, @rwstauner).
* Update `NoMethodError#message` to not use `#inspect` on receiver (#3681, @rwstauner).

Performance:

Expand Down
3 changes: 3 additions & 0 deletions spec/ruby/core/exception/fixtures/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ class NoMethodErrorD; end

class InstanceException < Exception
end

class AClass; end
module AModule; end
end

class NameErrorSpecs
Expand Down
185 changes: 145 additions & 40 deletions spec/ruby/core/exception/no_method_error_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
end

ruby_version_is ""..."3.3" do
it "calls receiver.inspect only when calling Exception#message" do
it "calls #inspect when calling Exception#message" do
ScratchPad.record []
test_class = Class.new do
def inspect
Expand All @@ -76,68 +76,173 @@ def inspect
end
end
instance = test_class.new

begin
instance.bar
rescue Exception => e
e.name.should == :bar
ScratchPad.recorded.should == []
e.message.should =~ /undefined method.+\bbar\b/
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']bar' for <inspect>:#<Class:0x\h+>$/
ScratchPad.recorded.should == [:inspect_called]
end
end
end

ruby_version_is "3.3" do
it "does not call receiver.inspect even when calling Exception#message" do
ScratchPad.record []
it "fallbacks to a simpler representation of the receiver when receiver.inspect raises an exception" do
test_class = Class.new do
def inspect
ScratchPad << :inspect_called
"<inspect>"
raise NoMethodErrorSpecs::InstanceException
end
end
instance = test_class.new

begin
instance.bar
rescue Exception => e
e.name.should == :bar
ScratchPad.recorded.should == []
e.message.should =~ /undefined method.+\bbar\b/
ScratchPad.recorded.should == []
rescue NoMethodError => error
message = error.message
message.should =~ /undefined method.+\bbar\b/
message.should include test_class.inspect
end
end
end

it "fallbacks to a simpler representation of the receiver when receiver.inspect raises an exception" do
test_class = Class.new do
def inspect
raise NoMethodErrorSpecs::InstanceException
it "uses #name to display the receiver if it is a class" do
klass = Class.new { def self.name; "MyClass"; end }

begin
klass.foo
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']foo' for MyClass:Class$/
end
end
instance = test_class.new
begin
instance.bar
rescue Exception => e
e.name.should == :bar
message = e.message
message.should =~ /undefined method.+\bbar\b/
message.should include test_class.inspect

it "uses #name to display the receiver if it is a module" do
mod = Module.new { def self.name; "MyModule"; end }

begin
mod.foo
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']foo' for MyModule:Module$/
end
end
end

it "uses #name to display the receiver if it is a class or a module" do
klass = Class.new { def self.name; "MyClass"; end }
begin
klass.foo
rescue NoMethodError => error
error.message.lines.first.chomp.should =~ /^undefined method [`']foo' for /
ruby_version_is "3.3" do
it "uses a literal name when receiver is nil" do
begin
nil.foo
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']foo' for nil\Z/
end
end

mod = Module.new { def self.name; "MyModule"; end }
begin
mod.foo
rescue NoMethodError => error
error.message.lines.first.chomp.should =~ /^undefined method [`']foo' for /
it "uses a literal name when receiver is true" do
begin
true.foo
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']foo' for true\Z/
end
end

it "uses a literal name when receiver is false" do
begin
false.foo
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']foo' for false\Z/
end
end

it "uses #name when receiver is a class" do
begin
NoMethodErrorSpecs::AClass.foo
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']foo' for class NoMethodErrorSpecs::AClass\Z/
end
end

it "uses class' string representation when receiver is an anonymous class" do
klass = Class.new

begin
klass.foo
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']foo' for class #<Class:0x\h+>\Z/
end
end

it "uses class' string representation when receiver is a singleton class" do
obj = Object.new
singleton_class = obj.singleton_class

begin
singleton_class.foo
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']foo' for class #<Class:#<Object:0x\h+>>\Z/
end
end

it "uses #name when receiver is a module" do
begin
NoMethodErrorSpecs::AModule.foo
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']foo' for module NoMethodErrorSpecs::AModule\Z/
end
end

it "uses module's string representation when receiver is an anonymous module" do
m = Module.new

begin
m.foo
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']foo' for module #<Module:0x\h+>\Z/
end
end

it "uses class name when receiver is an ordinary object" do
instance = NoMethodErrorSpecs::NoMethodErrorA.new

begin
instance.bar
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']bar' for an instance of NoMethodErrorSpecs::NoMethodErrorA\Z/
end
end

it "uses class string representation when receiver is an instance of anonymous class" do
klass = Class.new
instance = klass.new

begin
instance.bar
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']bar' for an instance of #<Class:0x\h+>\Z/
end
end

it "uses class name when receiver has a singleton class" do
instance = NoMethodErrorSpecs::NoMethodErrorA.new
def instance.foo; end

begin
instance.bar
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']bar' for #<NoMethodErrorSpecs::NoMethodErrorA:0x\h+>\Z/
end
end

it "does not call #inspect when calling Exception#message" do
ScratchPad.record []
test_class = Class.new do
def inspect
ScratchPad << :inspect_called
"<inspect>"
end
end
instance = test_class.new

begin
instance.bar
rescue NoMethodError => error
error.message.should =~ /\Aundefined method [`']bar' for an instance of #<Class:0x\h+>\Z/
ScratchPad.recorded.should == []
end
end
end
end
Expand Down
1 change: 0 additions & 1 deletion spec/tags/core/exception/no_method_error_tags.txt

This file was deleted.

19 changes: 12 additions & 7 deletions src/main/ruby/truffleruby/core/truffle/exception_operations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,25 @@ def self.class_name(receiver)
# MRI: name_err_mesg_to_str
def self.receiver_string(receiver)
ret = begin
if Primitive.respond_to?(receiver, :inspect, false)
case receiver
when NilClass, TrueClass, FalseClass
Truffle::Type.rb_inspect(receiver)
when Class
"class #{receiver}"
when Module
"module #{receiver}"
else
nil
klass = Primitive.metaclass(receiver)
unless klass.singleton_class?
"an instance of #{klass}"
end
# otherwise fall through to rb_any_to_s
end
rescue Exception # rubocop:disable Lint/RescueException
nil
end
ret = Primitive.rb_any_to_s(receiver) unless ret && ret.bytesize <= 65
if ret.start_with?('#')
ret
else
"#{ret}:#{class_name(receiver)}"
end
ret
end

# MRI: inspect_frozen_obj
Expand Down
2 changes: 1 addition & 1 deletion test/mri/tests/bigdecimal/test_bigdecimal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ def test_s_ver

def test_s_allocate
if RUBY_ENGINE == "truffleruby"
assert_raise_with_message(NoMethodError, /undefined.+allocate.+for BigDecimal/) { BigDecimal.allocate }
assert_raise_with_message(NoMethodError, /undefined.+allocate.+for class BigDecimal/) { BigDecimal.allocate }
else
assert_raise_with_message(TypeError, /allocator undefined for BigDecimal/) { BigDecimal.allocate }
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/inlined_method.rb:13:in `meth_with_inlined_plus': undefined method `+' for nil:NilClass (NoMethodError)
/inlined_method.rb:13:in `meth_with_inlined_plus': undefined method `+' for nil (NoMethodError)
from /inlined_method.rb:17:in `block in <main>'
from /backtraces.rb:17:in `check'
from /inlined_method.rb:16:in `<main>'
Loading