From e78570d5646e5ac11b1cca5c2934e4fa6eb3c0e5 Mon Sep 17 00:00:00 2001 From: Paul Jolly Date: Sat, 24 Mar 2018 19:07:08 +0000 Subject: [PATCH 1/3] compiler: support arbitrary value js struct tag values Javascript allows for arbitrary strings to be used as index values on an Object. A good example of where this comes up is React where `"data-*"` and `"aria-*"` indexes are used on objects (https://reactjs.org/docs/dom-elements.html). By switching from property selectors to the index operator (where required) we can support arbitrary string values in js-tagged fields for `*js.Object` special structs. Fixes #778 --- compiler/expressions.go | 6 +++--- compiler/statements.go | 2 +- compiler/utils.go | 29 +++++++++++++++++++++++++++++ js/js_test.go | 31 +++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 4 deletions(-) diff --git a/compiler/expressions.go b/compiler/expressions.go index d87b8c0d0..42fe624b6 100644 --- a/compiler/expressions.go +++ b/compiler/expressions.go @@ -516,9 +516,9 @@ func (c *funcContext) translateExpr(expr ast.Expr) *expression { fields, jsTag := c.translateSelection(sel, e.Pos()) if jsTag != "" { if _, ok := sel.Type().(*types.Signature); ok { - return c.formatExpr("$internalize(%1e.%2s.%3s, %4s, %1e.%2s)", e.X, strings.Join(fields, "."), jsTag, c.typeName(sel.Type())) + return c.formatExpr("$internalize(%1e.%2s%3s, %4s, %1e.%2s)", e.X, strings.Join(fields, "."), formatJSStructTagVal(jsTag), c.typeName(sel.Type())) } - return c.internalize(c.formatExpr("%e.%s.%s", e.X, strings.Join(fields, "."), jsTag), sel.Type()) + return c.internalize(c.formatExpr("%e.%s%s", e.X, strings.Join(fields, "."), formatJSStructTagVal(jsTag)), sel.Type()) } return c.formatExpr("%e.%s", e.X, strings.Join(fields, ".")) case types.MethodVal: @@ -669,7 +669,7 @@ func (c *funcContext) translateExpr(expr ast.Expr) *expression { case types.FieldVal: fields, jsTag := c.translateSelection(sel, f.Pos()) if jsTag != "" { - call := c.formatExpr("%e.%s.%s(%s)", f.X, strings.Join(fields, "."), jsTag, externalizeArgs(e.Args)) + call := c.formatExpr("%e.%s%s(%s)", f.X, strings.Join(fields, "."), formatJSStructTagVal(jsTag), externalizeArgs(e.Args)) switch sig.Results().Len() { case 0: return call diff --git a/compiler/statements.go b/compiler/statements.go index 43a526d5d..b83396235 100644 --- a/compiler/statements.go +++ b/compiler/statements.go @@ -703,7 +703,7 @@ func (c *funcContext) translateAssign(lhs, rhs ast.Expr, define bool) string { } fields, jsTag := c.translateSelection(sel, l.Pos()) if jsTag != "" { - return fmt.Sprintf("%s.%s.%s = %s;", c.translateExpr(l.X), strings.Join(fields, "."), jsTag, c.externalize(rhsExpr.String(), sel.Type())) + return fmt.Sprintf("%s.%s%s = %s;", c.translateExpr(l.X), strings.Join(fields, "."), formatJSStructTagVal(jsTag), c.externalize(rhsExpr.String(), sel.Type())) } return fmt.Sprintf("%s.%s = %s;", c.translateExpr(l.X), strings.Join(fields, "."), rhsExpr) case *ast.StarExpr: diff --git a/compiler/utils.go b/compiler/utils.go index 8e385229d..d48cdd02b 100644 --- a/compiler/utils.go +++ b/compiler/utils.go @@ -12,6 +12,8 @@ import ( "sort" "strconv" "strings" + "text/template" + "unicode" "github.com/gopherjs/gopherjs/compiler/analysis" "github.com/gopherjs/gopherjs/compiler/typesutil" @@ -643,3 +645,30 @@ func endsWithReturn(stmts []ast.Stmt) bool { func encodeIdent(name string) string { return strings.Replace(url.QueryEscape(name), "%", "$", -1) } + +// formatJSStructTagVal returns a string of JavaScript code appropriate for +// accessing the property identified by jsTag. If the jsTag value can be safely +// encoded in JavaScript using the dot notation this is used, else we fallback +// to the bracket notation. For more details see: +// +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Property_Accessors +// +// Uses definition of an identifier from +// https://developer.mozilla.org/en-US/docs/Glossary/Identifier +func formatJSStructTagVal(jsTag string) string { + useDot := true + + for i, r := range jsTag { + ok := unicode.IsLetter(r) || (i != 0 && unicode.IsNumber(r)) || r == '$' || r == '_' + if !ok { + useDot = false + break + } + } + + if useDot { + return "." + jsTag + } + + return "[\"" + template.JSEscapeString(jsTag) + "\"]" +} diff --git a/js/js_test.go b/js/js_test.go index cbac7c7db..8aa5659a6 100644 --- a/js/js_test.go +++ b/js/js_test.go @@ -597,3 +597,34 @@ func TestTypeSwitchJSObject(t *testing.T) { } } } + +type StructWithNonIdentifierJsTags struct { + object *js.Object + Name string `js:"@&\"'<>//my name"` +} + +func TestStructWithNonIdentifierJsTags(t *testing.T) { + s := StructWithNonIdentifierJsTags{ + object: js.Global.Get("Object").New(), + } + + const want = "Paul" + + // externalise a value + s.Name = want + + // internalise again + got := s.Name + + if want != got { + t.Errorf("Value via non-identifier js tag field gave %q, want %q", got, want) + } + + // verify we can do a Get with the struct tag + got = s.object.Get("@&\"'<>//my name").String() + + if want != got { + t.Errorf("Value via .Get gave %q, want %q", got, want) + } + +} From 6672256c9b6ab8eb4f725fd927dd8dc83fede71f Mon Sep 17 00:00:00 2001 From: Paul Jolly Date: Sun, 15 Apr 2018 07:32:53 +0100 Subject: [PATCH 2/3] Address feedback in new commit --- compiler/utils.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/compiler/utils.go b/compiler/utils.go index d48cdd02b..d88c622d5 100644 --- a/compiler/utils.go +++ b/compiler/utils.go @@ -656,19 +656,12 @@ func encodeIdent(name string) string { // Uses definition of an identifier from // https://developer.mozilla.org/en-US/docs/Glossary/Identifier func formatJSStructTagVal(jsTag string) string { - useDot := true - for i, r := range jsTag { ok := unicode.IsLetter(r) || (i != 0 && unicode.IsNumber(r)) || r == '$' || r == '_' if !ok { - useDot = false - break + return "[\"" + template.JSEscapeString(jsTag) + "\"]" } } - if useDot { - return "." + jsTag - } - - return "[\"" + template.JSEscapeString(jsTag) + "\"]" + return "." + jsTag } From 5ade898a39d8739bbe2c67f7776af633204d1dda Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Mon, 16 Apr 2018 00:21:07 -0400 Subject: [PATCH 3/3] Apply minor style and documentation changes. Add examples to show what kind of output formatJSStructTagVal returns. Simplify and shorten test. Remove unhelpful blank lines, they're best used sparingly. --- compiler/utils.go | 24 +++++++++++++++--------- js/js_test.go | 34 +++++++++++++--------------------- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/compiler/utils.go b/compiler/utils.go index d88c622d5..d5452e0a6 100644 --- a/compiler/utils.go +++ b/compiler/utils.go @@ -646,22 +646,28 @@ func encodeIdent(name string) string { return strings.Replace(url.QueryEscape(name), "%", "$", -1) } -// formatJSStructTagVal returns a string of JavaScript code appropriate for -// accessing the property identified by jsTag. If the jsTag value can be safely -// encoded in JavaScript using the dot notation this is used, else we fallback -// to the bracket notation. For more details see: +// formatJSStructTagVal returns JavaScript code for accessing an object's property +// identified by jsTag. It prefers the dot notation over the bracket notation when +// possible, since the dot notation produces slightly smaller output. // -// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Property_Accessors +// For example: +// +// "my_name" -> ".my_name" +// "my name" -> `["my name"]` +// +// For more information about JavaScript property accessors and identifiers, see +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Property_Accessors and +// https://developer.mozilla.org/en-US/docs/Glossary/Identifier. // -// Uses definition of an identifier from -// https://developer.mozilla.org/en-US/docs/Glossary/Identifier func formatJSStructTagVal(jsTag string) string { for i, r := range jsTag { ok := unicode.IsLetter(r) || (i != 0 && unicode.IsNumber(r)) || r == '$' || r == '_' if !ok { - return "[\"" + template.JSEscapeString(jsTag) + "\"]" + // Saw an invalid JavaScript identifier character, + // so use bracket notation. + return `["` + template.JSEscapeString(jsTag) + `"]` } } - + // Safe to use dot notation without any escaping. return "." + jsTag } diff --git a/js/js_test.go b/js/js_test.go index 8aa5659a6..39afc530d 100644 --- a/js/js_test.go +++ b/js/js_test.go @@ -598,33 +598,25 @@ func TestTypeSwitchJSObject(t *testing.T) { } } -type StructWithNonIdentifierJsTags struct { - object *js.Object - Name string `js:"@&\"'<>//my name"` -} - -func TestStructWithNonIdentifierJsTags(t *testing.T) { - s := StructWithNonIdentifierJsTags{ - object: js.Global.Get("Object").New(), +func TestStructWithNonIdentifierJSTag(t *testing.T) { + type S struct { + *js.Object + Name string `js:"@&\"'<>//my name"` } + s := S{Object: js.Global.Get("Object").New()} - const want = "Paul" + // externalise a value via field + s.Name = "Paul" - // externalise a value - s.Name = want - - // internalise again + // internalise via field got := s.Name - - if want != got { - t.Errorf("Value via non-identifier js tag field gave %q, want %q", got, want) + if want := "Paul"; got != want { + t.Errorf("value via field with non-identifier js tag gave %q, want %q", got, want) } // verify we can do a Get with the struct tag - got = s.object.Get("@&\"'<>//my name").String() - - if want != got { - t.Errorf("Value via .Get gave %q, want %q", got, want) + got = s.Get("@&\"'<>//my name").String() + if want := "Paul"; got != want { + t.Errorf("value via js.Object.Get gave %q, want %q", got, want) } - }