[GR-19220] Fix Module#undef_method error message and Module#{to_s,inspect}.

PullRequest: truffleruby/1163

Author: eregon

CHANGELOG.md

@@ -29,6 +29,7 @@ Bug fixes:
 * Fixed missing partial evaluation boundary in `Array#{sort,sort!}` (#1727).
 * Fixed the class of `self` and the wrapping `Module` for `Kernel#load(path, wrap=true)` (#1739).
 * Fixed missing polyglot type declaration for `RSTRING_PTR` to help with native/managed interop.
+* Fixed `Module#to_s` and `Module#inspect` to not return an extra `#<Class:` for singleton classes.
 
 Compatibility:
 

spec/ruby/core/class/to_s_spec.rb

@@ -6,6 +6,9 @@ def self.without_test_modules(modules)
 
   CONST = :plain_constant
 
+  class NamedClass
+  end
+
   module PrivConstModule
     PRIVATE_CONSTANT = 1
     private_constant :PRIVATE_CONSTANT

spec/ruby/core/module/fixtures/classes.rb

@@ -2,17 +2,44 @@
 require_relative 'fixtures/classes'
 
 describe "Module#to_s" do
+  it 'returns the name of the module if it has a name' do
+    Enumerable.to_s.should == 'Enumerable'
+    String.to_s.should == 'String'
+  end
+
   it "returns the full constant path leading to the module" do
     ModuleSpecs::LookupMod.to_s.should == "ModuleSpecs::LookupMod"
   end
 
   it "works with an anonymous module" do
     m = Module.new
-    m.to_s.should =~ /#<Module:0x[0-9a-f]+>/
+    m.to_s.should =~ /\A#<Module:0x\h+>\z/
   end
 
   it "works with an anonymous class" do
     c = Class.new
-    c.to_s.should =~ /#<Class:0x[0-9a-f]+>/
+    c.to_s.should =~ /\A#<Class:0x\h+>\z/
+  end
+
+  it 'for the singleton class of an object of an anonymous class' do
+    klass = Class.new
+    obj = klass.new
+    sclass = obj.singleton_class
+    sclass.to_s.should == "#<Class:#{obj}>"
+    sclass.to_s.should =~ /\A#<Class:#<#{klass}:0x\h+>>\z/
+    sclass.to_s.should =~ /\A#<Class:#<#<Class:0x\h+>:0x\h+>>\z/
+  end
+
+  it 'for a singleton class of a module includes the module name' do
+    ModuleSpecs.singleton_class.to_s.should == '#<Class:ModuleSpecs>'
+  end
+
+  it 'for a metaclass includes the class name' do
+    ModuleSpecs::NamedClass.singleton_class.to_s.should == '#<Class:ModuleSpecs::NamedClass>'
+  end
+
+  it 'for objects includes class name and object ID' do
+    obj = ModuleSpecs::NamedClass.new
+    obj.singleton_class.to_s.should =~ /\A#<Class:#<ModuleSpecs::NamedClass:0x\h+>>\z/
   end
 end

spec/ruby/core/module/to_s_spec.rb

@@ -56,8 +56,37 @@ def another_method_to_undef() 1 end
     @module.send(:undef_method, :method_to_undef).should equal(@module)
   end
 
-  it "raises a NameError when passed a missing name" do
-    -> { @module.send :undef_method, :not_exist }.should raise_error(NameError) { |e|
+  it "raises a NameError when passed a missing name for a module" do
+    -> { @module.send :undef_method, :not_exist }.should raise_error(NameError, /undefined method `not_exist' for module `#{@module}'/) { |e|
+      # a NameError and not a NoMethodError
+      e.class.should == NameError
+    }
+  end
+
+  it "raises a NameError when passed a missing name for a class" do
+    klass = Class.new
+    -> { klass.send :undef_method, :not_exist }.should raise_error(NameError, /undefined method `not_exist' for class `#{klass}'/) { |e|
+      # a NameError and not a NoMethodError
+      e.class.should == NameError
+    }
+  end
+
+  it "raises a NameError when passed a missing name for a singleton class" do
+    klass = Class.new
+    obj = klass.new
+    sclass = obj.singleton_class
+
+    -> { sclass.send :undef_method, :not_exist }.should raise_error(NameError, /undefined method `not_exist' for class `#{sclass}'/) { |e|
+      e.message.should include('`#<Class:#<#<Class:')
+
+      # a NameError and not a NoMethodError
+      e.class.should == NameError
+    }
+  end
+
+  it "raises a NameError when passed a missing name for a metaclass" do
+    klass = String.singleton_class
+    -> { klass.send :undef_method, :not_exist }.should raise_error(NameError, /undefined method `not_exist' for class `String'/) { |e|
       # a NameError and not a NoMethodError
       e.class.should == NameError
     }

spec/ruby/core/module/undef_method_spec.rb

@@ -646,7 +646,11 @@ public DynamicObject nameErrorInstanceVariableNotDefined(String name, Object rec
     public DynamicObject nameErrorUndefinedMethod(String name, DynamicObject module, Node currentNode) {
         assert RubyGuards.isRubyModule(module);
         return nameError(
-                StringUtils.format("undefined method `%s' for %s", name, Layouts.MODULE.getFields(module).getName()),
+                StringUtils.format(
+                        "undefined method `%s' for %s `%s'",
+                        name,
+                        Layouts.CLASS.isClass(module) ? "class" : "module",
+                        Layouts.MODULE.getFields(module).getName()),
                 module,
                 name,
                 currentNode);

spec/tags/core/module/undef_method_tags.txt

@@ -446,9 +446,16 @@ public boolean removeMethod(String methodName) {
     public void undefMethod(RubyContext context, Node currentNode, String methodName) {
         final InternalMethod method = ModuleOperations.lookupMethodUncached(rubyModuleObject, methodName, null);
         if (method == null || method.isUndefined()) {
+            final DynamicObject moduleForError;
+            if (RubyGuards.isMetaClass(rubyModuleObject)) {
+                moduleForError = Layouts.CLASS.getAttached(rubyModuleObject);
+            } else {
+                moduleForError = rubyModuleObject;
+            }
+
             throw new RaiseException(context, context.getCoreExceptions().nameErrorUndefinedMethod(
                     methodName,
-                    rubyModuleObject,
+                    moduleForError,
                     currentNode));
         } else {
             addMethod(context, currentNode, method.undefined());

src/main/java/org/truffleruby/core/exception/CoreExceptions.java

@@ -1854,19 +1854,19 @@ protected DynamicObject toS(DynamicObject module) {
             final ModuleFields fields = Layouts.MODULE.getFields(module);
             if (RubyGuards.isSingletonClass(module)) {
                 final DynamicObject attached = Layouts.CLASS.getAttached(module);
-                final String name;
-                if (Layouts.CLASS.isClass(attached) || Layouts.MODULE.isModule(attached)) {
+                final String attachedName;
+                if (Layouts.MODULE.isModule(attached)) {
+                    attachedName = Layouts.MODULE.getFields(attached).getName();
+                } else {
                     if (callRbInspect == null) {
                         CompilerDirectives.transferToInterpreterAndInvalidate();
                         callRbInspect = insert(CallDispatchHeadNode.createPrivate());
                     }
                     final Object inspectResult = callRbInspect
                             .call(coreLibrary().getTruffleTypeModule(), "rb_inspect", attached);
-                    name = StringOperations.getString((DynamicObject) inspectResult);
-                } else {
-                    name = fields.getName();
+                    attachedName = StringOperations.getString((DynamicObject) inspectResult);
                 }
-                moduleName = "#<Class:" + name + ">";
+                moduleName = "#<Class:" + attachedName + ">";
             } else if (fields.isRefinement()) {
                 final String refinedClass = Layouts.MODULE.getFields(fields.getRefinedClass()).getName();
                 final String refinementNamespace = Layouts.MODULE.getFields(fields.getRefinementNamespace()).getName();