Skip to content

feat: Handle omitzero in structfield linter #3881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Not-Dhananjay-Mishra wants to merge 5 commits into
base: master
Choose a base branch
from
+123 −10
Open

feat: Handle omitzero in structfield linter #3881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7454100
add omitzero check in structfield linter
Not-Dhananjay-Mishra Dec 15, 2025
9be5c0b
add test cases
Not-Dhananjay-Mishra Dec 16, 2025
c3eb5ce
allowing omitzero only for slices, maps, and structs
Not-Dhananjay-Mishra Dec 16, 2025
e6ec5c6
add more test
Not-Dhananjay-Mishra Dec 16, 2025
4000936
simplified code
Not-Dhananjay-Mishra Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 96 additions & 10 deletions tools/structfield/structfield.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,84 @@ 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" {
if tagType == "url" && hasOmitEmpty {
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 *Struct
if ident, ok := ft.X.(*ast.Ident); ok {
if obj := pass.TypesInfo.ObjectOf(ident); obj != nil {
if _, ok := obj.Type().Underlying().(*types.Struct); ok {
return true
}
}
}
if arrType, ok := ft.X.(*ast.ArrayType); ok {
// For *[]Struct
if ident, ok := arrType.Elt.(*ast.Ident); ok {
if obj := pass.TypesInfo.ObjectOf(ident); obj != nil {
if _, ok := obj.Type().Underlying().(*types.Struct); ok {
const msg = "change the %q field type to %q in the struct %q"
pass.Reportf(tokenPos, msg, goFieldName, "[]*"+ident.Name, structName)
return true
}
}
}
// For *[]*Struct
if starExpr, ok := arrType.Elt.(*ast.StarExpr); ok {
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)
return true
}
}
// For *[]builtin
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)
return true
}
}
if _, ok := ft.X.(*ast.MapType); ok {
return true
}
// Slice
case *ast.ArrayType:
return true

// Map
case *ast.MapType:
return true

// Struct
case *ast.Ident:
if obj := pass.TypesInfo.ObjectOf(ft); obj != nil {
if _, ok := obj.Type().Underlying().(*types.Struct); ok {
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] {
Expand All @@ -171,16 +237,36 @@ 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 omitempty && omitzero:
skipOmitzero := checkAndReportInvalidTypesForOmitzero(structName, goFieldName, field.Type, tokenPos, pass)
skipOmitempty := checkAndReportInvalidTypes(structName, goFieldName, field.Type, tokenPos, pass)
if !skipOmitzero {
const msg = `change the %q field type to a slice, map, or struct in the struct %q because its tag uses "omitzero"`
pass.Reportf(tokenPos, msg, goFieldName, structName)
}
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)
}

skipOmitempty := checkAndReportInvalidTypes(structName, goFieldName, field.Type, tokenPos, pass)
case omitzero:
skipOmitzero := checkAndReportInvalidTypesForOmitzero(structName, goFieldName, field.Type, tokenPos, pass)
if !skipOmitzero {
const msg = `change the %q field type to a slice, map, or struct in the struct %q because its tag uses "omitzero"`
pass.Reportf(tokenPos, msg, goFieldName, structName)
}

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)
}
}
}

Expand Down
13 changes: 13 additions & 0 deletions tools/structfield/testdata/src/has-warnings/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ 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 `change the "Count" field type to a slice, map, or struct in the struct "JSONFieldType" because its tag uses "omitzero"`
Copy link
Collaborator

@gmlewis gmlewis Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message does not make sense to me.

A user is not going to change the type of a field from a primitive to a slice, map, or struct just to satisfy the "omitzero".

Instead, a user is going to REMOVE the "omitzero" when using a primitive, right?

Copy link
Contributor Author

@Not-Dhananjay-Mishra Not-Dhananjay-Mishra Dec 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message does not make sense to me.

A user is not going to change the type of a field from a primitive to a slice, map, or struct just to satisfy the "omitzero".

Instead, a user is going to REMOVE the "omitzero" when using a primitive, right?

the Count field in struct JSONField uses "omitzero" with a primitive type; remove "omitzero"
How is this message?

Copy link
Contributor Author

@Not-Dhananjay-Mishra Not-Dhananjay-Mishra Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or
the Count field in struct JSONField uses "omitzero" with a primitive type; remove "omitzero", as it is only allowed with structs, maps, and slices

Size *int `json:"size,omitzero"` // want `change the "Size" field type to a slice, map, or struct in the struct "JSONFieldType" because its tag uses "omitzero"`
Copy link
Collaborator

@gmlewis gmlewis Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here. In this case, we probably should suggest they use omitempty instead of omitzero.

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,omitempty"` // want `change the "PointerSliceInt" field type to "\[\]int" in the struct "JSONFieldType"`

PointerStructBoth Struct `json:"pointer_struct_both,omitempty,omitzero"` // want `change the "PointerStructBoth" field type to "\*Struct" in the struct "JSONFieldType" because its tag uses "omitempty"`
StringBoth *string `json:"string_both,omitempty,omitzero"` // want `change the "StringBoth" field type to a slice, map, or struct in the struct "JSONFieldType" because its tag uses "omitzero"`
}

type Struct struct{}
Expand All @@ -34,4 +44,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`
}
14 changes: 14 additions & 0 deletions tools/structfield/testdata/src/no-warnings/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,20 @@ 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"`
SliceOfPointerInts []*int `json:"slice_of_pointer_ints,omitzero"`
MapOfStringToInt map[string]int `json:"map_of_string_to_int,omitzero"`
MapOfPointerStringToInt *map[string]int `json:"map_of_pointer_string_to_int,omitzero"`
StructField Struct `json:"struct_field,omitzero"`
PointerStructField *Struct `json:"pointer_struct_field,omitzero"`
SliceOfPointerStructsZero []*Struct `json:"slice_of_pointer_structs_zero,omitzero"`
SliceOfNonPointerStructsZero []Struct `json:"slice_of_non_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 {
Expand Down
Loading