golang.org/x/sys v0.38.1-0.20251125153526-08e54827f670
golang.org/x/telemetry v0.0.0-20251128220624-abf20d0e57ec
golang.org/x/term v0.37.0
- golang.org/x/tools v0.39.1-0.20251130212600-1ad6f3d02713
+ golang.org/x/tools v0.39.1-0.20251205000126-062ef7b6ced2
)
require (
golang.org/x/term v0.37.0/go.mod h1:5pB4lxRNYYVZuTLmy8oR2BH8dflOR+IbTYFD8fi3254=
golang.org/x/text v0.31.1-0.20251128220601-087616b6cde9 h1:IjQf87/qLz2y0SiCc0uY3DwajALXkAgP1Pxal0mmdrM=
golang.org/x/text v0.31.1-0.20251128220601-087616b6cde9/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM=
-golang.org/x/tools v0.39.1-0.20251130212600-1ad6f3d02713 h1:i4GzAuZW4RuKXltwKyLYAfk7E1TSKQBxRAI7XKfLjSk=
-golang.org/x/tools v0.39.1-0.20251130212600-1ad6f3d02713/go.mod h1:JnefbkDPyD8UU2kI5fuf8ZX4/yUeh9W877ZeBONxUqQ=
+golang.org/x/tools v0.39.1-0.20251205000126-062ef7b6ced2 h1:2Qqv605Nus9iUp3ErvEU/q92Q3HAzeROztzl9pzAno8=
+golang.org/x/tools v0.39.1-0.20251205000126-062ef7b6ced2/go.mod h1:JnefbkDPyD8UU2kI5fuf8ZX4/yUeh9W877ZeBONxUqQ=
rsc.io/markdown v0.0.0-20240306144322-0bf8f97ee8ef h1:mqLYrXCXYEZOop9/Dbo6RPX11539nwiCNBb1icVPmw8=
rsc.io/markdown v0.0.0-20240306144322-0bf8f97ee8ef/go.mod h1:8xcPgWmwlZONN1D9bjxtHEjrUtSEa3fakVF8iaewYKQ=
omitzero: suggest replacing omitempty with omitzero for struct fields
-The omitzero analyzer identifies uses of the `omitempty` JSON struct tag on
-fields that are themselves structs. The `omitempty` tag has no effect on
-struct-typed fields. The analyzer offers two suggestions: either remove the
+The omitzero analyzer identifies uses of the `omitempty` JSON struct
+tag on fields that are themselves structs. For struct-typed fields,
+the `omitempty` tag has no effect on the behavior of json.Marshal and
+json.Unmarshal. The analyzer offers two suggestions: either remove the
tag, or replace it with `omitzero` (added in Go 1.24), which correctly
omits the field if the struct value is zero.
+However, some other serialization packages (notably kubebuilder, see
+https://book.kubebuilder.io/reference/markers.html) may have their own
+interpretation of the `json:",omitzero"` tag, so removing it may affect
+program behavior. For this reason, the omitzero modernizer will not
+make changes in any package that contains +kubebuilder annotations.
+
Replacing `omitempty` with `omitzero` is a change in behavior. The
original code would always encode the struct field, whereas the
modified code will omit it if it is a zero-value.
"go/types"
"reflect"
"strconv"
+ "strings"
+ "sync"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#omitzero",
}
-func checkOmitEmptyField(pass *analysis.Pass, info *types.Info, curField *ast.Field) {
- typ := info.TypeOf(curField.Type)
- _, ok := typ.Underlying().(*types.Struct)
- if !ok {
- // Not a struct
- return
- }
- tag := curField.Tag
- if tag == nil {
- // No tag to check
- return
- }
- // The omitempty tag may be used by other packages besides json, but we should only modify its use with json
- tagconv, _ := strconv.Unquote(tag.Value)
- match := omitemptyRegex.FindStringSubmatchIndex(tagconv)
- if match == nil {
- // No omitempty in json tag
- return
- }
- omitEmpty, err := astutil.RangeInStringLiteral(curField.Tag, match[2], match[3])
- if err != nil {
- return
- }
- var remove analysis.Range = omitEmpty
+// The omitzero pass searches for instances of "omitempty" in a json field tag on a
+// struct. Since "omitfilesUsingGoVersions not have any effect when applied to a struct field,
+// it suggests either deleting "omitempty" or replacing it with "omitzero", which
+// correctly excludes structs from a json encoding.
+func omitzero(pass *analysis.Pass) (any, error) {
+ // usesKubebuilder reports whether "+kubebuilder:" appears in
+ // any comment in the package, since it has its own
+ // interpretation of what omitzero means; see go.dev/issue/76649.
+ // It is computed once, on demand.
+ usesKubebuilder := sync.OnceValue[bool](func() bool {
+ for _, file := range pass.Files {
+ for _, comment := range file.Comments {
+ if strings.Contains(comment.Text(), "+kubebuilder:") {
+ return true
+ }
+ }
+ }
+ return false
+ })
+
+ checkField := func(field *ast.Field) {
+ typ := pass.TypesInfo.TypeOf(field.Type)
+ _, ok := typ.Underlying().(*types.Struct)
+ if !ok {
+ // Not a struct
+ return
+ }
+ tag := field.Tag
+ if tag == nil {
+ // No tag to check
+ return
+ }
+ // The omitempty tag may be used by other packages besides json, but we should only modify its use with json
+ tagconv, _ := strconv.Unquote(tag.Value)
+ match := omitemptyRegex.FindStringSubmatchIndex(tagconv)
+ if match == nil {
+ // No omitempty in json tag
+ return
+ }
+ omitEmpty, err := astutil.RangeInStringLiteral(field.Tag, match[2], match[3])
+ if err != nil {
+ return
+ }
+ var remove analysis.Range = omitEmpty
- jsonTag := reflect.StructTag(tagconv).Get("json")
- if jsonTag == ",omitempty" {
- // Remove the entire struct tag if json is the only package used
- if match[1]-match[0] == len(tagconv) {
- remove = curField.Tag
- } else {
- // Remove the json tag if omitempty is the only field
- remove, err = astutil.RangeInStringLiteral(curField.Tag, match[0], match[1])
- if err != nil {
- return
+ jsonTag := reflect.StructTag(tagconv).Get("json")
+ if jsonTag == ",omitempty" {
+ // Remove the entire struct tag if json is the only package used
+ if match[1]-match[0] == len(tagconv) {
+ remove = field.Tag
+ } else {
+ // Remove the json tag if omitempty is the only field
+ remove, err = astutil.RangeInStringLiteral(field.Tag, match[0], match[1])
+ if err != nil {
+ return
+ }
}
}
- }
- pass.Report(analysis.Diagnostic{
- Pos: curField.Tag.Pos(),
- End: curField.Tag.End(),
- Message: "Omitempty has no effect on nested struct fields",
- SuggestedFixes: []analysis.SuggestedFix{
- {
- Message: "Remove redundant omitempty tag",
- TextEdits: []analysis.TextEdit{
- {
- Pos: remove.Pos(),
- End: remove.End(),
+
+ // Don't offer a fix if the package seems to use kubebuilder,
+ // as it has its own intepretation of "omitzero" tags.
+ // https://book.kubebuilder.io/reference/markers.html
+ if usesKubebuilder() {
+ return
+ }
+
+ pass.Report(analysis.Diagnostic{
+ Pos: field.Tag.Pos(),
+ End: field.Tag.End(),
+ Message: "Omitempty has no effect on nested struct fields",
+ SuggestedFixes: []analysis.SuggestedFix{
+ {
+ Message: "Remove redundant omitempty tag",
+ TextEdits: []analysis.TextEdit{
+ {
+ Pos: remove.Pos(),
+ End: remove.End(),
+ },
},
},
- },
- {
- Message: "Replace omitempty with omitzero (behavior change)",
- TextEdits: []analysis.TextEdit{
- {
- Pos: omitEmpty.Pos(),
- End: omitEmpty.End(),
- NewText: []byte(",omitzero"),
+ {
+ Message: "Replace omitempty with omitzero (behavior change)",
+ TextEdits: []analysis.TextEdit{
+ {
+ Pos: omitEmpty.Pos(),
+ End: omitEmpty.End(),
+ NewText: []byte(",omitzero"),
+ },
},
},
- },
- }})
-}
+ }})
+ }
-// The omitzero pass searches for instances of "omitempty" in a json field tag on a
-// struct. Since "omitfilesUsingGoVersions not have any effect when applied to a struct field,
-// it suggests either deleting "omitempty" or replacing it with "omitzero", which
-// correctly excludes structs from a json encoding.
-func omitzero(pass *analysis.Pass) (any, error) {
for curFile := range filesUsingGoVersion(pass, versions.Go1_24) {
for curStruct := range curFile.Preorder((*ast.StructType)(nil)) {
for _, curField := range curStruct.Node().(*ast.StructType).Fields.List {
- checkOmitEmptyField(pass, pass.TypesInfo, curField)
+ checkField(curField)
}
}
}
+
return nil, nil
}
// The following must hold for a replacement to occur:
//
// 1. All instances of i and s must be in one of these forms.
-// Binary expressions:
-// (a): establishing that i < 0: e.g.: i < 0, 0 > i, i == -1, -1 == i
-// (b): establishing that i > -1: e.g.: i >= 0, 0 <= i, i == 0, 0 == i
+//
+// Binary expressions must be inequalities equivalent to
+// "Index failed" (e.g. i < 0) or "Index succeeded" (i >= 0),
+// or identities such as these (and their negations):
+//
+// 0 > i (flips left and right)
+// i <= -1, -1 >= i (replace strict inequality by non-strict)
+// i == -1, -1 == i (Index() guarantees i < 0 => i == -1)
//
// Slice expressions:
// a: s[:i], s[0:i]
// use(before, after)
// }
//
-// If the condition involving `i` establishes that i > -1, then we replace it with
-// `if ok“. Variants listed above include i >= 0, i > 0, and i == 0.
-// If the condition is negated (e.g. establishes `i < 0`), we use `if !ok` instead.
+// If the condition involving `i` is equivalent to i >= 0, then we replace it with
+// `if ok“.
+// If the condition is negated (e.g. equivalent to `i < 0`), we use `if !ok` instead.
// If the slices of `s` match `s[:i]` or `s[i+len(substr):]` or their variants listed above,
// then we replace them with before and after.
//
// len(substr)]), then we can replace the call to Index()
// with a call to Cut() and use the returned ok, before,
// and after variables accordingly.
- lessZero, greaterNegOne, beforeSlice, afterSlice := checkIdxUses(pass.TypesInfo, index.Uses(iObj), s, substr)
+ negative, nonnegative, beforeSlice, afterSlice := checkIdxUses(pass.TypesInfo, index.Uses(iObj), s, substr)
// Either there are no uses of before, after, or ok, or some use
// of i does not match our criteria - don't suggest a fix.
- if lessZero == nil && greaterNegOne == nil && beforeSlice == nil && afterSlice == nil {
+ if negative == nil && nonnegative == nil && beforeSlice == nil && afterSlice == nil {
continue
}
// If the only uses are ok and !ok, don't suggest a Cut() fix - these should be using Contains()
- isContains := (len(lessZero) > 0 || len(greaterNegOne) > 0) && len(beforeSlice) == 0 && len(afterSlice) == 0
+ isContains := (len(negative) > 0 || len(nonnegative) > 0) && len(beforeSlice) == 0 && len(afterSlice) == 0
scope := iObj.Parent()
var (
// If there will be no uses of ok, before, or after, use the
// blank identifier instead.
- if len(lessZero) == 0 && len(greaterNegOne) == 0 {
+ if len(negative) == 0 && len(nonnegative) == 0 {
okVarName = "_"
}
if len(beforeSlice) == 0 {
replacedFunc := "Cut"
if isContains {
replacedFunc = "Contains"
- replace(lessZero, "!"+foundVarName) // idx < 0 -> !found
- replace(greaterNegOne, foundVarName) // idx > -1 -> found
+ replace(negative, "!"+foundVarName) // idx < 0 -> !found
+ replace(nonnegative, foundVarName) // idx > -1 -> found
// Replace the assignment with found, and replace the call to
// Index or IndexByte with a call to Contains.
NewText: []byte("Contains"),
})
} else {
- replace(lessZero, "!"+okVarName) // idx < 0 -> !ok
- replace(greaterNegOne, okVarName) // idx > -1 -> ok
+ replace(negative, "!"+okVarName) // idx < 0 -> !ok
+ replace(nonnegative, okVarName) // idx > -1 -> ok
replace(beforeSlice, beforeVarName) // s[:idx] -> before
replace(afterSlice, afterVarName) // s[idx+k:] -> after
// one of the following four valid formats, it returns a list of occurrences for
// each format. If any of the uses do not match one of the formats, return nil
// for all values, since we should not offer a replacement.
-// 1. lessZero - a condition involving i establishing that i is negative (e.g. i < 0, 0 > i, i == -1, -1 == i)
-// 2. greaterNegOne - a condition involving i establishing that i is non-negative (e.g. i >= 0, 0 <= i, i == 0, 0 == i)
+// 1. negative - a condition equivalent to i < 0
+// 2. nonnegative - a condition equivalent to i >= 0
// 3. beforeSlice - a slice of `s` that matches either s[:i], s[0:i]
// 4. afterSlice - a slice of `s` that matches one of: s[i+len(substr):], s[len(substr) + i:], s[i + const], s[k + i] (where k = len(substr))
-func checkIdxUses(info *types.Info, uses iter.Seq[inspector.Cursor], s, substr ast.Expr) (lessZero, greaterNegOne, beforeSlice, afterSlice []ast.Expr) {
+func checkIdxUses(info *types.Info, uses iter.Seq[inspector.Cursor], s, substr ast.Expr) (negative, nonnegative, beforeSlice, afterSlice []ast.Expr) {
use := func(cur inspector.Cursor) bool {
ek, _ := cur.ParentEdge()
n := cur.Parent().Node()
check := n.(*ast.BinaryExpr)
switch checkIdxComparison(info, check) {
case -1:
- lessZero = append(lessZero, check)
+ negative = append(negative, check)
return true
case 1:
- greaterNegOne = append(greaterNegOne, check)
+ nonnegative = append(nonnegative, check)
return true
}
- // Check does not establish that i < 0 or i > -1.
+ // Check is not equivalent to that i < 0 or i >= 0.
// Might be part of an outer slice expression like s[i + k]
// which requires a different check.
// Check that the thing being sliced is s and that the slice
return nil, nil, nil, nil
}
}
- return lessZero, greaterNegOne, beforeSlice, afterSlice
+ return negative, nonnegative, beforeSlice, afterSlice
}
// hasModifyingUses reports whether any of the uses involve potential
return false
}
-// checkIdxComparison reports whether the check establishes that i is negative
-// or non-negative. It returns -1 in the first case, 1 in the second, and 0 if
-// we can confirm neither condition. We assume that a check passed to
-// checkIdxComparison has i as one of its operands.
+// checkIdxComparison reports whether the check is equivalent to i < 0 or its negation, or neither.
+// For equivalent to i >= 0, we only accept this exact BinaryExpr since
+// expressions like i > 0 or i >= 1 make a stronger statement about the value of i.
+// We avoid suggesting a fix in this case since it may result in an invalid
+// transformation (See golang/go#76687).
+// Since strings.Index returns exactly -1 if the substring is not found, we
+// don't need to handle expressions like i <= -3.
+// We return 0 if the expression does not match any of these options.
+// We assume that a check passed to checkIdxComparison has i as one of its operands.
func checkIdxComparison(info *types.Info, check *ast.BinaryExpr) int {
- // Check establishes that i is negative.
- // e.g.: i < 0, 0 > i, i == -1, -1 == i
- if check.Op == token.LSS && (isNegativeConst(info, check.Y) || isZeroIntConst(info, check.Y)) || //i < (0 or neg)
- check.Op == token.GTR && (isNegativeConst(info, check.X) || isZeroIntConst(info, check.X)) || // (0 or neg) > i
- check.Op == token.LEQ && (isNegativeConst(info, check.Y)) || //i <= (neg)
- check.Op == token.GEQ && (isNegativeConst(info, check.X)) || // (neg) >= i
- check.Op == token.EQL &&
- (isNegativeConst(info, check.X) || isNegativeConst(info, check.Y)) { // i == neg; neg == i
- return -1
+ // Ensure that the constant (if any) is on the right.
+ x, op, y := check.X, check.Op, check.Y
+ if info.Types[x].Value != nil {
+ x, op, y = y, flip(op), x
}
- // Check establishes that i is non-negative.
- // e.g.: i >= 0, 0 <= i, i == 0, 0 == i
- if check.Op == token.GTR && (isNonNegativeConst(info, check.Y) || isIntLiteral(info, check.Y, -1)) || // i > (non-neg or -1)
- check.Op == token.LSS && (isNonNegativeConst(info, check.X) || isIntLiteral(info, check.X, -1)) || // (non-neg or -1) < i
- check.Op == token.GEQ && isNonNegativeConst(info, check.Y) || // i >= (non-neg)
- check.Op == token.LEQ && isNonNegativeConst(info, check.X) || // (non-neg) <= i
- check.Op == token.EQL &&
- (isNonNegativeConst(info, check.X) || isNonNegativeConst(info, check.Y)) { // i == non-neg; non-neg == i
- return 1
+
+ yIsInt := func(k int64) bool {
+ return isIntLiteral(info, y, k)
}
- return 0
-}
-// isNegativeConst returns true if the expr is a const int with value < zero.
-func isNegativeConst(info *types.Info, expr ast.Expr) bool {
- if tv, ok := info.Types[expr]; ok && tv.Value != nil && tv.Value.Kind() == constant.Int {
- if v, ok := constant.Int64Val(tv.Value); ok {
- return v < 0
- }
+ if op == token.LSS && yIsInt(0) || // i < 0
+ op == token.EQL && yIsInt(-1) || // i == -1
+ op == token.LEQ && yIsInt(-1) { // i <= -1
+ return -1 // check <=> i is negative
}
- return false
+
+ if op == token.GEQ && yIsInt(0) || // i >= 0
+ op == token.NEQ && yIsInt(-1) || // i != -1
+ op == token.GTR && yIsInt(-1) { // i > -1
+ return +1 // check <=> i is non-negative
+ }
+
+ return 0 // unknown
}
-// isNonNegativeConst returns true if the expr is a const int with value >= zero.
-func isNonNegativeConst(info *types.Info, expr ast.Expr) bool {
- if tv, ok := info.Types[expr]; ok && tv.Value != nil && tv.Value.Kind() == constant.Int {
- if v, ok := constant.Int64Val(tv.Value); ok {
- return v >= 0
- }
+// flip changes the comparison token as if the operands were flipped.
+// It is defined only for == and the four inequalities.
+func flip(op token.Token) token.Token {
+ switch op {
+ case token.EQL:
+ return token.EQL // (same)
+ case token.GEQ:
+ return token.LEQ
+ case token.GTR:
+ return token.LSS
+ case token.LEQ:
+ return token.GEQ
+ case token.LSS:
+ return token.GTR
}
- return false
+ return op
}
// isBeforeSlice reports whether the SliceExpr is of the form s[:i] or s[0:i].
// printf checker.
import (
+ "archive/zip"
"encoding/gob"
"encoding/json"
"flag"
VetxOnly bool // run analysis only for facts, not diagnostics
VetxOutput string // where to write file of fact information
Stdout string // write stdout (e.g. JSON, unified diff) to this file
+ FixArchive string // write fixed files to this zip archive, if non-empty
SucceedOnTypecheckFailure bool // obsolete awful hack; see #18395 and below
}
// In VetxOnly mode, the analysis is run only for facts.
if !cfg.VetxOnly {
- code = processResults(fset, cfg.ID, results)
+ code = processResults(fset, cfg.ID, cfg.FixArchive, results)
}
os.Exit(code)
return cfg, nil
}
-func processResults(fset *token.FileSet, id string, results []result) (exit int) {
+func processResults(fset *token.FileSet, id, fixArchive string, results []result) (exit int) {
if analysisflags.Fix {
// Don't print the diagnostics,
// but apply all fixes from the root actions.
Diagnostics: res.diagnostics,
}
}
- if err := driverutil.ApplyFixes(fixActions, analysisflags.Diff, false); err != nil {
+
+ // By default, fixes overwrite the original file.
+ // With the -diff flag, print the diffs to stdout.
+ // If "go fix" provides a fix archive, we write files
+ // into it so that mutations happen after the build.
+ write := func(filename string, content []byte) error {
+ return os.WriteFile(filename, content, 0644)
+ }
+ if fixArchive != "" {
+ f, err := os.Create(fixArchive)
+ if err != nil {
+ log.Fatalf("can't create -fix archive: %v", err)
+ }
+ zw := zip.NewWriter(f)
+ zw.SetComment(id) // ignore error
+ defer func() {
+ if err := zw.Close(); err != nil {
+ log.Fatalf("closing -fix archive zip writer: %v", err)
+ }
+ if err := f.Close(); err != nil {
+ log.Fatalf("closing -fix archive file: %v", err)
+ }
+ }()
+ write = func(filename string, content []byte) error {
+ f, err := zw.Create(filename)
+ if err != nil {
+ return err
+ }
+ _, err = f.Write(content)
+ return err
+ }
+ }
+
+ if err := driverutil.ApplyFixes(fixActions, write, analysisflags.Diff, false); err != nil {
// Fail when applying fixes failed.
log.Print(err)
exit = 1
//
// If printDiff (from the -diff flag) is set, instead of updating the
// files it display the final patch composed of all the cleanly merged
-// fixes.
+// fixes. (It is tempting to factor printDiff as just a variant of
+// writeFile that is provided the old and new content, but it's hard
+// to generate a good summary that way.)
//
// TODO(adonovan): handle file-system level aliases such as symbolic
// links using robustio.FileID.
-func ApplyFixes(actions []FixAction, printDiff, verbose bool) error {
+func ApplyFixes(actions []FixAction, writeFile func(filename string, content []byte) error, printDiff, verbose bool) error {
generated := make(map[*token.File]bool)
// Select fixes to apply.
os.Stdout.WriteString(unified)
} else {
- // write
+ // write file
totalFiles++
- // TODO(adonovan): abstract the I/O.
- if err := os.WriteFile(file, final, 0644); err != nil {
+ if err := writeFile(file, final); err != nil {
log.Println(err)
- continue
+ continue // (causes ApplyFix to return an error)
}
filesUpdated++
}
paramType := paramTypeAtIndex(sig, call, i)
ifaceAssign := paramType == nil || types.IsInterface(paramType)
affectsInference := false
- if fn := typeutil.StaticCallee(info, call); fn != nil {
- if sig2 := fn.Type().(*types.Signature); sig2.Recv() == nil {
+ switch callee := typeutil.Callee(info, call).(type) {
+ case *types.Builtin:
+ // Consider this litmus test:
+ //
+ // func f(x int64) any { return max(x) }
+ // func main() { fmt.Printf("%T", f(42)) }
+ //
+ // If we lose the implicit conversion from untyped int
+ // to int64, the type inferred for the max(x) call changes,
+ // resulting in a different dynamic behavior: it prints
+ // int, not int64.
+ //
+ // Inferred result type affected:
+ // new
+ // complex, real, imag
+ // min, max
+ //
+ // Dynamic behavior change:
+ // append -- dynamic type of append([]any(nil), x)[0]
+ // delete(m, x) -- dynamic key type where m is map[any]unit
+ // panic -- dynamic type of panic value
+ //
+ // Unaffected:
+ // recover
+ // make
+ // len, cap
+ // clear
+ // close
+ // copy
+ // print, println -- only uses underlying types (?)
+ //
+ // The dynamic type cases are all covered by
+ // the ifaceAssign logic.
+ switch callee.Name() {
+ case "new", "complex", "real", "imag", "min", "max":
+ affectsInference = true
+ }
+
+ case *types.Func:
+ // Only standalone (non-method) functions have type
+ // parameters affected by the call arguments.
+ if sig2 := callee.Signature(); sig2.Recv() == nil {
originParamType := paramTypeAtIndex(sig2, call, i)
affectsInference = originParamType == nil || new(typeparams.Free).Has(originParamType)
}
golang.org/x/text/language
golang.org/x/text/transform
golang.org/x/text/unicode/norm
-# golang.org/x/tools v0.39.1-0.20251130212600-1ad6f3d02713
+# golang.org/x/tools v0.39.1-0.20251205000126-062ef7b6ced2
## explicit; go 1.24.0
golang.org/x/tools/cmd/bisect
golang.org/x/tools/cover
projects / 0xmirror / go.git / commitdiff