cmd/vet: diagnose non-space-separated struct tag like `json:"x",xml:"y"`

This is not strictly illegal but it probably should be (too late)
and doesn't mean what it looks like it means:
the second key-value pair has the key ",xml".

Fixes #14466.

Change-Id: I174bccc23fd28affeb87f57f77c6591634ade641
Reviewed-on: https://go-review.googlesource.com/32031
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Quentin Smith <quentin@golang.org>

src/cmd/vet/structtag.go

@@ -46,7 +46,7 @@ func checkCanonicalFieldTag(f *File, field *ast.Field, seen *map[[2]string]token
 
 	if err := validateStructTag(tag); err != nil {
 		raw, _ := strconv.Unquote(field.Tag.Value) // field.Tag.Value is known to be a quoted string
-		f.Badf(field.Pos(), "struct field tag %q not compatible with reflect.StructTag.Get: %s", raw, err)
+		f.Badf(field.Pos(), "struct field tag %#q not compatible with reflect.StructTag.Get: %s", raw, err)
 	}
 
 	for _, key := range checkTagDups {
@@ -91,6 +91,7 @@ var (
 	errTagSyntax      = errors.New("bad syntax for struct tag pair")
 	errTagKeySyntax   = errors.New("bad syntax for struct tag key")
 	errTagValueSyntax = errors.New("bad syntax for struct tag value")
+	errTagSpace       = errors.New("key:\"value\" pairs not separated by spaces")
 )
 
 // validateStructTag parses the struct tag and returns an error if it is not
@@ -99,7 +100,13 @@ var (
 func validateStructTag(tag string) error {
 	// This code is based on the StructTag.Get code in package reflect.
 
-	for tag != "" {
+	n := 0
+	for ; tag != ""; n++ {
+		if n > 0 && tag != "" && tag[0] != ' ' {
+			// More restrictive than reflect, but catches likely mistakes
+			// like `x:"foo",y:"bar"`, which parses as `x:"foo" ,y:"bar"` with second key ",y".
+			return errTagSpace
+		}
 		// Skip leading space.
 		i := 0
 		for i < len(tag) && tag[i] == ' ' {

src/cmd/vet/testdata/structtag.go

@@ -15,6 +15,8 @@ type StructTagTest struct {
 	F   int `:"emptykey"`      // ERROR "not compatible with reflect.StructTag.Get: bad syntax for struct tag key"
 	G   int `x:"noEndQuote`    // ERROR "not compatible with reflect.StructTag.Get: bad syntax for struct tag value"
 	H   int `x:"trunc\x0"`     // ERROR "not compatible with reflect.StructTag.Get: bad syntax for struct tag value"
+	I   int `x:"foo",y:"bar"`  // ERROR "not compatible with reflect.StructTag.Get: key:.value. pairs not separated by spaces"
+	J   int `x:"foo"y:"bar"`   // ERROR "not compatible with reflect.StructTag.Get: key:.value. pairs not separated by spaces"
 	OK0 int `x:"y" u:"v" w:""`
 	OK1 int `x:"y:z" u:"v" w:""` // note multiple colons.
 	OK2 int "k0:\"values contain spaces\" k1:\"literal\ttabs\" k2:\"and\\tescaped\\tabs\""
@@ -37,12 +39,12 @@ type JSONEmbeddedField struct {
 
 type DuplicateJSONFields struct {
 	JSON              int `json:"a"`
-	DuplicateJSON     int `json:"a"` // ERROR "struct field DuplicateJSON repeats json tag .a. also at testdata/structtag.go:39"
+	DuplicateJSON     int `json:"a"` // ERROR "struct field DuplicateJSON repeats json tag .a. also at testdata/structtag.go:41"
 	IgnoredJSON       int `json:"-"`
 	OtherIgnoredJSON  int `json:"-"`
 	OmitJSON          int `json:",omitempty"`
 	OtherOmitJSON     int `json:",omitempty"`
-	DuplicateOmitJSON int `json:"a,omitempty"` // ERROR "struct field DuplicateOmitJSON repeats json tag .a. also at testdata/structtag.go:39"
+	DuplicateOmitJSON int `json:"a,omitempty"` // ERROR "struct field DuplicateOmitJSON repeats json tag .a. also at testdata/structtag.go:41"
 	NonJSON           int `foo:"a"`
 	DuplicateNonJSON  int `foo:"a"`
 	Embedded          struct {
@@ -50,12 +52,12 @@ type DuplicateJSONFields struct {
 	}
 
 	XML              int `xml:"a"`
-	DuplicateXML     int `xml:"a"` // ERROR "struct field DuplicateXML repeats xml tag .a. also at testdata/structtag.go:52"
+	DuplicateXML     int `xml:"a"` // ERROR "struct field DuplicateXML repeats xml tag .a. also at testdata/structtag.go:54"
 	IgnoredXML       int `xml:"-"`
 	OtherIgnoredXML  int `xml:"-"`
 	OmitXML          int `xml:",omitempty"`
 	OtherOmitXML     int `xml:",omitempty"`
-	DuplicateOmitXML int `xml:"a,omitempty"` // ERROR "struct field DuplicateOmitXML repeats xml tag .a. also at testdata/structtag.go:52"
+	DuplicateOmitXML int `xml:"a,omitempty"` // ERROR "struct field DuplicateOmitXML repeats xml tag .a. also at testdata/structtag.go:54"
 	NonXML           int `foo:"a"`
 	DuplicateNonXML  int `foo:"a"`
 	Embedded         struct {