diff --git a/src/cmd/vet/main.go b/src/cmd/vet/main.go index b317c88738..8149ba04e0 100644 --- a/src/cmd/vet/main.go +++ b/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) diff --git a/src/cmd/vet/structtag.go b/src/cmd/vet/structtag.go index 8134c585b3..1b92aaf51b 100644 --- a/src/cmd/vet/structtag.go +++ b/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 } diff --git a/src/cmd/vet/testdata/structtag.go b/src/cmd/vet/testdata/structtag.go index 6878f5642d..74c7b541cb 100644 --- a/src/cmd/vet/testdata/structtag.go +++ b/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 + } +}