Repository: goby-lang/goby
Pull request #782 (2019-09-25 01:21:11)

Improve the output of `Object#inspect`
I revised the output of `Object#inspect` method from `<Instance of: Class>` to `#<Foo:824635618784 >` or `#<Foo:824635618784 @bar={ k: [4, 5, "str"] } @foo=[1, 2, 3] >` (in the case of instances with instance variables).

```ruby
# Goby's new `#inspect` and `#to_s`
» class Foo; attr_accessor :foo, :bar;end
#» Foo
» a = Foo.new; a.foo = [1, 2, 3]; a.bar = {k: [4, 5, "str"]}
#» { k: [4, 5, "str"] }
» a.inspect
#» #<Foo:824635618784 @bar={ k: [4, 5, "str"] } @foo=[1, 2, 3] >
» a.to_s
#» #<Foo:824635618784 >
```

Note that the outputs of `Object#inspect` and `to_s` in Ruby are different as follows:

```ruby
# Ruby's #inspect and #to_s
» class Foo
»   attr_accessor :foo, :bar
» end
» a = Foo.new; a.foo = [1, 2, 3]; a.bar = {k: [4, 5, "str"]}
#» {:k=>[4, 5, "str"]}
» a.inspect
#» "#<Foo:0x00007fe103935bc0 @foo=[1, 2, 3], @bar={:k=>[4, 5, \"str\"]}>"
» a.to_s
#» "#<Foo:0x00007fe103935bc0>"
```

codecov[bot]: # [Codecov](https://codecov.io/gh/goby-lang/goby/pull/782?src=pr&el=h1) Report
> Merging [#782](https://codecov.io/gh/goby-lang/goby/pull/782?src=pr&el=desc) into [master](https://codecov.io/gh/goby-lang/goby/commit/000d27b86aa8b47767ca0603844491864f89b955?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `54.54%`.

[![Impacted file tree graph](https://codecov.io/gh/goby-lang/goby/pull/782/graphs/tree.svg?width=650&token=giIKD13jMt&height=150&src=pr)](https://codecov.io/gh/goby-lang/goby/pull/782?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #782      +/-   ##
==========================================
- Coverage   80.03%   80.01%   -0.03%     
==========================================
  Files          54       54              
  Lines        7382     7389       +7     
==========================================
+ Hits         5908     5912       +4     
- Misses       1246     1248       +2     
- Partials      228      229       +1
```


| [Impacted Files](https://codecov.io/gh/goby-lang/goby/pull/782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [vm/concurrent\_rw\_lock.go](https://codecov.io/gh/goby-lang/goby/pull/782/diff?src=pr&el=tree#diff-dm0vY29uY3VycmVudF9yd19sb2NrLmdv) | `88.23% <0%> (ø)` | :arrow_up: |
| [vm/object.go](https://codecov.io/gh/goby-lang/goby/pull/782/diff?src=pr&el=tree#diff-dm0vb2JqZWN0Lmdv) | `74.6% <50%> (-3.37%)` | :arrow_down: |
| [vm/validate.go](https://codecov.io/gh/goby-lang/goby/pull/782/diff?src=pr&el=tree#diff-dm0vdmFsaWRhdGUuZ28=) | `54.48% <75%> (+0.89%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/goby-lang/goby/pull/782?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/goby-lang/goby/pull/782?src=pr&el=footer). Last update [000d27b...ab9a1e5](https://codecov.io/gh/goby-lang/goby/pull/782?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/vm/concurrent_rw_lock.go b/vm/concurrent_rw_lock.go
index 2a9d7ed..ccc9db9 100644
--- a/vm/concurrent_rw_lock.go
+++ b/vm/concurrent_rw_lock.go
@@ -247,7 +247,7 @@ func (lock *ConcurrentRWLockObject) Value() interface{} {
 
 // ToString returns the object's name as the string format
 func (lock *ConcurrentRWLockObject) ToString() string {
-	return "<Instance of: " + lock.class.Name + ">"
+	return "#<" + lock.class.Name + " >"
 }
 
 // Inspect delegates to ToString
diff --git a/vm/object.go b/vm/object.go
index 337545a..0c8cfa9 100644
--- a/vm/object.go
+++ b/vm/object.go
@@ -141,12 +141,17 @@ type RObject struct {
 
 // ToString returns the object's name as the string format
 func (ro *RObject) ToString() string {
-	return "<Instance of: " + ro.class.Name + ">"
+	return "#<" + ro.class.Name + ":" + fmt.Sprint(ro.id()) + " >"
 }
 
 // Inspect delegates to ToString
 func (ro *RObject) Inspect() string {
-	return ro.ToString()
+	var iv string
+	for _, n := range ro.InstanceVariables.names() {
+		v, _ := ro.InstanceVariableGet(n)
+		iv = iv + n + "=" + v.ToString() + " "
+	}
+	return "#<" + ro.class.Name + ":" + fmt.Sprint(ro.id()) + " " + iv + ">"
 }
 
 // ToJSON just delegates to ToString
diff --git a/vm/validate.go b/vm/validate.go
index 66a88e0..dab8783 100644
--- a/vm/validate.go
+++ b/vm/validate.go
@@ -1,6 +1,10 @@
 package vm
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/dlclark/regexp2"
+)
 
 // Verification helpers
 
@@ -83,13 +87,15 @@ func verifyNullObject(t *testing.T, i int, obj Object) bool {
 
 func verifyStringObject(t *testing.T, i int, obj Object, expected string) bool {
 	t.Helper()
+	var fuzStr string
 	switch result := obj.(type) {
 	case *StringObject:
-		if result.value != expected {
+		re, _ := regexp2.Compile("(?<=#<[a-zA-Z0-9_]+:)[0-9]{12}(?=[ ]>?)", 0)
+		fuzStr, _ = re.Replace(result.value, "##OBJECTID##", 0, -1)
+		if fuzStr != expected {
 			t.Errorf("At test case %d: object has wrong value. expect=%q, got=%q", i, expected, result.value)
 			return false
 		}
-
 		return true
 	case *Error:
 		t.Errorf(result.Message())


Test patch:
diff --git a/vm/class_test.go b/vm/class_test.go
index 5e32e56..87fda00 100644
--- a/vm/class_test.go
+++ b/vm/class_test.go
@@ -1013,6 +1013,72 @@ func TestInheritsMethodMissingMethod(t *testing.T) {
 	}
 }
 
+func TestInspectMethod(t *testing.T) {
+	tests := []errorTestCase{
+		{`inspect`, "#<Object:##OBJECTID## >", 1},
+		{`Object.inspect`, "Object", 1},
+		{`Class.inspect`, "Class", 1},
+		{`String.inspect`, "String", 1},
+		{`Integer.inspect`, "Integer", 1},
+		{`Decimal.inspect`, "Decimal", 1},
+		{`Float.inspect`, "Float", 1},
+		{`Array.inspect`, "Array", 1},
+		{`Hash.inspect`, "Hash", 1},
+		{`Null.inspect`, "Null", 1},
+		{`MatchData.inspect`, "MatchData", 1},
+		{`Regexp.inspect`, "Regexp", 1},
+		{`RangeEnumerator.inspect`, "RangeEnumerator", 1},
+		{`Range.inspect`, "Range", 1},
+		{`File.inspect`, "File", 1},
+		{`GoMap.inspect`, "GoMap", 1},
+		{`Block.inspect`, "Block", 1},
+		{`Channel.inspect`, "Channel", 1},
+		{`require 'json';JSON.inspect`, "JSON", 1},
+		{`require 'net/http';Net.inspect`, "Net", 1},
+		{`require 'net/http';Net::HTTP.inspect`, "HTTP", 1},
+		{`require 'net/http';Net::HTTP::Request.inspect`, "Request", 1},
+		{`require 'concurrent/array';Concurrent::Array.inspect`, "Array", 1},
+		{`require 'concurrent/hash';Concurrent::Hash.inspect`, "Hash", 1},
+		{`require 'concurrent/rw_lock';Concurrent::RWLock.inspect`, "RWLock", 1},
+		{`require 'net/simple_server';Net::SimpleServer.inspect`, "SimpleServer", 1},
+		{`require 'spec';Spec.inspect`, "Spec", 1},
+		{`require 'uri';URI.inspect`, "URI", 1},
+		{`
+    class Foo
+    end
+    Foo.new.inspect`, "#<Foo:##OBJECTID## >", 1},
+		{`
+		class Foo
+		 attr_accessor :foo, :bar
+		end
+		Foo.new.inspect`, "#<Foo:##OBJECTID## >", 1},
+		{`
+		class Foo
+		 def self.bar
+       { k: :value }
+     end
+		end
+		Foo.bar.inspect`, `{ k: "value" }`, 1},
+		{`
+		class Foo
+		 attr_accessor :foo, :bar
+		 def initialize
+		   @foo = [42, "string", { key: :value }]
+		   @bar = { float: 2.71, decimal: 3.14.to_d }
+		 end
+		end
+		Foo.new.inspect`, `#<Foo:##OBJECTID## @bar={ decimal: 3.14, float: 2.71 } @foo=[42, "string", { key: "value" }] >`, 1},
+	}
+
+	for i, tt := range tests {
+		v := initTestVM()
+		evaluated := v.testEval(t, tt.input, getFilename())
+		VerifyExpected(t, i, evaluated, tt.expected)
+		v.checkCFP(t, i, 0)
+		v.checkSP(t, i, 1)
+	}
+}
+
 func TestRaiseMethod(t *testing.T) {
 	testsFail := []struct {
 		input       string
@@ -1421,7 +1487,6 @@ func TestGeneralKindOfMethodFail(t *testing.T) {
 	}
 }
 
-
 func TestGeneralIsNilMethod(t *testing.T) {
 	tests := []struct {
 		input    string
@@ -1717,6 +1782,72 @@ func TestObjectIdMethod(t *testing.T) {
 	}
 }
 
+func TestToSMethod(t *testing.T) {
+	tests := []errorTestCase{
+		{`to_s`, "main", 1},
+		{`Object.to_s`, "Object", 1},
+		{`Class.to_s`, "Class", 1},
+		{`String.to_s`, "String", 1},
+		{`Integer.to_s`, "Integer", 1},
+		{`Decimal.to_s`, "Decimal", 1},
+		{`Float.to_s`, "Float", 1},
+		{`Array.to_s`, "Array", 1},
+		{`Hash.to_s`, "Hash", 1},
+		{`Null.to_s`, "Null", 1},
+		{`MatchData.to_s`, "MatchData", 1},
+		{`Regexp.to_s`, "Regexp", 1},
+		{`RangeEnumerator.to_s`, "RangeEnumerator", 1},
+		{`Range.to_s`, "Range", 1},
+		{`File.to_s`, "File", 1},
+		{`GoMap.to_s`, "GoMap", 1},
+		{`Block.to_s`, "Block", 1},
+		{`Channel.to_s`, "Channel", 1},
+		{`require 'json';JSON.to_s`, "JSON", 1},
+		{`require 'net/http';Net.to_s`, "Net", 1},
+		{`require 'net/http';Net::HTTP.to_s`, "HTTP", 1},
+		{`require 'net/http';Net::HTTP::Request.to_s`, "Request", 1},
+		{`require 'concurrent/array';Concurrent::Array.to_s`, "Array", 1},
+		{`require 'concurrent/hash';Concurrent::Hash.to_s`, "Hash", 1},
+		{`require 'concurrent/rw_lock';Concurrent::RWLock.to_s`, "RWLock", 1},
+		{`require 'net/simple_server';Net::SimpleServer.to_s`, "SimpleServer", 1},
+		{`require 'spec';Spec.to_s`, "Spec", 1},
+		{`require 'uri';URI.to_s`, "URI", 1},
+		{`
+    class Foo
+    end
+    Foo.new.to_s`, "#<Foo:##OBJECTID## >", 1},
+		{`
+		class Foo
+		 attr_accessor :foo, :bar
+		end
+		Foo.new.to_s`, "#<Foo:##OBJECTID## >", 1},
+		{`
+		class Foo
+		 def self.bar
+       { k: :value }
+     end
+		end
+		Foo.bar.to_s`, `{ k: "value" }`, 1},
+		{`
+		class Foo
+		 attr_accessor :foo, :bar
+		 def initialize
+		   @foo = [42, "string", { key: :value }]
+		   @bar = { float: 2.71, decimal: 3.14.to_d }
+		 end
+		end
+		Foo.new.to_s`, `#<Foo:##OBJECTID## >`, 1},
+	}
+
+	for i, tt := range tests {
+		v := initTestVM()
+		evaluated := v.testEval(t, tt.input, getFilename())
+		VerifyExpected(t, i, evaluated, tt.expected)
+		v.checkCFP(t, i, 0)
+		v.checkSP(t, i, 1)
+	}
+}
+
 func TestClassInspectCallsToString(t *testing.T) {
 	input := `c = Class.new; c.to_s == c.inspect`
 	expected := true
diff --git a/vm/error_test.go b/vm/error_test.go
index 4c49537..302f617 100644
--- a/vm/error_test.go
+++ b/vm/error_test.go
@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+
+	"github.com/dlclark/regexp2"
 )
 
 type errorTestCase struct {
@@ -159,33 +161,33 @@ func TestStackTraces(t *testing.T) {
 
 func TestNoMethodError(t *testing.T) {
 	tests := []errorTestCase{
-		{`a`, "NoMethodError: Undefined Method 'a' for <Instance of: Object>", 1},
+		{`a`, "NoMethodError: Undefined Method 'a' for #<Object:##OBJECTID## >", 1},
 		{`class Foo
 		 end
 
 		 a = Foo.new
 		 a.bar = "fuz"
-		`, "NoMethodError: Undefined Method 'bar=' for <Instance of: Foo>", 1},
+		`, "NoMethodError: Undefined Method 'bar=' for #<Foo:##OBJECTID## >", 1},
 		{`class Foo
 		   attr_reader("foo")
 		 end
 
 		 a = Foo.new
 		 a.bar = "fuz"
-		`, "NoMethodError: Undefined Method 'bar=' for <Instance of: Foo>", 1},
+		`, "NoMethodError: Undefined Method 'bar=' for #<Foo:##OBJECTID## >", 1},
 		{`class Foo
 		  attr_reader("bar")
 		end
 
 		a = Foo.new
 		a.bar = "fuz"
-		`, "NoMethodError: Undefined Method 'bar=' for <Instance of: Foo>", 1},
+		`, "NoMethodError: Undefined Method 'bar=' for #<Foo:##OBJECTID## >", 1},
 	}
 
 	for i, tt := range tests {
 		v := initTestVM()
 		evaluated := v.testEval(t, tt.input, getFilename())
-		checkErrorMsg(t, i, evaluated, tt.expected)
+		checkFuzzifiedErrorMsg(t, i, evaluated, tt.expected)
 		v.checkCFP(t, i, tt.expectedCFP)
 		v.checkSP(t, i, 1)
 	}
@@ -448,3 +450,21 @@ func checkErrorTraces(t *testing.T, index int, evaluated Object, expectedTraces
 		t.Fatalf("At test case %d: Expect traces to be:\n%s \n got: \n%s", index, joinedExpectedTraces, joinedTraces)
 	}
 }
+
+func checkFuzzifiedErrorMsg(t *testing.T, index int, evaluated Object, expectedErrMsg string) {
+	t.Helper()
+	err, ok := evaluated.(*Error)
+	if !ok {
+		t.Fatalf("At test case %d: Expect Error. got=%T (%+v)", index, evaluated, evaluated)
+	}
+
+	if fuzzifyMessage(err.message) != expectedErrMsg {
+		t.Fatalf("At test case %d: Expect error message to be:\n  %s. got: \n%s", index, expectedErrMsg, err.message)
+	}
+}
+
+func fuzzifyMessage(message string) string {
+	re, _ := regexp2.Compile("(?<=#<[a-zA-Z0-9_]+:)[0-9]{12}(?=[ ]>?)", 0)
+	fuzMsg, _ := re.Replace(message, "##OBJECTID##", 0, -1)
+	return fuzMsg
+}
diff --git a/vm/evaluation_test.go b/vm/evaluation_test.go
index 328f922..e6c6dd9 100644
--- a/vm/evaluation_test.go
+++ b/vm/evaluation_test.go
@@ -944,13 +944,13 @@ func TestUnusedKeywordFail(t *testing.T) {
 	testsFail := []errorTestCase{
 		{`
 		if true then puts 1 end
-		`, "NoMethodError: Undefined Method 'then' for <Instance of: Object>", 1},
+		`, "NoMethodError: Undefined Method 'then' for #<Object:##OBJECTID## >", 1},
 	}
 
 	for i, tt := range testsFail {
 		v := initTestVM()
 		evaluated := v.testEval(t, tt.input, getFilename())
-		checkErrorMsg(t, i, evaluated, tt.expected)
+		checkFuzzifiedErrorMsg(t, i, evaluated, tt.expected)
 		v.checkCFP(t, i, 1)
 		v.checkSP(t, i, 1)
 	}
@@ -2196,7 +2196,7 @@ func TestMethodMissingFail(t *testing.T) {
 		end
 		
 		Foo.new.bar
-`, "NoMethodError: Undefined Method 'bar' for <Instance of: Foo>"},
+`, "NoMethodError: Undefined Method 'bar' for #<Foo:##OBJECTID## >"},
 		{`
 		module Bar
 		  def method_missing
@@ -2209,14 +2209,14 @@ func TestMethodMissingFail(t *testing.T) {
 		end
 		
 		Foo.new.bar
-`, "NoMethodError: Undefined Method 'bar' for <Instance of: Foo>"},
+`, "NoMethodError: Undefined Method 'bar' for #<Foo:##OBJECTID## >"},
 	}
 
 	for i, tt := range tests {
 		v := initTestVM()
 		evaluated := v.testEval(t, tt.input, getFilename())
 
-		checkErrorMsg(t, i, evaluated, tt.expected)
+		checkFuzzifiedErrorMsg(t, i, evaluated, tt.expected)
 		v.checkCFP(t, i, 1)
 		v.checkSP(t, i, 1)
 	}
@@ -2524,17 +2524,17 @@ func TestUnusedVariableFail(t *testing.T) {
 		{`
 		_ = 1
 		_
-		`, "NoMethodError: Undefined Method '_' for <Instance of: Object>", 1},
+		`, "NoMethodError: Undefined Method '_' for #<Object:##OBJECTID## >", 1},
 		{`
 		_, b = [1, 2]
 		_
-		`, "NoMethodError: Undefined Method '_' for <Instance of: Object>", 1},
+		`, "NoMethodError: Undefined Method '_' for #<Object:##OBJECTID## >", 1},
 	}
 
 	for i, tt := range testsFail {
 		v := initTestVM()
 		evaluated := v.testEval(t, tt.input, getFilename())
-		checkErrorMsg(t, i, evaluated, tt.expected)
+		checkFuzzifiedErrorMsg(t, i, evaluated, tt.expected)
 		v.checkCFP(t, i, 1)
 		v.checkSP(t, i, 1)
 	}
diff --git a/vm/hash_test.go b/vm/hash_test.go
index c7d8dd9..9d3a5fa 100644
--- a/vm/hash_test.go
+++ b/vm/hash_test.go
@@ -1922,7 +1922,7 @@ func compareJSONResult(t *testing.T, evaluated Object, exp interface{}) {
 }
 
 func TestHashInspectCallsToString(t *testing.T) {
-	input := `{ a: 1, b: "234", c: true, d: nil, e: Class.new}.to_s == { a: 1, b: "234", c: true, d: nil, e: Class.new}.inspect`
+	input := `h = { a: 1, b: "234", c: true, d: nil, e: Class.new}; h.to_s == h.inspect`
 	expected := true
 
 	vm := initTestVM()
@@ -1941,7 +1941,7 @@ func TestHashInspectCallsChildElementToString(t *testing.T) {
 		e = [1,2,3]
 		{ a: a, b: b, c: c, d: d, e: e }.inspect
 	`
-	expected := `{ a: nil, b: "234", c: true, d: <Instance of: Class>, e: [1, 2, 3] }`
+	expected := `{ a: nil, b: "234", c: true, d: #<Class:##OBJECTID## >, e: [1, 2, 3] }`
 
 	vm := initTestVM()
 	evaluated := vm.testEval(t, input, getFilename())