Skip to content

Commit 85e8b42

Browse files
madelinekalilmadelinekalil
committed
gopls/internal/analysis/modernize: omitzero
Adds a new modernizer that suggests removing or replacing instances of "omitempty" on struct fields with "omitzero." Example (before): type Foo struct { A struct{ b int } `json:"name,omitempty" } Example (after - replace): type Foo struct { A struct{ b int } `json:"name,omitzero" } Example (after - remove): type Foo struct { A struct{ b int } `json:"name" } Updates golang/go#70815 Change-Id: I7d651880340d24929ea5cae4751557a1f60e5f8e Reviewed-on: https://go-review.googlesource.com/c/tools/+/640041 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent 344e482 commit 85e8b42

File tree

10 files changed

+266
-46
lines changed

10 files changed

+266
-46
lines changed

‎gopls/doc/analyzers.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,7 @@ existing code by using more modern features of Go, such as:
482482
added in go1.19;
483483
- replacing uses of context.WithCancel in tests with t.Context, added in
484484
go1.24;
485+
- replacing omitempty by omitzero on structs, added in go 1.24
485486

486487
Default: on.
487488

‎gopls/internal/analysis/modernize/doc.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,5 @@
2525
// added in go1.19;
2626
// - replacing uses of context.WithCancel in tests with t.Context, added in
2727
// go1.24;
28+
// - replacing omitempty by omitzero on structs, added in go 1.24
2829
package modernize

‎gopls/internal/analysis/modernize/modernize.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"go/token"
1212
"go/types"
1313
"iter"
14+
"regexp"
1415
"strings"
1516

1617
"golang.org/x/tools/go/analysis"
@@ -65,6 +66,7 @@ func run(pass *analysis.Pass) (any, error) {
6566
fmtappendf(pass)
6667
mapsloop(pass)
6768
minmax(pass)
69+
omitzero(pass)
6870
slicescontains(pass)
6971
sortslice(pass)
7072
testingContext(pass)
@@ -121,12 +123,13 @@ func filesUsing(inspect *inspector.Inspector, info *types.Info, version string)
121123
}
122124

123125
var (
124-
builtinAny = types.Universe.Lookup("any")
125-
builtinAppend = types.Universe.Lookup("append")
126-
builtinBool = types.Universe.Lookup("bool")
127-
builtinFalse = types.Universe.Lookup("false")
128-
builtinMake = types.Universe.Lookup("make")
129-
builtinNil = types.Universe.Lookup("nil")
130-
builtinTrue = types.Universe.Lookup("true")
131-
byteSliceType = types.NewSlice(types.Typ[types.Byte])
126+
builtinAny = types.Universe.Lookup("any")
127+
builtinAppend = types.Universe.Lookup("append")
128+
builtinBool = types.Universe.Lookup("bool")
129+
builtinFalse = types.Universe.Lookup("false")
130+
builtinMake = types.Universe.Lookup("make")
131+
builtinNil = types.Universe.Lookup("nil")
132+
builtinTrue = types.Universe.Lookup("true")
133+
byteSliceType = types.NewSlice(types.Typ[types.Byte])
134+
omitemptyRegex = regexp.MustCompile(`(?:^json| json):"[^"]*(,omitempty)(?:"|,[^"]*")\s?`)
132135
)

‎gopls/internal/analysis/modernize/modernize_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ func Test(t *testing.T) {
1919
"fmtappendf",
2020
"mapsloop",
2121
"minmax",
22+
"omitzero",
2223
"slicescontains",
2324
"sortslice",
2425
"testingcontext",

‎gopls/internal/analysis/modernize/omitzero.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package modernize
6+
7+
import (
8+
"go/ast"
9+
"go/types"
10+
"reflect"
11+
"strconv"
12+
13+
"golang.org/x/tools/go/analysis"
14+
"golang.org/x/tools/go/analysis/passes/inspect"
15+
"golang.org/x/tools/go/ast/inspector"
16+
"golang.org/x/tools/internal/astutil"
17+
)
18+
19+
func checkOmitEmptyField(pass *analysis.Pass, info *types.Info, curField *ast.Field) {
20+
typ := info.TypeOf(curField.Type)
21+
_, ok := typ.Underlying().(*types.Struct)
22+
if !ok {
23+
// Not a struct
24+
return
25+
}
26+
tag := curField.Tag
27+
if tag == nil {
28+
// No tag to check
29+
return
30+
}
31+
// The omitempty tag may be used by other packages besides json, but we should only modify its use with json
32+
tagconv, _ := strconv.Unquote(tag.Value)
33+
match := omitemptyRegex.FindStringSubmatchIndex(tagconv)
34+
if match == nil {
35+
// No omitempty in json tag
36+
return
37+
}
38+
omitEmptyPos, err := astutil.PosInStringLiteral(curField.Tag, match[2])
39+
if err != nil {
40+
return
41+
}
42+
omitEmptyEnd, err := astutil.PosInStringLiteral(curField.Tag, match[3])
43+
if err != nil {
44+
return
45+
}
46+
removePos, removeEnd := omitEmptyPos, omitEmptyEnd
47+
48+
jsonTag := reflect.StructTag(tagconv).Get("json")
49+
if jsonTag == ",omitempty" {
50+
// Remove the entire struct tag if json is the only package used
51+
if match[1]-match[0] == len(tagconv) {
52+
removePos = curField.Tag.Pos()
53+
removeEnd = curField.Tag.End()
54+
} else {
55+
// Remove the json tag if omitempty is the only field
56+
removePos, err = astutil.PosInStringLiteral(curField.Tag, match[0])
57+
if err != nil {
58+
return
59+
}
60+
removeEnd, err = astutil.PosInStringLiteral(curField.Tag, match[1])
61+
if err != nil {
62+
return
63+
}
64+
}
65+
}
66+
pass.Report(analysis.Diagnostic{
67+
Pos: curField.Tag.Pos(),
68+
End: curField.Tag.End(),
69+
Category: "omitzero",
70+
Message: "Omitempty has no effect on nested struct fields",
71+
SuggestedFixes: []analysis.SuggestedFix{
72+
{
73+
Message: "Remove redundant omitempty tag",
74+
TextEdits: []analysis.TextEdit{
75+
{
76+
Pos: removePos,
77+
End: removeEnd,
78+
},
79+
},
80+
},
81+
{
82+
Message: "Replace omitempty with omitzero (behavior change)",
83+
TextEdits: []analysis.TextEdit{
84+
{
85+
Pos: omitEmptyPos,
86+
End: omitEmptyEnd,
87+
NewText: []byte(",omitzero"),
88+
},
89+
},
90+
},
91+
}})
92+
}
93+
94+
// The omitzero pass searches for instances of "omitempty" in a json field tag on a
95+
// struct. Since "omitempty" does not have any effect when applied to a struct field,
96+
// it suggests either deleting "omitempty" or replacing it with "omitzero", which
97+
// correctly excludes structs from a json encoding.
98+
func omitzero(pass *analysis.Pass) {
99+
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
100+
info := pass.TypesInfo
101+
for curFile := range filesUsing(inspect, info, "go1.24") {
102+
for curStruct := range curFile.Preorder((*ast.StructType)(nil)) {
103+
for _, curField := range curStruct.Node().(*ast.StructType).Fields.List {
104+
checkOmitEmptyField(pass, info, curField)
105+
}
106+
}
107+
}
108+
}

‎gopls/internal/analysis/modernize/testdata/src/omitzero/omitzero.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package omitzero
2+
3+
type Foo struct {
4+
EmptyStruct struct{} `json:",omitempty"` // want "Omitempty has no effect on nested struct fields"
5+
}
6+
7+
type Bar struct {
8+
NonEmptyStruct struct{ a int } `json:",omitempty"` // want "Omitempty has no effect on nested struct fields"
9+
}
10+
11+
type C struct {
12+
D string `json:",omitempty"`
13+
}
14+
15+
type R struct {
16+
M string `json:",omitempty"`
17+
}
18+
19+
type A struct {
20+
C C `json:"test,omitempty"` // want "Omitempty has no effect on nested struct fields"
21+
R R `json:"test"`
22+
}
23+
24+
type X struct {
25+
NonEmptyStruct struct{ a int } `json:",omitempty" yaml:",omitempty"` // want "Omitempty has no effect on nested struct fields"
26+
}
27+
28+
type Y struct {
29+
NonEmptyStruct struct{ a int } `yaml:",omitempty" json:",omitempty"` // want "Omitempty has no effect on nested struct fields"
30+
}

‎gopls/internal/analysis/modernize/testdata/src/omitzero/omitzero.go.golden

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
-- Replace omitempty with omitzero (behavior change) --
2+
package omitzero
3+
4+
type Foo struct {
5+
EmptyStruct struct{} `json:",omitzero"` // want "Omitempty has no effect on nested struct fields"
6+
}
7+
8+
type Bar struct {
9+
NonEmptyStruct struct{ a int } `json:",omitzero"` // want "Omitempty has no effect on nested struct fields"
10+
}
11+
12+
type C struct {
13+
D string `json:",omitempty"`
14+
}
15+
16+
type R struct {
17+
M string `json:",omitempty"`
18+
}
19+
20+
type A struct {
21+
C C `json:"test,omitzero"` // want "Omitempty has no effect on nested struct fields"
22+
R R `json:"test"`
23+
}
24+
25+
type X struct {
26+
NonEmptyStruct struct{ a int } `json:",omitzero" yaml:",omitempty"` // want "Omitempty has no effect on nested struct fields"
27+
}
28+
29+
type Y struct {
30+
NonEmptyStruct struct{ a int } `yaml:",omitempty" json:",omitzero"` // want "Omitempty has no effect on nested struct fields"
31+
}
32+
33+
-- Remove redundant omitempty tag --
34+
package omitzero
35+
36+
type Foo struct {
37+
EmptyStruct struct{} // want "Omitempty has no effect on nested struct fields"
38+
}
39+
40+
type Bar struct {
41+
NonEmptyStruct struct{ a int } // want "Omitempty has no effect on nested struct fields"
42+
}
43+
44+
type C struct {
45+
D string `json:",omitempty"`
46+
}
47+
48+
type R struct {
49+
M string `json:",omitempty"`
50+
}
51+
52+
type A struct {
53+
C C `json:"test"` // want "Omitempty has no effect on nested struct fields"
54+
R R `json:"test"`
55+
}
56+
57+
type X struct {
58+
NonEmptyStruct struct{ a int } `yaml:",omitempty"` // want "Omitempty has no effect on nested struct fields"
59+
}
60+
61+
type Y struct {
62+
NonEmptyStruct struct{ a int } `yaml:",omitempty"` // want "Omitempty has no effect on nested struct fields"
63+
}

‎gopls/internal/doc/api.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@
472472
},
473473
{
474474
"Name": "\"modernize\"",
475-
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;",
475+
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go 1.24",
476476
"Default": "true"
477477
},
478478
{
@@ -1129,7 +1129,7 @@
11291129
},
11301130
{
11311131
"Name": "modernize",
1132-
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;",
1132+
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go 1.24",
11331133
"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize",
11341134
"Default": true
11351135
},

‎gopls/internal/golang/highlight.go

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ import (
1212
"go/types"
1313
"strconv"
1414
"strings"
15-
"unicode/utf8"
1615

17-
"golang.org/x/tools/go/ast/astutil"
16+
astutil "golang.org/x/tools/go/ast/astutil"
1817
"golang.org/x/tools/gopls/internal/cache"
1918
"golang.org/x/tools/gopls/internal/file"
2019
"golang.org/x/tools/gopls/internal/protocol"
20+
internalastutil "golang.org/x/tools/internal/astutil"
2121
"golang.org/x/tools/internal/event"
2222
"golang.org/x/tools/internal/fmtstr"
2323
)
@@ -210,11 +210,11 @@ func highlightPrintf(call *ast.CallExpr, idx int, cursorPos token.Pos, lit *ast.
210210

211211
// highlightPair highlights the operation and its potential argument pair if the cursor is within either range.
212212
highlightPair := func(rang fmtstr.Range, argIndex int) {
213-
rangeStart, err := posInStringLiteral(lit, rang.Start)
213+
rangeStart, err := internalastutil.PosInStringLiteral(lit, rang.Start)
214214
if err != nil {
215215
return
216216
}
217-
rangeEnd, err := posInStringLiteral(lit, rang.End)
217+
rangeEnd, err := internalastutil.PosInStringLiteral(lit, rang.End)
218218
if err != nil {
219219
return
220220
}
@@ -277,38 +277,6 @@ func highlightPrintf(call *ast.CallExpr, idx int, cursorPos token.Pos, lit *ast.
277277
}
278278
}
279279

280-
// posInStringLiteral returns the position within a string literal
281-
// corresponding to the specified byte offset within the logical
282-
// string that it denotes.
283-
func posInStringLiteral(lit *ast.BasicLit, offset int) (token.Pos, error) {
284-
raw := lit.Value
285-
286-
value, err := strconv.Unquote(raw)
287-
if err != nil {
288-
return 0, err
289-
}
290-
if !(0 <= offset && offset <= len(value)) {
291-
return 0, fmt.Errorf("invalid offset")
292-
}
293-
294-
// remove quotes
295-
quote := raw[0] // '"' or '`'
296-
raw = raw[1 : len(raw)-1]
297-
298-
var (
299-
i = 0 // byte index within logical value
300-
pos = lit.ValuePos + 1 // position within literal
301-
)
302-
for raw != "" && i < offset {
303-
r, _, rest, _ := strconv.UnquoteChar(raw, quote) // can't fail
304-
sz := len(raw) - len(rest) // length of literal char in raw bytes
305-
pos += token.Pos(sz)
306-
raw = raw[sz:]
307-
i += utf8.RuneLen(r)
308-
}
309-
return pos, nil
310-
}
311-
312280
type posRange struct {
313281
start, end token.Pos
314282
}

‎internal/astutil/util.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package astutil
6+
7+
import (
8+
"fmt"
9+
"go/ast"
10+
"go/token"
11+
"strconv"
12+
"unicode/utf8"
13+
)
14+
15+
// PosInStringLiteral returns the position within a string literal
16+
// corresponding to the specified byte offset within the logical
17+
// string that it denotes.
18+
func PosInStringLiteral(lit *ast.BasicLit, offset int) (token.Pos, error) {
19+
raw := lit.Value
20+
21+
value, err := strconv.Unquote(raw)
22+
if err != nil {
23+
return 0, err
24+
}
25+
if !(0 <= offset && offset <= len(value)) {
26+
return 0, fmt.Errorf("invalid offset")
27+
}
28+
29+
// remove quotes
30+
quote := raw[0] // '"' or '`'
31+
raw = raw[1 : len(raw)-1]
32+
33+
var (
34+
i = 0 // byte index within logical value
35+
pos = lit.ValuePos + 1 // position within literal
36+
)
37+
for raw != "" && i < offset {
38+
r, _, rest, _ := strconv.UnquoteChar(raw, quote) // can't fail
39+
sz := len(raw) - len(rest) // length of literal char in raw bytes
40+
pos += token.Pos(sz)
41+
raw = raw[sz:]
42+
i += utf8.RuneLen(r)
43+
}
44+
return pos, nil
45+
}

0 commit comments

Comments
 (0)