From 5fc26c036a81bd4a44b42b68fd6ed27a0dadc71c Mon Sep 17 00:00:00 2001 From: Paul Jolly Date: Tue, 22 Aug 2017 19:18:28 +0100 Subject: [PATCH 1/3] compiler: fix incorrect emitted Javascript for type switch This brings the code emitted for a type switch with a *js.Object case in line with the runtime $assertType function. In the case of a *js.Object value, we have to dereference via .$val.object. Fixes #682 --- compiler/statements.go | 4 +++- js/js_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/compiler/statements.go b/compiler/statements.go index 529f40a39..43a526d5d 100644 --- a/compiler/statements.go +++ b/compiler/statements.go @@ -123,7 +123,9 @@ func (c *funcContext) translateStmt(stmt ast.Stmt, label *types.Label) { var bodyPrefix []ast.Stmt if implicit := c.p.Implicits[clause]; implicit != nil { value := refVar - if _, isInterface := implicit.Type().Underlying().(*types.Interface); !isInterface { + if typesutil.IsJsObject(implicit.Type().Underlying()) { + value += ".$val.object" + } else if _, ok := implicit.Type().Underlying().(*types.Interface); !ok { value += ".$val" } bodyPrefix = []ast.Stmt{&ast.AssignStmt{ diff --git a/js/js_test.go b/js/js_test.go index f96cac143..55d5182de 100644 --- a/js/js_test.go +++ b/js/js_test.go @@ -571,3 +571,28 @@ func TestUint8Array(t *testing.T) { t.Errorf("Non-empty byte array is not externalized as a Uint8Array") } } + +func TestTypeSwitchJsObject(t *testing.T) { + obj := js.Global.Get("Object").New() + obj.Set("foo", "bar") + + exp := "bar" + + if act := obj.Get("foo").String(); act != exp { + t.Fatalf("Direct access to *js.Object field gave %q; expected %q", act, exp) + } + + var x interface{} = obj + switch x := x.(type) { + case *js.Object: + if act := x.Get("foo").String(); act != exp { + t.Fatalf("Value passed through interface and type switch gave %q; expected %q", act, exp) + } + } + + if y, ok := x.(*js.Object); ok { + if act := y.Get("foo").String(); act != exp { + t.Fatalf("Value passed through interface and type assert gave %q; expected %q", act, exp) + } + } +} From a14dd1a51027ce494f1587a85903600fef8a5c26 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Wed, 6 Sep 2017 14:03:28 -0400 Subject: [PATCH 2/3] js: Use more idiomatic style in test. Use the more common got, want naming scheme. Use "JS" rather than "Js" in test name. Reference: https://golang.org/s/style#initialisms. --- js/js_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/js/js_test.go b/js/js_test.go index 55d5182de..764537346 100644 --- a/js/js_test.go +++ b/js/js_test.go @@ -572,27 +572,27 @@ func TestUint8Array(t *testing.T) { } } -func TestTypeSwitchJsObject(t *testing.T) { +func TestTypeSwitchJSObject(t *testing.T) { obj := js.Global.Get("Object").New() obj.Set("foo", "bar") - exp := "bar" + want := "bar" - if act := obj.Get("foo").String(); act != exp { - t.Fatalf("Direct access to *js.Object field gave %q; expected %q", act, exp) + if got := obj.Get("foo").String(); got != want { + t.Fatalf("Direct access to *js.Object field gave %q, want %q", got, want) } var x interface{} = obj switch x := x.(type) { case *js.Object: - if act := x.Get("foo").String(); act != exp { - t.Fatalf("Value passed through interface and type switch gave %q; expected %q", act, exp) + if got := x.Get("foo").String(); got != want { + t.Fatalf("Value passed through interface and type switch gave %q, want %q", got, want) } } if y, ok := x.(*js.Object); ok { - if act := y.Get("foo").String(); act != exp { - t.Fatalf("Value passed through interface and type assert gave %q; expected %q", act, exp) + if got := y.Get("foo").String(); got != want { + t.Fatalf("Value passed through interface and type assert gave %q, want %q", got, want) } } } From d58cd815e4ca775cf48b206c2e7ecdee3cd1b6d5 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Wed, 6 Sep 2017 14:10:19 -0400 Subject: [PATCH 3/3] js: Make test cases non-fatal. There's no reason for these to be fatal. We can continue running the test further if any one of them fails. That way, if there are multiple failures, it's easier to see them all at once. Separate var x from the switch by a blank line, since it's used by both switch and the if statement below. Having it without a blank line makes it seem that it's only needed for the switch. --- js/js_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/js/js_test.go b/js/js_test.go index 764537346..cbac7c7db 100644 --- a/js/js_test.go +++ b/js/js_test.go @@ -579,20 +579,21 @@ func TestTypeSwitchJSObject(t *testing.T) { want := "bar" if got := obj.Get("foo").String(); got != want { - t.Fatalf("Direct access to *js.Object field gave %q, want %q", got, want) + t.Errorf("Direct access to *js.Object field gave %q, want %q", got, want) } var x interface{} = obj + switch x := x.(type) { case *js.Object: if got := x.Get("foo").String(); got != want { - t.Fatalf("Value passed through interface and type switch gave %q, want %q", got, want) + t.Errorf("Value passed through interface and type switch gave %q, want %q", got, want) } } if y, ok := x.(*js.Object); ok { if got := y.Get("foo").String(); got != want { - t.Fatalf("Value passed through interface and type assert gave %q, want %q", got, want) + t.Errorf("Value passed through interface and type assert gave %q, want %q", got, want) } } }