syscall/js: improve Value.String() for non-string values

This change modifies Value.String() to use the following
representations for non-string values:
  <undefined>
  <null>
  <boolean: true>
  <number: 42>
  <symbol>
  <object>
  <function>

It avoids JavaScript conversion semantics in the Go API and lowers the
risk of hidden bugs by unexpected conversions, e.g. the conversion
of the number 42 to the string "42". See discussion in #29642.

This is a breaking change, which are still allowed for syscall/js.
The impact should be small since it only affects uses of
Value.String() with non-string values, which should be uncommon.

Updates #29642.

Change-Id: I2c27be6e24befe8cb713031fbf66f7b6041e7148
Reviewed-on: https://go-review.googlesource.com/c/go/+/169757
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/syscall/js/js.go b/src/syscall/js/js.go
index bccf188fa51d6daa7f387302194ec64d681380d6..0acc7da9bfdea8045e1c90067c8cae64cd1d7965 100644 (file)
--- a/src/syscall/js/js.go
+++ b/src/syscall/js/js.go
@@ -422,9 +422,35 @@ func (v Value) Truthy() bool {
        }
 }
 
-// String returns the value v converted to string according to JavaScript type conversions.
+// String returns the value v as a string.
+// String is a special case because of Go's String method convention. Unlike the other getters,
+// it does not panic if v's Type is not TypeString. Instead, it returns a string of the form "<T>"
+// or "<T: V>" where T is v's type and V is a string representation of v's value.
 func (v Value) String() string {
-       str, length := valuePrepareString(v.ref)
+       switch v.Type() {
+       case TypeString:
+               return jsString(v.ref)
+       case TypeUndefined:
+               return "<undefined>"
+       case TypeNull:
+               return "<null>"
+       case TypeBoolean:
+               return "<boolean: " + jsString(v.ref) + ">"
+       case TypeNumber:
+               return "<number: " + jsString(v.ref) + ">"
+       case TypeSymbol:
+               return "<symbol>"
+       case TypeObject:
+               return "<object>"
+       case TypeFunction:
+               return "<function>"
+       default:
+               panic("bad type")
+       }
+}
+
+func jsString(v ref) string {
+       str, length := valuePrepareString(v)
        b := make([]byte, length)
        valueLoadString(str, b)
        return string(b)

diff --git a/src/syscall/js/js_test.go b/src/syscall/js/js_test.go
index 594284faf972bc52d838ec8be563e3413ff0d9a8..20ccac77790f082d20822aa5395c02b332aaa313 100644 (file)
--- a/src/syscall/js/js_test.go
+++ b/src/syscall/js/js_test.go
@@ -72,10 +72,26 @@ func TestString(t *testing.T) {
                t.Errorf("same value not equal")
        }
 
-       wantInt := "42"
-       o = dummys.Get("someInt")
-       if got := o.String(); got != wantInt {
-               t.Errorf("got %#v, want %#v", got, wantInt)
+       if got, want := js.Undefined().String(), "<undefined>"; got != want {
+               t.Errorf("got %#v, want %#v", got, want)
+       }
+       if got, want := js.Null().String(), "<null>"; got != want {
+               t.Errorf("got %#v, want %#v", got, want)
+       }
+       if got, want := js.ValueOf(true).String(), "<boolean: true>"; got != want {
+               t.Errorf("got %#v, want %#v", got, want)
+       }
+       if got, want := js.ValueOf(42.5).String(), "<number: 42.5>"; got != want {
+               t.Errorf("got %#v, want %#v", got, want)
+       }
+       if got, want := js.Global().Call("Symbol").String(), "<symbol>"; got != want {
+               t.Errorf("got %#v, want %#v", got, want)
+       }
+       if got, want := js.Global().String(), "<object>"; got != want {
+               t.Errorf("got %#v, want %#v", got, want)
+       }
+       if got, want := js.Global().Get("setTimeout").String(), "<function>"; got != want {
+               t.Errorf("got %#v, want %#v", got, want)
        }
 }