Repository: goby-lang/goby
Pull request #836 (2020-03-14 08:50:06)

Fix method call & error message handling
The issues of error messages are
- we're calling `to_s` instead of `inspect` on the objects
- `raise ErrorClass` still displays it as `InternalError` instead of `ErrorClass`


Patch:
diff --git a/vm/array.go b/vm/array.go
index 774ecb9..b9ac081 100644
--- a/vm/array.go
+++ b/vm/array.go
@@ -1482,7 +1482,7 @@ var builtinArrayInstanceMethods = []*BuiltinMethodObject{
 				}
 
 				if len(kv.Elements) != 2 {
-					return t.vm.InitErrorObject(errors.ArgumentError, sourceLine, "Expect element #%d to have 2 elements as a key-value pair. got: %s", i, kv.ToString())
+					return t.vm.InitErrorObject(errors.ArgumentError, sourceLine, "Expect element #%d to have 2 elements as a key-value pair. got: %s", i, kv.Inspect())
 				}
 
 				k := kv.Elements[0]
diff --git a/vm/class.go b/vm/class.go
index a610fd9..f2cb629 100644
--- a/vm/class.go
+++ b/vm/class.go
@@ -1316,7 +1316,13 @@ var builtinClassCommonInstanceMethods = []*BuiltinMethodObject{
 			case 0:
 				return t.vm.InitErrorObject(errors.InternalError, sourceLine, "")
 			case 1:
-				return t.vm.InitErrorObject(errors.InternalError, sourceLine, "'%s'", args[0].ToString())
+				errorClass, ok := args[0].(*RClass)
+
+				if !ok {
+					return t.vm.InitErrorObject(errors.InternalError, sourceLine, "%s", args[0].Inspect())
+				}
+
+				return t.vm.InitErrorObject(errorClass.Name, sourceLine, "%s", args[0].Inspect())
 			case 2:
 				errorClass, ok := args[0].(*RClass)
 
@@ -1324,7 +1330,7 @@ var builtinClassCommonInstanceMethods = []*BuiltinMethodObject{
 					return t.vm.InitErrorObject(errors.ArgumentError, sourceLine, errors.WrongArgumentTypeFormatNum, 2, "a class", args[0].Class().Name)
 				}
 
-				return t.vm.InitErrorObject(errorClass.Name, sourceLine, "'%s'", args[1].ToString())
+				return t.vm.InitErrorObject(errorClass.Name, sourceLine, "%s", args[1].Inspect())
 			}
 
 			return t.vm.InitErrorObject(errors.ArgumentError, sourceLine, errors.WrongNumberOfArgumentLess, 2, aLen)
diff --git a/vm/instruction.go b/vm/instruction.go
index 7706011..c465127 100644
--- a/vm/instruction.go
+++ b/vm/instruction.go
@@ -391,7 +391,6 @@ func init() {
 
 		},
 		bytecode.Send: func(t *Thread, sourceLine int, cf *normalCallFrame, args ...interface{}) {
-			var method Object
 			var blockFlag string
 
 			methodName := args[0].(string)
@@ -419,34 +418,6 @@ func init() {
 			receiverPr := argPr - 1
 			receiver := t.Stack.data[receiverPr].Target
 
-			// Find Method
-			method = receiver.findMethod(methodName)
-
-			if method == nil {
-				mm := receiver.findMethodMissing(receiver.Class().inheritsMethodMissing)
-
-				if mm == nil {
-					t.setErrorObject(receiverPr, argPr, errors.NoMethodError, sourceLine, errors.UndefinedMethod, methodName, receiver.ToString())
-				} else {
-					// Move up args for missed method's name
-					// before: | arg 1       | arg 2 |
-					// after:  | method name | arg 1 | arg 2 |
-					// TODO: Improve this
-					t.Stack.Push(nil)
-
-					for i := argCount - 1; i >= 0; i-- {
-						position := argPr + i
-						arg := t.Stack.data[argPr+i]
-						t.Stack.Set(position+1, arg)
-					}
-
-					t.Stack.Set(argPr, &Pointer{Target: t.vm.InitStringObject(methodName)})
-					argCount++
-
-					method = mm
-				}
-			}
-
 			// Find Block
 			blockFrame := t.retrieveBlock(cf.FileName(), blockFlag, cf.SourceLine())
 
@@ -457,16 +428,7 @@ func init() {
 				t.callFrameStack.push(blockFrame)
 			}
 
-			switch m := method.(type) {
-			case *MethodObject:
-				callObj := newCallObject(receiver, m, receiverPr, argCount, argSet, blockFrame, sourceLine)
-				t.evalMethodObject(callObj)
-			case *BuiltinMethodObject:
-				t.evalBuiltinMethod(receiver, m, receiverPr, argCount, argSet, blockFrame, sourceLine, cf.fileName)
-			case *Error:
-				t.pushErrorObject(errors.InternalError, sourceLine, m.ToString())
-			}
-
+			t.findAndCallMethod(receiver, methodName, receiverPr, argSet, argCount, argPr, sourceLine, blockFrame, cf.fileName)
 		},
 		bytecode.InvokeBlock: func(t *Thread, sourceLine int, cf *normalCallFrame, args ...interface{}) {
 			argCount := args[0].(int)
diff --git a/vm/regexp.go b/vm/regexp.go
index 4ea8c3b..5925b8e 100644
--- a/vm/regexp.go
+++ b/vm/regexp.go
@@ -58,7 +58,7 @@ var builtInRegexpClassMethods = []*BuiltinMethodObject{
 
 			r := t.vm.initRegexpObject(args[0].ToString())
 			if r == nil {
-				return t.vm.InitErrorObject(errors.ArgumentError, sourceLine, "Invalid regexp: %v", args[0].ToString())
+				return t.vm.InitErrorObject(errors.ArgumentError, sourceLine, "Invalid regexp: %v", args[0].Inspect())
 			}
 			return r
 
diff --git a/vm/thread.go b/vm/thread.go
index 643fd2f..08504d3 100644
--- a/vm/thread.go
+++ b/vm/thread.go
@@ -277,9 +277,51 @@ func (t *Thread) retrieveBlock(fileName, blockFlag string, sourceLine int) (bloc
 	return
 }
 
-func (t *Thread) sendMethod(methodName string, argCount int, blockFrame *normalCallFrame, sourceLine int) {
-	var method Object
+func (t *Thread) findMethod(receiver Object, methodName string, receiverPr int, argCount int, argPr int, sourceLine int) (method Object, argC int) {
+	method = receiver.findMethod(methodName)
+
+	if method == nil {
+		mm := receiver.findMethodMissing(receiver.Class().inheritsMethodMissing)
+
+		if mm == nil {
+			t.setErrorObject(receiverPr, argPr, errors.NoMethodError, sourceLine, errors.UndefinedMethod, methodName, receiver.Inspect())
+		} else {
+			// Move up args for missed method's name
+			// before: | arg 1       | arg 2 |
+			// after:  | method name | arg 1 | arg 2 |
+			// TODO: Improve this
+			t.Stack.Push(nil)
+
+			for i := argCount - 1; i >= 0; i-- {
+				position := argPr + i
+				arg := t.Stack.data[argPr+i]
+				t.Stack.Set(position+1, arg)
+			}
+
+			t.Stack.Set(argPr, &Pointer{Target: t.vm.InitStringObject(methodName)})
+			argCount++
+
+			method = mm
+		}
+	}
+
+	return method, argCount
+}
+
+func (t *Thread) findAndCallMethod(receiver Object, methodName string, receiverPr int, argSet *bytecode.ArgSet, argCount int, argPr int, sourceLine int, blockFrame *normalCallFrame, fileName string) {
+	// argCount change if we ended up calling method_missing
+	method, argCount := t.findMethod(receiver, methodName, receiverPr, argCount, argPr, sourceLine)
 
+	switch m := method.(type) {
+	case *MethodObject:
+		callObj := newCallObject(receiver, m, receiverPr, argCount, argSet, blockFrame, sourceLine)
+		t.evalMethodObject(callObj)
+	case *BuiltinMethodObject:
+		t.evalBuiltinMethod(receiver, m, receiverPr, argCount, argSet, blockFrame, sourceLine, fileName)
+	}
+}
+
+func (t *Thread) sendMethod(methodName string, argCount int, blockFrame *normalCallFrame, sourceLine int) {
 	if arr, ok := t.Stack.top().Target.(*ArrayObject); ok && arr.splat {
 		// Pop array
 		t.Stack.Pop()
@@ -319,23 +361,9 @@ func (t *Thread) sendMethod(methodName string, argCount int, blockFrame *normalC
 
 	t.Stack.pointer--
 
-	method = receiver.findMethod(methodName)
-
-	if method == nil {
-		t.setErrorObject(receiverPr, argPr, errors.NoMethodError, sourceLine, errors.UndefinedMethod, methodName, receiver.ToString())
-	}
-
 	sendCallFrame := t.callFrameStack.top()
 
-	switch m := method.(type) {
-	case *MethodObject:
-		callObj := newCallObject(receiver, m, receiverPr, argCount, &bytecode.ArgSet{}, blockFrame, 1)
-		t.evalMethodObject(callObj)
-	case *BuiltinMethodObject:
-		t.evalBuiltinMethod(receiver, m, receiverPr, argCount, &bytecode.ArgSet{}, blockFrame, sourceLine, sendCallFrame.FileName())
-	case *Error:
-		t.pushErrorObject(errors.InternalError, sourceLine, m.ToString())
-	}
+	t.findAndCallMethod(receiver, methodName, receiverPr, &bytecode.ArgSet{}, argCount, argPr, sourceLine, blockFrame, sendCallFrame.FileName())
 }
 
 func (t *Thread) evalBuiltinMethod(receiver Object, method *BuiltinMethodObject, receiverPtr, argCount int, argSet *bytecode.ArgSet, blockFrame *normalCallFrame, sourceLine int, fileName string) {


Test patch:
diff --git a/vm/array_enumerator_test.go b/vm/array_enumerator_test.go
index d22007c..2d95857 100644
--- a/vm/array_enumerator_test.go
+++ b/vm/array_enumerator_test.go
@@ -44,7 +44,7 @@ func TestArrayEnumeratorEnumerationWithElements(t *testing.T) {
 func TestArrayEnumeratorRaiseErrorWhenNoElementsOnNext(t *testing.T) {
 	testCase := errorTestCase{`
 	ArrayEnumerator.new([]).next
-	`, "StopIteration: 'No more elements!'", 2}
+	`, "StopIteration: \"No more elements!\"", 2}
 
 	v := initTestVM()
 	evaluated := v.testEval(t, testCase.input, getFilename())
diff --git a/vm/class_test.go b/vm/class_test.go
index 87fda00..33f0d30 100644
--- a/vm/class_test.go
+++ b/vm/class_test.go
@@ -1087,10 +1087,10 @@ func TestRaiseMethod(t *testing.T) {
 		expectedSP  int
 	}{
 		{`raise`, "InternalError: ", 1, 1},
-		{`raise "Foo"`, "InternalError: 'Foo'", 1, 1},
+		{`raise "Foo"`, "InternalError: \"Foo\"", 1, 1},
 		{`
 		class BarError; end
-		raise BarError, "Foo"`, "BarError: 'Foo'", 1, 1},
+		raise BarError, "Foo"`, "BarError: \"Foo\"", 1, 1},
 		{`
 		class FooError; end
 
@@ -1103,7 +1103,7 @@ func TestRaiseMethod(t *testing.T) {
 			// Expect CFP to be 2 is because the `raise_foo`'s frame is not popped
 			// Expect SP to be 2 cause the program got stopped before it replaces receiver with the return value (error)
 			// TODO: This means we need to pop error object when implementing `rescue`
-			"FooError: 'Foo'", 2, 2},
+			"FooError: \"Foo\"", 2, 2},
 	}
 
 	for i, tt := range testsFail {
@@ -1313,6 +1313,15 @@ func TestSendMethod(t *testing.T) {
 		a = Foo.new
 		a.send(:bar, 7, 8) do |i, j| i * j; end
 		`, 56},
+		{`
+		class Foo
+		  def method_missing(name)
+		    10
+		  end
+		end
+		
+		Foo.new.send(:bar)
+		`, 10},
 	}
 
 	for i, tt := range tests {
@@ -1555,7 +1564,7 @@ func TestClassNameClassMethod(t *testing.T) {
 
 func TestClassNameClassMethodFail(t *testing.T) {
 	testsFail := []errorTestCase{
-		{`"Taipei".name`, "NoMethodError: Undefined Method 'name' for Taipei", 1},
+		{`"Taipei".name`, "NoMethodError: Undefined Method 'name' for \"Taipei\"", 1},
 		{`123.name`, "NoMethodError: Undefined Method 'name' for 123", 1},
 		{`true.name`, "NoMethodError: Undefined Method 'name' for true", 1},
 		{`Integer.name(Integer)`, "ArgumentError: Expect 0 argument(s). got: 1", 1},
@@ -1607,7 +1616,7 @@ func TestClassSuperclassClassMethod(t *testing.T) {
 
 func TestClassSuperclassClassMethodFail(t *testing.T) {
 	testsFail := []errorTestCase{
-		{`"Taipei".superclass`, "NoMethodError: Undefined Method 'superclass' for Taipei", 1},
+		{`"Taipei".superclass`, "NoMethodError: Undefined Method 'superclass' for \"Taipei\"", 1},
 		{`123.superclass`, "NoMethodError: Undefined Method 'superclass' for 123", 1},
 		{`true.superclass`, "NoMethodError: Undefined Method 'superclass' for true", 1},
 		{`Integer.superclass(Integer)`, "ArgumentError: Expect 0 argument(s). got: 1", 1},
diff --git a/vm/error_test.go b/vm/error_test.go
index 2648729..196a005 100644
--- a/vm/error_test.go
+++ b/vm/error_test.go
@@ -119,7 +119,7 @@ func TestStackTraces(t *testing.T) {
 
 		raise_foo
 		`,
-			"FooError: 'Foo'",
+			"FooError: \"Foo\"",
 			[]string{
 				fmt.Sprintf("from %s:4", getFilename()),
 				fmt.Sprintf("from %s:7", getFilename()),
diff --git a/vm/range_enumerator_test.go b/vm/range_enumerator_test.go
index d75a114..8a20490 100644
--- a/vm/range_enumerator_test.go
+++ b/vm/range_enumerator_test.go
@@ -75,7 +75,7 @@ func TestRangeEnumeratorRaiseErrorWhenNoElementsOnNext(t *testing.T) {
 			enumerator.next
 			enumerator.next
 			`,
-			"StopIteration: 'No more elements!'",
+			"StopIteration: \"No more elements!\"",
 			2,
 		},
 		{`
@@ -84,7 +84,7 @@ func TestRangeEnumeratorRaiseErrorWhenNoElementsOnNext(t *testing.T) {
 			enumerator.next
 			enumerator.next
 			`,
-			"StopIteration: 'No more elements!'",
+			"StopIteration: \"No more elements!\"",
 			2,
 		},
 	}
diff --git a/vm/vm_test.go b/vm/vm_test.go
index 5ec23f0..f36b1ee 100644
--- a/vm/vm_test.go
+++ b/vm/vm_test.go
@@ -8,7 +8,6 @@ import (
 	"github.com/goby-lang/goby/compiler/bytecode"
 	"github.com/goby-lang/goby/compiler/lexer"
 	"github.com/goby-lang/goby/compiler/parser"
-	"github.com/goby-lang/goby/vm/errors"
 )
 
 func TestVM_REPLExec(t *testing.T) {
@@ -160,7 +159,7 @@ func TestVM_REPLExecFail(t *testing.T) {
 			[]string{
 				`raise ArgumentError`,
 			},
-			fmt.Sprintf("InternalError: '%s'", errors.ArgumentError),
+			"ArgumentError: ArgumentError",
 		},
 		{
 			[]string{