go/analysis/passes/structtag: recognize multiple keys per tag

As of Go 1.16 the reflect package now supports multiple keys per tag.

For golang/go#40281
Fixes golang/go#43083

Change-Id: I55cdc35c857a5e73dc009c2842d7bd83c63d7712
Reviewed-on: https://go-review.googlesource.com/c/tools/+/277092
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>

go/analysis/passes/structtag/structtag.go

@@ -207,12 +207,12 @@ var (
 )
 
 // validateStructTag parses the struct tag and returns an error if it is not
-// in the canonical format, which is a space-separated list of key:"value"
-// settings. The value may contain spaces.
+// in the canonical format, as defined by reflect.StructTag.
 func validateStructTag(tag string) error {
 	// This code is based on the StructTag.Get code in package reflect.
 
 	n := 0
+	var keys []string
 	for ; tag != ""; n++ {
 		if n > 0 && tag != "" && tag[0] != ' ' {
 			// More restrictive than reflect, but catches likely mistakes
@@ -240,14 +240,27 @@ func validateStructTag(tag string) error {
 		if i == 0 {
 			return errTagKeySyntax
 		}
-		if i+1 >= len(tag) || tag[i] != ':' {
+		if i+1 >= len(tag) || tag[i] < ' ' || tag[i] == 0x7f {
 			return errTagSyntax
 		}
-		if tag[i+1] != '"' {
+		key := tag[:i]
+		keys = append(keys, key)
+		tag = tag[i:]
+
+		// If we found a space char here - assume that we have a tag with
+		// multiple keys.
+		if tag[0] == ' ' {
+			continue
+		}
+
+		// Spaces were filtered above so we assume that here we have
+		// only valid tag value started with `:"`.
+		if tag[0] != ':' || tag[1] != '"' {
 			return errTagValueSyntax
 		}
-		key := tag[:i]
-		tag = tag[i+1:]
+
+		// Remove the colon leaving tag at the start of the quoted string.
+		tag = tag[1:]
 
 		// Scan quoted string to find value.
 		i = 1
@@ -263,51 +276,56 @@ func validateStructTag(tag string) error {
 		qvalue := tag[:i+1]
 		tag = tag[i+1:]
 
-		value, err := strconv.Unquote(qvalue)
+		wholeValue, err := strconv.Unquote(qvalue)
 		if err != nil {
 			return errTagValueSyntax
 		}
 
-		if !checkTagSpaces[key] {
-			continue
-		}
-
-		switch key {
-		case "xml":
-			// If the first or last character in the XML tag is a space, it is
-			// suspicious.
-			if strings.Trim(value, " ") != value {
-				return errTagValueSpace
+		for _, key := range keys {
+			if !checkTagSpaces[key] {
+				continue
 			}
 
-			// If there are multiple spaces, they are suspicious.
-			if strings.Count(value, " ") > 1 {
-				return errTagValueSpace
-			}
+			value := wholeValue
+			switch key {
+			case "xml":
+				// If the first or last character in the XML tag is a space, it is
+				// suspicious.
+				if strings.Trim(value, " ") != value {
+					return errTagValueSpace
+				}
 
-			// If there is no comma, skip the rest of the checks.
-			comma := strings.IndexRune(value, ',')
-			if comma < 0 {
-				continue
+				// If there are multiple spaces, they are suspicious.
+				if strings.Count(value, " ") > 1 {
+					return errTagValueSpace
+				}
+
+				// If there is no comma, skip the rest of the checks.
+				comma := strings.IndexRune(value, ',')
+				if comma < 0 {
+					continue
+				}
+
+				// If the character before a comma is a space, this is suspicious.
+				if comma > 0 && value[comma-1] == ' ' {
+					return errTagValueSpace
+				}
+				value = value[comma+1:]
+			case "json":
+				// JSON allows using spaces in the name, so skip it.
+				comma := strings.IndexRune(value, ',')
+				if comma < 0 {
+					continue
+				}
+				value = value[comma+1:]
 			}
 
-			// If the character before a comma is a space, this is suspicious.
-			if comma > 0 && value[comma-1] == ' ' {
+			if strings.IndexByte(value, ' ') >= 0 {
 				return errTagValueSpace
 			}
-			value = value[comma+1:]
-		case "json":
-			// JSON allows using spaces in the name, so skip it.
-			comma := strings.IndexRune(value, ',')
-			if comma < 0 {
-				continue
-			}
-			value = value[comma+1:]
 		}
 
-		if strings.IndexByte(value, ' ') >= 0 {
-			return errTagValueSpace
-		}
+		keys = keys[:0]
 	}
 	return nil
 }

go/analysis/passes/structtag/structtag_go16_test.go

@@ -0,0 +1,21 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build go1.16
+
+package structtag_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/analysis/passes/structtag"
+)
+
+// Test the multiple key format added in Go 1.16.
+
+func TestGo16(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, structtag.Analyzer, "go16")
+}

go/analysis/passes/structtag/testdata/src/go16/go16.go

@@ -0,0 +1,27 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Tests for the new multiple-key struct tag format supported in 1.16.
+
+package go16
+
+type Go16StructTagTest struct {
+	OK int `multiple keys can:"share a value"`
+	OK2 int `json bson xml form:"field_1,omitempty" other:"value"`
+}
+
+type Go16UnexportedEncodingTagTest struct {
+	F int `json xml:"ff"`
+
+	// We currently always check json first, and return after an error.
+	f1 int `json xml:"f1"` // want "struct field f1 has json tag but is not exported"
+	f2 int `xml json:"f2"` // want "struct field f2 has json tag but is not exported"
+	f3 int `xml bson:"f3"` // want "struct field f3 has xml tag but is not exported"
+	f4 int `bson xml:"f4"` // want "struct field f4 has xml tag but is not exported"
+}
+
+type Go16DuplicateFields struct {
+	JSONXML int `json xml:"c"`
+	DuplicateJSONXML int `json xml:"c"` // want "struct field DuplicateJSONXML repeats json tag .c. also at go16.go:25" "struct field DuplicateJSONXML repeats xml tag .c. also at go16.go:25"
+}