diff --git a/tools/structfield/structfield.go b/tools/structfield/structfield.go index 19ac0db4de8..dd25e538877 100644 --- a/tools/structfield/structfield.go +++ b/tools/structfield/structfield.go @@ -146,18 +146,83 @@ func processTag(structName string, goField *ast.Ident, field *ast.Field, structT return } - if strings.Contains(tagName, ",omitempty") { - checkGoFieldType(structName, goField.Name, field, field.Type.Pos(), pass, allowedTagTypes) + hasOmitEmpty := strings.Contains(tagName, ",omitempty") + hasOmitZero := strings.Contains(tagName, ",omitzero") + + if hasOmitEmpty || hasOmitZero { + if tagType == "url" && hasOmitZero { + const msg = "the %q field in struct %q uses unsupported omitzero tag for URL tags" + pass.Reportf(field.Pos(), msg, goField.Name, structName) + } else { + checkGoFieldType(structName, goField.Name, field, field.Pos(), pass, allowedTagTypes, hasOmitEmpty, hasOmitZero) + } + tagName = strings.ReplaceAll(tagName, ",omitzero", "") tagName = strings.ReplaceAll(tagName, ",omitempty", "") } - if tagType == "url" { tagName = strings.ReplaceAll(tagName, ",comma", "") } - checkGoFieldName(structName, goField.Name, tagName, goField.Pos(), pass, allowedTagNames) } +func checkAndReportInvalidTypesForOmitzero(structName, goFieldName string, fieldType ast.Expr, tokenPos token.Pos, pass *analysis.Pass) bool { + switch ft := fieldType.(type) { + case *ast.StarExpr: + // Check for *[]T where T is builtin - should be []T + if arrType, ok := ft.X.(*ast.ArrayType); ok { + if ident, ok := arrType.Elt.(*ast.Ident); ok && isBuiltinType(ident.Name) { + const msg = "change the %q field type to %q in the struct %q" + pass.Reportf(tokenPos, msg, goFieldName, "[]"+ident.Name, structName) + } else if starExpr, ok := arrType.Elt.(*ast.StarExpr); ok { + // Check for *[]*T - should be []*T + if ident, ok := starExpr.X.(*ast.Ident); ok { + const msg = "change the %q field type to %q in the struct %q" + pass.Reportf(tokenPos, msg, goFieldName, "[]*"+ident.Name, structName) + } + } else { + checkStructArrayType(structName, goFieldName, arrType, tokenPos, pass) + } + return true + } + // Check for *int - should not to be used with omitzero only with omitempty + if ident, ok := ft.X.(*ast.Ident); ok { + if isBuiltinType(ident.Name) { + const msg = `the %q field in struct %q uses "omitzero" with a primitive type; remove "omitzero" and use only "omitempty" for pointer primitive types"` + pass.Reportf(tokenPos, msg, goFieldName, structName) + return true + } + } + // Check for *map - should be map + if _, ok := ft.X.(*ast.MapType); ok { + const msg = "change the %q field type to %q in the struct %q" + pass.Reportf(tokenPos, msg, goFieldName, exprToString(ft.X), structName) + return true + } + return true + case *ast.MapType: + return true + case *ast.ArrayType: + checkStructArrayType(structName, goFieldName, ft, tokenPos, pass) + return true + case *ast.Ident: + if obj := pass.TypesInfo.ObjectOf(ft); obj != nil { + switch obj.Type().Underlying().(type) { + case *types.Struct: + // For Struct - should be *Struct + const msg = "change the %q field type to %q in the struct %q" + pass.Reportf(tokenPos, msg, goFieldName, "*"+ft.Name, structName) + return true + case *types.Basic: + // For Builtin - should not to be used with omitzero + const msg = `the %q field in struct %q uses "omitzero" with a primitive type; remove "omitzero", as it is only allowed with structs, maps, and slices` + pass.Reportf(tokenPos, msg, goFieldName, structName) + return true + } + } + } + return false +} + func checkGoFieldName(structName, goFieldName, tagName string, tokenPos token.Pos, pass *analysis.Pass, allowedNames map[string]bool) { fullName := structName + "." + goFieldName if allowedNames[fullName] { @@ -171,16 +236,24 @@ func checkGoFieldName(structName, goFieldName, tagName string, tokenPos token.Po } } -func checkGoFieldType(structName, goFieldName string, field *ast.Field, tokenPos token.Pos, pass *analysis.Pass, allowedTypes map[string]bool) { +func checkGoFieldType(structName, goFieldName string, field *ast.Field, tokenPos token.Pos, pass *analysis.Pass, allowedTypes map[string]bool, omitempty, omitzero bool) { if allowedTypes[structName+"."+goFieldName] { return } + switch { + case omitzero: + skipOmitzero := checkAndReportInvalidTypesForOmitzero(structName, goFieldName, field.Type, tokenPos, pass) + if !skipOmitzero { + const msg = `the %q field in struct %q uses "omitzero"; remove "omitzero", as it is only allowed with structs, maps, and slices` + pass.Reportf(tokenPos, msg, goFieldName, structName) + } - skipOmitempty := checkAndReportInvalidTypes(structName, goFieldName, field.Type, tokenPos, pass) - - if !skipOmitempty { - const msg = `change the %q field type to %q in the struct %q because its tag uses "omitempty"` - pass.Reportf(tokenPos, msg, goFieldName, "*"+exprToString(field.Type), structName) + case omitempty: + skipOmitempty := checkAndReportInvalidTypes(structName, goFieldName, field.Type, tokenPos, pass) + if !skipOmitempty { + const msg = `change the %q field type to %q in the struct %q because its tag uses "omitempty"` + pass.Reportf(tokenPos, msg, goFieldName, "*"+exprToString(field.Type), structName) + } } } diff --git a/tools/structfield/testdata/src/has-warnings/main.go b/tools/structfield/testdata/src/has-warnings/main.go index 26d2704fc78..4d0576e1294 100644 --- a/tools/structfield/testdata/src/has-warnings/main.go +++ b/tools/structfield/testdata/src/has-warnings/main.go @@ -22,6 +22,23 @@ type JSONFieldType struct { PointerToSliceOfPointerStructs *[]*Struct `json:"pointer_to_slice_of_pointer_structs,omitempty"` // want `change the "PointerToSliceOfPointerStructs" field type to "\[\]\*Struct" in the struct "JSONFieldType"` PointerToMap *map[string]string `json:"pointer_to_map,omitempty"` // want `change the "PointerToMap" field type to "map\[string\]string" in the struct "JSONFieldType"` SliceOfInts []*int `json:"slice_of_ints,omitempty"` // want `change the "SliceOfInts" field type to "\[\]int" in the struct "JSONFieldType"` + + Count int `json:"count,omitzero"` // want `the "Count" field in struct "JSONFieldType" uses "omitzero" with a primitive type; remove "omitzero", as it is only allowed with structs, maps, and slices` + Size *int `json:"size,omitzero"` // want `the "Size" field in struct "JSONFieldType" uses "omitzero" with a primitive type; remove "omitzero" and use only "omitempty" for pointer primitive types` + PointerToSliceOfStringsZero *[]string `json:"pointer_to_slice_of_strings_zero,omitzero"` // want `change the "PointerToSliceOfStringsZero" field type to "\[\]string" in the struct "JSONFieldType"` + PointerToSliceOfStructsZero *[]Struct `json:"pointer_to_slice_of_structs_zero,omitzero"` // want `change the "PointerToSliceOfStructsZero" field type to "\[\]\*Struct" in the struct "JSONFieldType"` + PointerToSliceOfPointerStructsZero *[]*Struct `json:"pointer_to_slice_of_pointer_structs_zero,omitzero"` // want `change the "PointerToSliceOfPointerStructsZero" field type to "\[\]\*Struct" in the struct "JSONFieldType"` + PointerSliceInt *[]int `json:"pointer_slice_int,omitzero"` // want `change the "PointerSliceInt" field type to "\[\]int" in the struct "JSONFieldType"` + AnyZero any `json:"any_zero,omitzero"` // want `the "AnyZero" field in struct "JSONFieldType" uses "omitzero"; remove "omitzero", as it is only allowed with structs, maps, and slices` + StringPointerSlice []*string `json:"string_pointer_slice,omitzero"` // want `change the "StringPointerSlice" field type to "\[\]string" in the struct "JSONFieldType"` + PointerToMapZero *map[string]string `json:"pointer__to_map_zero,omitzero"` // want `change the "PointerToMapZero" field type to "map\[string\]string" in the struct "JSONFieldType"` + SliceOfStructsZero []Struct `json:"slice_of_structs_zero,omitzero"` // want `change the "SliceOfStructsZero" field type to "\[\]\*Struct" in the struct "JSONFieldType"` + StructZero Struct `json:"struct_zero,omitzero"` // want `change the "StructZero" field type to "\*Struct" in the struct "JSONFieldType"` + + AnyBoth any `json:"any_both,omitempty,omitzero"` // want `the "AnyBoth" field in struct "JSONFieldType" uses "omitzero"; remove "omitzero", as it is only allowed with structs, maps, and slices` + NonPointerStructBoth Struct `json:"non_pointer_struct_both,omitempty,omitzero"` // want `change the "NonPointerStructBoth" field type to "\*Struct" in the struct "JSONFieldType"` + PointerStringBoth *string `json:"pointer_string_both,omitempty,omitzero"` // want `the "PointerStringBoth" field in struct "JSONFieldType" uses "omitzero" with a primitive type; remove "omitzero" and use only "omitempty" for pointer primitive types` + StructZeroBoth Struct `json:"struct_zero_both,omitempty,omitzero"` // want `change the "StructZeroBoth" field type to "\*Struct" in the struct "JSONFieldType"` } type Struct struct{} @@ -34,4 +51,7 @@ type URLFieldType struct { Page string `url:"page,omitempty"` // want `change the "Page" field type to "\*string" in the struct "URLFieldType" because its tag uses "omitempty"` PerPage int `url:"per_page,omitempty"` // want `change the "PerPage" field type to "\*int" in the struct "URLFieldType" because its tag uses "omitempty"` Participating bool `url:"participating,omitempty"` // want `change the "Participating" field type to "\*bool" in the struct "URLFieldType" because its tag uses "omitempty"` + + PerPageZeros []int `url:"per_page_zeros,omitzero"` // want `the "PerPageZeros" field in struct "URLFieldType" uses unsupported omitzero tag for URL tags` + PerPageBoth *int `url:"per_page_both,omitempty,omitzero"` // want `the "PerPageBoth" field in struct "URLFieldType" uses unsupported omitzero tag for URL tags` } diff --git a/tools/structfield/testdata/src/no-warnings/main.go b/tools/structfield/testdata/src/no-warnings/main.go index fc0236f1cfe..7c43789c659 100644 --- a/tools/structfield/testdata/src/no-warnings/main.go +++ b/tools/structfield/testdata/src/no-warnings/main.go @@ -27,6 +27,16 @@ type JSONFieldType struct { Exception string `json:"exception,omitempty"` Value any `json:"value,omitempty"` SliceOfPointerStructs []*Struct `json:"slice_of_pointer_structs,omitempty"` + + SliceOfStrings []string `json:"slice_of_strings,omitzero"` + MapOfStringToInt map[string]int `json:"map_of_string_to_int,omitzero"` + PointerStructField *Struct `json:"pointer_struct_field,omitzero"` + SliceOfPointerStructsZero []*Struct `json:"slice_of_pointer_structs_zero,omitzero"` + + SliceOfStringsBoth []string `json:"slice_of_strings_both,omitzero,omitempty"` + MapOfStringToIntBoth map[string]int `json:"map_of_string_to_int_both,omitzero,omitempty"` + SliceOfPointerStructsBoth []*Struct `json:"slice_of_pointer_structs_both,omitzero,omitempty"` + StructFieldBoth *Struct `json:"struct_field_both,omitzero,omitempty"` } type URLFieldName struct {