cmd/vet: check for duplicate json, xml struct field tags

It is easy to make the mistake of duplicating json struct field
tags especially when copy/pasting. This commit causes go vet to
report the mistake. Only field tags in the same struct type are
considered, because that is the only case which is undoubtedly an
error.

Fixes #12791.

Change-Id: I4130e4c04b177694cc0daf8f1acaf0751d4f062b
Reviewed-on: https://go-review.googlesource.com/16704
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

src/cmd/vet/main.go

@@ -133,13 +133,13 @@ var (
 	callExpr      *ast.CallExpr
 	compositeLit  *ast.CompositeLit
 	exprStmt      *ast.ExprStmt
-	field         *ast.Field
 	funcDecl      *ast.FuncDecl
 	funcLit       *ast.FuncLit
 	genDecl       *ast.GenDecl
 	interfaceType *ast.InterfaceType
 	rangeStmt     *ast.RangeStmt
 	returnStmt    *ast.ReturnStmt
+	structType    *ast.StructType
 
 	// checkers is a two-level map.
 	// The outer level is keyed by a nil pointer, one of the AST vars above.
@@ -478,8 +478,6 @@ func (f *File) Visit(node ast.Node) ast.Visitor {
 		key = compositeLit
 	case *ast.ExprStmt:
 		key = exprStmt
-	case *ast.Field:
-		key = field
 	case *ast.FuncDecl:
 		key = funcDecl
 	case *ast.FuncLit:
@@ -492,6 +490,8 @@ func (f *File) Visit(node ast.Node) ast.Visitor {
 		key = rangeStmt
 	case *ast.ReturnStmt:
 		key = returnStmt
+	case *ast.StructType:
+		key = structType
 	}
 	for _, fn := range f.checkers[key] {
 		fn(f, node)

src/cmd/vet/structtag.go

@@ -9,20 +9,31 @@ package main
 import (
 	"errors"
 	"go/ast"
+	"go/token"
 	"reflect"
 	"strconv"
+	"strings"
 )
 
 func init() {
 	register("structtags",
 		"check that struct field tags have canonical format and apply to exported fields as needed",
-		checkCanonicalFieldTag,
-		field)
+		checkStructFieldTags,
+		structType)
 }
 
-// checkCanonicalFieldTag checks a struct field tag.
-func checkCanonicalFieldTag(f *File, node ast.Node) {
-	field := node.(*ast.Field)
+// checkStructFieldTags checks all the field tags of a struct, including checking for duplicates.
+func checkStructFieldTags(f *File, node ast.Node) {
+	var seen map[[2]string]token.Pos
+	for _, field := range node.(*ast.StructType).Fields.List {
+		checkCanonicalFieldTag(f, field, &seen)
+	}
+}
+
+var checkTagDups = []string{"json", "xml"}
+
+// checkCanonicalFieldTag checks a single struct field tag.
+func checkCanonicalFieldTag(f *File, field *ast.Field, seen *map[[2]string]token.Pos) {
 	if field.Tag == nil {
 		return
 	}
@@ -38,6 +49,24 @@ func checkCanonicalFieldTag(f *File, node ast.Node) {
 		f.Badf(field.Pos(), "struct field tag %q not compatible with reflect.StructTag.Get: %s", raw, err)
 	}
 
+	for _, key := range checkTagDups {
+		val := reflect.StructTag(tag).Get(key)
+		if val == "" || val == "-" || val[0] == ',' {
+			continue
+		}
+		if i := strings.Index(val, ","); i >= 0 {
+			val = val[:i]
+		}
+		if *seen == nil {
+			*seen = map[[2]string]token.Pos{}
+		}
+		if pos, ok := (*seen)[[2]string{key, val}]; ok {
+			f.Badf(field.Pos(), "struct field %s repeats %s tag %q also at %s", field.Names[0].Name, key, val, f.loc(pos))
+		} else {
+			(*seen)[[2]string{key, val}] = field.Pos()
+		}
+	}
+
 	// Check for use of json or xml tags with unexported fields.
 
 	// Embedded struct. Nothing to do for now, but that
@@ -50,9 +79,8 @@ func checkCanonicalFieldTag(f *File, node ast.Node) {
 		return
 	}
 
-	st := reflect.StructTag(tag)
 	for _, enc := range [...]string{"json", "xml"} {
-		if st.Get(enc) != "" {
+		if reflect.StructTag(tag).Get(enc) != "" {
 			f.Badf(field.Pos(), "struct field %s has %s tag but is not exported", field.Names[0].Name, enc)
 			return
 		}

src/cmd/vet/testdata/structtag.go

@@ -34,3 +34,31 @@ type JSONEmbeddedField struct {
 	UnexportedEncodingTagTest `is:"embedded"`
 	unexp                     `is:"embedded,notexported" json:"unexp"` // OK for now, see issue 7363
 }
+
+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"
+	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"
+	NonJSON           int `foo:"a"`
+	DuplicateNonJSON  int `foo:"a"`
+	Embedded          struct {
+		DuplicateJSON int `json:"a"` // OK because its not in the same struct type
+	}
+
+	XML              int `xml:"a"`
+	DuplicateXML     int `xml:"a"` // ERROR "struct field DuplicateXML repeats xml tag .a. also at testdata/structtag.go:52"
+	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"
+	NonXML           int `foo:"a"`
+	DuplicateNonXML  int `foo:"a"`
+	Embedded         struct {
+		DuplicateXML int `xml:"a"` // OK because its not in the same struct type
+	}
+}