Skip to content

Commit

Permalink
cmd/go: add vet check for json/xml omitempty struct tags on non-basic…
Browse files Browse the repository at this point in the history
… types

Fixes golang/go#51261

Change-Id: I99c3a0afc5171777b39f8288e2159f8554151e43
  • Loading branch information
andig committed Oct 16, 2023
1 parent 43c41b5 commit faa46f8
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 5 deletions.
2 changes: 1 addition & 1 deletion cmd/guru/serial/serial.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ type FreeVar struct {
// types to which it is assignable, and the set of named/*named types
// (concrete or non-empty interface) which may be assigned to it.
type Implements struct {
T ImplementsType `json:"type,omitempty"` // the queried type
T ImplementsType `json:"type"` // the queried type
AssignableTo []ImplementsType `json:"to,omitempty"` // types assignable to T
AssignableFrom []ImplementsType `json:"from,omitempty"` // interface types assignable from T
AssignableFromPtr []ImplementsType `json:"fromptr,omitempty"` // interface types assignable only from *T
Expand Down
26 changes: 24 additions & 2 deletions go/analysis/passes/structtag/structtag.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"go/types"
"path/filepath"
"reflect"
"slices"
"strconv"
"strings"

Expand Down Expand Up @@ -83,8 +84,10 @@ func (s *namesSeen) Set(key, name string, level int, pos token.Pos) {
(*s)[uniqueName{key, name, level}] = pos
}

var checkTagDups = []string{"json", "xml"}
var checkTagSpaces = map[string]bool{"json": true, "xml": true, "asn1": true}
var (
checkTagDups = []string{"json", "xml"}
checkTagSpaces = map[string]bool{"json": true, "xml": true, "asn1": true}
)

// checkCanonicalFieldTag checks a single struct field tag.
func checkCanonicalFieldTag(pass *analysis.Pass, field *types.Var, tag string, seen *namesSeen) {
Expand Down Expand Up @@ -112,6 +115,25 @@ func checkCanonicalFieldTag(pass *analysis.Pass, field *types.Var, tag string, s
return
}

// Check for use of json or xml omitempty tags with unsupported field types.
for _, enc := range [...]string{"json", "xml"} {
typ := field.Type()
switch typ.Underlying().(type) {
case *types.Basic, *types.Interface, *types.Map, *types.Pointer, *types.Slice:
continue
}

val := reflect.StructTag(tag).Get(enc)
pos := strings.IndexRune(val, ',')
if pos < 0 {
continue
}

if slices.Contains(strings.Split(val[pos+1:], ","), "omitempty") {
pass.Reportf(field.Pos(), "struct field %s has %s tag but underlying type %s is not an omittable type", field.Name(), val, typ.String())
}
}

if field.Exported() {
return
}
Expand Down
13 changes: 11 additions & 2 deletions go/analysis/passes/structtag/testdata/src/a/a.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package a
import (
"a/b"
"encoding/xml"
"time"
)

type StructTagTest struct {
Expand Down Expand Up @@ -44,8 +45,10 @@ type JSONEmbeddedField struct {
unexp `is:"embedded,notexported" json:"unexp"` // OK for now, see issue 7363
}

type AnonymousJSON struct{}
type AnonymousXML struct{}
type (
AnonymousJSON struct{}
AnonymousXML struct{}
)

type AnonymousJSONField struct {
DuplicateAnonJSON int `json:"a"`
Expand Down Expand Up @@ -138,3 +141,9 @@ type DuplicateWithAnotherPackage struct {
b.AnonymousJSONField
AnonymousJSONField2 // want "struct field DuplicateAnonJSON repeats json tag .a. also at b.b.go:8"
}

type OmitemptyUsage struct {
T0 string `json:",omitempty"`
T1 time.Time `json:"omitempty"`
T2 time.Time `json:"t2,omitempty"` // want "struct field T2 has t2,omitempty tag but underlying type time.Time is not an omittable type"
}

0 comments on commit faa46f8

Please sign in to comment.