syscall/js: improve type checks of js.Value's methods

Add more explicit checks if the given js.Value is of the correct type
instead of erroring on the JavaScript layer.

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

2 files changed, 95 insertions, 6 deletions

diff --git a/src/syscall/js/js.go b/src/syscall/js/js.go
index 0893db022d..bccf188fa5 100644
--- a/src/syscall/js/js.go
+++ b/src/syscall/js/js.go
@@ -216,6 +216,10 @@ func (t Type) String() string {
 	}
 }
 
+func (t Type) isObject() bool {
+	return t == TypeObject || t == TypeFunction
+}
+
 // Type returns the JavaScript type of the value v. It is similar to JavaScript's typeof operator,
 // except that it returns TypeNull instead of TypeObject for null.
 func (v Value) Type() Type {
@@ -244,28 +248,44 @@ func (v Value) Type() Type {
 }
 
 // Get returns the JavaScript property p of value v.
+// It panics if v is not a JavaScript object.
 func (v Value) Get(p string) Value {
+	if vType := v.Type(); !vType.isObject() {
+		panic(&ValueError{"Value.Get", vType})
+	}
 	return makeValue(valueGet(v.ref, p))
 }
 
 func valueGet(v ref, p string) ref
 
 // Set sets the JavaScript property p of value v to ValueOf(x).
+// It panics if v is not a JavaScript object.
 func (v Value) Set(p string, x interface{}) {
+	if vType := v.Type(); !vType.isObject() {
+		panic(&ValueError{"Value.Set", vType})
+	}
 	valueSet(v.ref, p, ValueOf(x).ref)
 }
 
 func valueSet(v ref, p string, x ref)
 
 // Index returns JavaScript index i of value v.
+// It panics if v is not a JavaScript object.
 func (v Value) Index(i int) Value {
+	if vType := v.Type(); !vType.isObject() {
+		panic(&ValueError{"Value.Index", vType})
+	}
 	return makeValue(valueIndex(v.ref, i))
 }
 
 func valueIndex(v ref, i int) ref
 
 // SetIndex sets the JavaScript index i of value v to ValueOf(x).
+// It panics if v is not a JavaScript object.
 func (v Value) SetIndex(i int, x interface{}) {
+	if vType := v.Type(); !vType.isObject() {
+		panic(&ValueError{"Value.SetIndex", vType})
+	}
 	valueSetIndex(v.ref, i, ValueOf(x).ref)
 }
 
@@ -280,7 +300,11 @@ func makeArgs(args []interface{}) []ref {
 }
 
 // Length returns the JavaScript property "length" of v.
+// It panics if v is not a JavaScript object.
 func (v Value) Length() int {
+	if vType := v.Type(); !vType.isObject() {
+		panic(&ValueError{"Value.SetIndex", vType})
+	}
 	return valueLength(v.ref)
 }
 
@@ -292,7 +316,7 @@ func valueLength(v ref) int
 func (v Value) Call(m string, args ...interface{}) Value {
 	res, ok := valueCall(v.ref, m, makeArgs(args))
 	if !ok {
-		if vType := v.Type(); vType != TypeObject && vType != TypeFunction { // check here to avoid overhead in success case
+		if vType := v.Type(); !vType.isObject() { // check here to avoid overhead in success case
 			panic(&ValueError{"Value.Call", vType})
 		}
 		if propType := v.Get(m).Type(); propType != TypeFunction {
@@ -306,7 +330,7 @@ func (v Value) Call(m string, args ...interface{}) Value {
 func valueCall(v ref, m string, args []ref) (ref, bool)
 
 // Invoke does a JavaScript call of the value v with the given arguments.
-// It panics if v is not a function.
+// It panics if v is not a JavaScript function.
 // The arguments get mapped to JavaScript values according to the ValueOf function.
 func (v Value) Invoke(args ...interface{}) Value {
 	res, ok := valueInvoke(v.ref, makeArgs(args))
@@ -322,11 +346,14 @@ func (v Value) Invoke(args ...interface{}) Value {
 func valueInvoke(v ref, args []ref) (ref, bool)
 
 // New uses JavaScript's "new" operator with value v as constructor and the given arguments.
-// It panics if v is not a function.
+// It panics if v is not a JavaScript function.
 // The arguments get mapped to JavaScript values according to the ValueOf function.
 func (v Value) New(args ...interface{}) Value {
 	res, ok := valueNew(v.ref, makeArgs(args))
 	if !ok {
+		if vType := v.Type(); vType != TypeFunction { // check here to avoid overhead in success case
+			panic(&ValueError{"Value.Invoke", vType})
+		}
 		panic(Error{makeValue(res)})
 	}
 	return makeValue(res)
@@ -350,17 +377,20 @@ func (v Value) float(method string) float64 {
 	return *(*float64)(unsafe.Pointer(&v.ref))
 }
 
-// Float returns the value v as a float64. It panics if v is not a JavaScript number.
+// Float returns the value v as a float64.
+// It panics if v is not a JavaScript number.
 func (v Value) Float() float64 {
 	return v.float("Value.Float")
 }
 
-// Int returns the value v truncated to an int. It panics if v is not a JavaScript number.
+// Int returns the value v truncated to an int.
+// It panics if v is not a JavaScript number.
 func (v Value) Int() int {
 	return int(v.float("Value.Int"))
 }
 
-// Bool returns the value v as a bool. It panics if v is not a JavaScript boolean.
+// Bool returns the value v as a bool.
+// It panics if v is not a JavaScript boolean.
 func (v Value) Bool() bool {
 	switch v.ref {
 	case valueTrue.ref:
diff --git a/src/syscall/js/js_test.go b/src/syscall/js/js_test.go
index c14d2cc24c..594284faf9 100644
--- a/src/syscall/js/js_test.go
+++ b/src/syscall/js/js_test.go
@@ -202,10 +202,30 @@ func TestLength(t *testing.T) {
 	}
 }
 
+func TestGet(t *testing.T) {
+	// positive cases get tested per type
+
+	expectValueError(t, func() {
+		dummys.Get("zero").Get("badField")
+	})
+}
+
+func TestSet(t *testing.T) {
+	// positive cases get tested per type
+
+	expectValueError(t, func() {
+		dummys.Get("zero").Set("badField", 42)
+	})
+}
+
 func TestIndex(t *testing.T) {
 	if got := dummys.Get("someArray").Index(1).Int(); got != 42 {
 		t.Errorf("got %#v, want %#v", got, 42)
 	}
+
+	expectValueError(t, func() {
+		dummys.Get("zero").Index(1)
+	})
 }
 
 func TestSetIndex(t *testing.T) {
@@ -213,6 +233,10 @@ func TestSetIndex(t *testing.T) {
 	if got := dummys.Get("someArray").Index(2).Int(); got != 99 {
 		t.Errorf("got %#v, want %#v", got, 99)
 	}
+
+	expectValueError(t, func() {
+		dummys.Get("zero").SetIndex(2, 99)
+	})
 }
 
 func TestCall(t *testing.T) {
@@ -223,6 +247,13 @@ func TestCall(t *testing.T) {
 	if got := dummys.Call("add", js.Global().Call("eval", "40"), 2).Int(); got != 42 {
 		t.Errorf("got %#v, want %#v", got, 42)
 	}
+
+	expectPanic(t, func() {
+		dummys.Call("zero")
+	})
+	expectValueError(t, func() {
+		dummys.Get("zero").Call("badMethod")
+	})
 }
 
 func TestInvoke(t *testing.T) {
@@ -230,12 +261,20 @@ func TestInvoke(t *testing.T) {
 	if got := dummys.Get("add").Invoke(i, 2).Int(); got != 42 {
 		t.Errorf("got %#v, want %#v", got, 42)
 	}
+
+	expectValueError(t, func() {
+		dummys.Get("zero").Invoke()
+	})
 }
 
 func TestNew(t *testing.T) {
 	if got := js.Global().Get("Array").New(42).Length(); got != 42 {
 		t.Errorf("got %#v, want %#v", got, 42)
 	}
+
+	expectValueError(t, func() {
+		dummys.Get("zero").New()
+	})
 }
 
 func TestInstanceOf(t *testing.T) {
@@ -379,3 +418,23 @@ func TestTruthy(t *testing.T) {
 		t.Errorf("got %#v, want %#v", got, want)
 	}
 }
+
+func expectValueError(t *testing.T, fn func()) {
+	defer func() {
+		err := recover()
+		if _, ok := err.(*js.ValueError); !ok {
+			t.Errorf("expected *js.ValueError, got %T", err)
+		}
+	}()
+	fn()
+}
+
+func expectPanic(t *testing.T, fn func()) {
+	defer func() {
+		err := recover()
+		if err == nil {
+			t.Errorf("expected panic")
+		}
+	}()
+	fn()
+}