Apply minor style and documentation changes.

dmitshur · dmitshur · commit 5ade898a39d8 · 2018-04-16T00:39:20.000-04:00
Add examples to show what kind of output formatJSStructTagVal returns.

Simplify and shorten test.

Remove unhelpful blank lines, they're best used sparingly.
diff --git 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
@@ -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)
 	}
-
 }
