[GR-59866] Update NoMethodError message to not use object#inspect (#3749

)

PullRequest: truffleruby/4429

CHANGELOG.md

@@ -50,6 +50,7 @@ Compatibility:
 * Add `rb_data_define()` to define Data (#3681, @andrykonchin).
 * Add `Refinement#target` (#3681, @andrykonchin).
 * Add `Range#overlap?` (#3681, @andrykonchin).
+* Update `NoMethodError#message` to not use `#inspect` on receiver (#3681, @rwstauner).
 
 Performance:
 

spec/ruby/core/exception/fixtures/common.rb

@@ -84,6 +84,9 @@ class NoMethodErrorD; end
 
   class InstanceException < Exception
   end
+
+  class AClass; end
+  module AModule; end
 end
 
 class NameErrorSpecs

spec/ruby/core/exception/no_method_error_spec.rb

@@ -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
@@ -76,19 +76,163 @@ 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
+
+    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
+        end
+      end
+      instance = test_class.new
+
+      begin
+        instance.bar
+      rescue NoMethodError => error
+        message = error.message
+        message.should =~ /undefined method.+\bbar\b/
+        message.should include test_class.inspect
+      end
+    end
+
+    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
+
+    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
 
   ruby_version_is "3.3" do
-    it "does not call receiver.inspect even when calling Exception#message" 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
+
+    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
+      klass = Class.new { def self.name; "MyClass"; end }
+
+      begin
+        klass.foo
+      rescue NoMethodError => error
+        error.message.should =~ /\Aundefined method [`']foo' for class MyClass\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
+      mod = Module.new { def self.name; "MyModule"; end }
+
+      begin
+        mod.foo
+      rescue NoMethodError => error
+        error.message.should =~ /\Aundefined method [`']foo' for module MyModule\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
+      klass = Class.new { def self.name; "MyClass"; end }
+      instance = klass.new
+
+      begin
+        instance.bar
+      rescue NoMethodError => error
+        error.message.should =~ /\Aundefined method [`']bar' for an instance of MyClass\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
@@ -97,47 +241,29 @@ 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 an instance of #<Class:0x\h+>\Z/
         ScratchPad.recorded.should == []
       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
-      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
-    end
-  end
+    it "does not truncate long class names" do
+      class_name = 'ExceptionSpecs::A' + 'a'*100
 
-  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 /
-    end
+      begin
+        eval <<~RUBY
+          class #{class_name}
+          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 /
+          obj = #{class_name}.new
+          obj.foo
+        RUBY
+      rescue NoMethodError => error
+        error.message.should =~ /\Aundefined method [`']foo' for an instance of #{class_name}\Z/
+      end
     end
   end
 end

src/main/ruby/truffleruby/core/truffle/exception_operations.rb

@@ -75,20 +75,31 @@ 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)
-          Truffle::Type.rb_inspect(receiver)
+        case receiver
+        when true, false
+          receiver.to_s
+        when nil
+          'nil'
+        when Class
+          name = receiver.name || receiver.to_s
+          "class #{name}"
+        when Module
+          name = receiver.name || receiver.to_s
+          "module #{name}"
         else
-          nil
+          klass = Primitive.metaclass(receiver)
+
+          unless klass.singleton_class?
+            class_name = klass.name || klass.to_s
+            "an instance of #{class_name}"
+          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 = Primitive.rb_any_to_s(receiver) unless ret
+      ret
     end
 
     # MRI: inspect_frozen_obj

test/mri/tests/bigdecimal/test_bigdecimal.rb

@@ -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

test/truffle/integration/backtraces/inlined_method.backtrace

@@ -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>'