Skip to content

Commit 4b2fd2a

Browse files
adonovangopherbot
authored andcommitted
gopls/internal/analysis/modernize: pass to use go1.26 new(x)
This CL adds a new analyzer to modernize declarations of and calls to functions of this form: func varOf(x int) *int { return &x } ... = varOf(123) so that they are transformed to: func varOf(x int) *int { return new(x) } ... = new(123) (Such functions are widely used in serialization packages, for instance the proto.{Int64,String,Bool} helpers used with protobufs.) In earlier drafts we also added //go:fix inline to the varOf function, but this isn't safe until we fix golang/go#75726. Also: - factor analysisinternal.EnclosingScope out of gopls' Rename. - fix a trivial panic recently introduced to stringscutprefix. + test, doc Updates golang/go#45624 Change-Id: Ic5fa36515c7da4377c01f7e155c622fca992adda Reviewed-on: https://go-review.googlesource.com/c/tools/+/704820 Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Madeline Kalil <mkalil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Alan Donovan <adonovan@google.com>
1 parent 2e5e03c commit 4b2fd2a

File tree

16 files changed

+394
-31
lines changed

16 files changed

+394
-31
lines changed

go/analysis/passes/modernize/any.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,12 @@ func runAny(pass *analysis.Pass) (any, error) {
3232
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
3333

3434
for curFile := range filesUsing(inspect, pass.TypesInfo, "go1.18") {
35-
file := curFile.Node().(*ast.File)
36-
3735
for curIface := range curFile.Preorder((*ast.InterfaceType)(nil)) {
3836
iface := curIface.Node().(*ast.InterfaceType)
3937

4038
if iface.Methods.NumFields() == 0 {
4139
// Check that 'any' is not shadowed.
42-
// TODO(adonovan): find scope using only local Cursor operations.
43-
scope := pass.TypesInfo.Scopes[file].Innermost(iface.Pos())
44-
if _, obj := scope.LookupParent("any", iface.Pos()); obj == builtinAny {
40+
if lookup(pass.TypesInfo, curIface, "any") == builtinAny {
4541
pass.Report(analysis.Diagnostic{
4642
Pos: iface.Pos(),
4743
End: iface.End(),

go/analysis/passes/modernize/doc.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,32 @@ This analyzer avoids making suggestions for floating-point types,
143143
as the behavior of `min` and `max` with NaN values can differ from
144144
the original if/else statement.
145145
146+
# Analyzer newexpr
147+
148+
newexpr: simplify code by using go1.26's new(expr)
149+
150+
This analyzer finds declarations of functions of this form:
151+
152+
func varOf(x int) *int { return &x }
153+
154+
and suggests a fix to turn them into inlinable wrappers around
155+
go1.26's built-in new(expr) function:
156+
157+
func varOf(x int) *int { return new(x) }
158+
159+
In addition, this analyzer suggests a fix for each call
160+
to one of the functions before it is transformed, so that
161+
162+
use(varOf(123))
163+
164+
is replaced by:
165+
166+
use(new(123))
167+
168+
(Wrapper functions such as varOf are common when working with Go
169+
serialization packages such as for JSON or protobuf, where pointers
170+
are often used to express optionality.)
171+
146172
# Analyzer omitzero
147173
148174
omitzero: suggest replacing omitempty with omitzero for struct fields

go/analysis/passes/modernize/minmax.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ func minmax(pass *analysis.Pass) (any, error) {
7070
b = compare.Y
7171
lhs = tassign.Lhs[0]
7272
rhs = tassign.Rhs[0]
73-
scope = pass.TypesInfo.Scopes[ifStmt.Body]
7473
sign = isInequality(compare.Op)
7574

7675
// callArg formats a call argument, preserving comments from [start-end).
@@ -104,7 +103,7 @@ func minmax(pass *analysis.Pass) (any, error) {
104103

105104
sym := cond(sign < 0, "min", "max")
106105

107-
if _, obj := scope.LookupParent(sym, ifStmt.Pos()); !is[*types.Builtin](obj) {
106+
if !is[*types.Builtin](lookup(pass.TypesInfo, curIfStmt, sym)) {
108107
return // min/max function is shadowed
109108
}
110109

@@ -160,7 +159,7 @@ func minmax(pass *analysis.Pass) (any, error) {
160159
}
161160
sym := cond(sign < 0, "min", "max")
162161

163-
if _, obj := scope.LookupParent(sym, ifStmt.Pos()); !is[*types.Builtin](obj) {
162+
if !is[*types.Builtin](lookup(pass.TypesInfo, curIfStmt, sym)) {
164163
return // min/max function is shadowed
165164
}
166165

go/analysis/passes/modernize/modernize.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ var Suite = []*analysis.Analyzer{
3838
ForVarAnalyzer,
3939
MapsLoopAnalyzer,
4040
MinMaxAnalyzer,
41+
NewExprAnalyzer,
4142
OmitZeroAnalyzer,
4243
RangeIntAnalyzer,
4344
ReflectTypeForAnalyzer,
@@ -163,6 +164,7 @@ var (
163164
builtinFalse = types.Universe.Lookup("false")
164165
builtinLen = types.Universe.Lookup("len")
165166
builtinMake = types.Universe.Lookup("make")
167+
builtinNew = types.Universe.Lookup("new")
166168
builtinNil = types.Universe.Lookup("nil")
167169
builtinString = types.Universe.Lookup("string")
168170
builtinTrue = types.Universe.Lookup("true")
@@ -207,3 +209,10 @@ func noEffects(info *types.Info, expr ast.Expr) bool {
207209
})
208210
return noEffects
209211
}
212+
213+
// lookup returns the symbol denoted by name at the position of the cursor.
214+
func lookup(info *types.Info, cur inspector.Cursor, name string) types.Object {
215+
scope := analysisinternal.EnclosingScope(info, cur)
216+
_, obj := scope.LookupParent(name, cur.Node().Pos())
217+
return obj
218+
}

go/analysis/passes/modernize/modernize_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ func TestMinMax(t *testing.T) {
3939
RunWithSuggestedFixes(t, TestData(), modernize.MinMaxAnalyzer, "minmax", "minmax/userdefined", "minmax/wrongoperators", "minmax/nonstrict", "minmax/wrongreturn")
4040
}
4141

42+
func TestNewExpr(t *testing.T) {
43+
RunWithSuggestedFixes(t, TestData(), modernize.NewExprAnalyzer, "newexpr")
44+
}
45+
4246
func TestOmitZero(t *testing.T) {
4347
RunWithSuggestedFixes(t, TestData(), modernize.OmitZeroAnalyzer, "omitzero")
4448
}
Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
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+
_ "embed"
9+
"go/ast"
10+
"go/token"
11+
"go/types"
12+
"strings"
13+
14+
"fmt"
15+
16+
"golang.org/x/tools/go/analysis"
17+
"golang.org/x/tools/go/analysis/passes/inspect"
18+
"golang.org/x/tools/go/ast/inspector"
19+
"golang.org/x/tools/go/types/typeutil"
20+
"golang.org/x/tools/internal/analysisinternal"
21+
)
22+
23+
var NewExprAnalyzer = &analysis.Analyzer{
24+
Name: "newexpr",
25+
Doc: analysisinternal.MustExtractDoc(doc, "newexpr"),
26+
URL: "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize#newexpr",
27+
Requires: []*analysis.Analyzer{inspect.Analyzer},
28+
Run: run,
29+
FactTypes: []analysis.Fact{&newLike{}},
30+
}
31+
32+
func run(pass *analysis.Pass) (any, error) {
33+
var (
34+
inspect = pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
35+
info = pass.TypesInfo
36+
)
37+
38+
// Detect functions that are new-like, i.e. have the form:
39+
//
40+
// func f(x T) *T { return &x }
41+
//
42+
// meaning that it is equivalent to new(x), if x has type T.
43+
for curFuncDecl := range inspect.Root().Preorder((*ast.FuncDecl)(nil)) {
44+
decl := curFuncDecl.Node().(*ast.FuncDecl)
45+
fn := info.Defs[decl.Name].(*types.Func)
46+
if decl.Body != nil && len(decl.Body.List) == 1 {
47+
if ret, ok := decl.Body.List[0].(*ast.ReturnStmt); ok && len(ret.Results) == 1 {
48+
if unary, ok := ret.Results[0].(*ast.UnaryExpr); ok && unary.Op == token.AND {
49+
if id, ok := unary.X.(*ast.Ident); ok {
50+
if v, ok := info.Uses[id].(*types.Var); ok {
51+
sig := fn.Signature()
52+
if sig.Results().Len() == 1 &&
53+
is[*types.Pointer](sig.Results().At(0).Type()) && // => no iface conversion
54+
sig.Params().Len() == 1 &&
55+
sig.Params().At(0) == v {
56+
57+
// Export a fact for each one.
58+
pass.ExportObjectFact(fn, &newLike{})
59+
60+
// Check file version.
61+
file := enclosingFile(curFuncDecl)
62+
if !fileUses(info, file, "go1.26") {
63+
continue // new(expr) not available in this file
64+
}
65+
66+
var edits []analysis.TextEdit
67+
68+
// If 'new' is not shadowed, replace func body: &x -> new(x).
69+
// This makes it safely and cleanly inlinable.
70+
curRet, _ := curFuncDecl.FindNode(ret)
71+
if lookup(info, curRet, "new") == builtinNew {
72+
edits = []analysis.TextEdit{
73+
// return &x
74+
// ---- -
75+
// return new(x)
76+
{
77+
Pos: unary.OpPos,
78+
End: unary.OpPos + token.Pos(len("&")),
79+
NewText: []byte("new("),
80+
},
81+
{
82+
Pos: unary.X.End(),
83+
End: unary.X.End(),
84+
NewText: []byte(")"),
85+
},
86+
}
87+
}
88+
89+
// Disabled until we resolve https://go.dev/issue/75726
90+
// (Go version skew between caller and callee in inliner.)
91+
// TODO(adonovan): fix and reenable.
92+
//
93+
// Also, restore these lines to our section of doc.go:
94+
// //go:fix inline
95+
// ...
96+
// (The directive comment causes the inline analyzer to suggest
97+
// that calls to such functions are inlined.)
98+
if false {
99+
// Add a //go:fix inline annotation, if not already present.
100+
// TODO(adonovan): use ast.ParseDirective when go1.26 is assured.
101+
if !strings.Contains(decl.Doc.Text(), "go:fix inline") {
102+
edits = append(edits, analysis.TextEdit{
103+
Pos: decl.Pos(),
104+
End: decl.Pos(),
105+
NewText: []byte("//go:fix inline\n"),
106+
})
107+
}
108+
}
109+
110+
if len(edits) > 0 {
111+
pass.Report(analysis.Diagnostic{
112+
Pos: decl.Name.Pos(),
113+
End: decl.Name.End(),
114+
Message: fmt.Sprintf("%s can be an inlinable wrapper around new(expr)", decl.Name),
115+
SuggestedFixes: []analysis.SuggestedFix{
116+
{
117+
Message: "Make %s an inlinable wrapper around new(expr)",
118+
TextEdits: edits,
119+
},
120+
},
121+
})
122+
}
123+
}
124+
}
125+
}
126+
}
127+
}
128+
}
129+
}
130+
131+
// Report and transform calls, when safe.
132+
// In effect, this is inlining the new-like function
133+
// even before we have marked the callee with //go:fix inline.
134+
for curCall := range inspect.Root().Preorder((*ast.CallExpr)(nil)) {
135+
call := curCall.Node().(*ast.CallExpr)
136+
var fact newLike
137+
if fn, ok := typeutil.Callee(info, call).(*types.Func); ok &&
138+
pass.ImportObjectFact(fn, &fact) {
139+
140+
// Check file version.
141+
file := enclosingFile(curCall)
142+
if !fileUses(info, file, "go1.26") {
143+
continue // new(expr) not available in this file
144+
}
145+
146+
// Check new is not shadowed.
147+
if lookup(info, curCall, "new") != builtinNew {
148+
continue
149+
}
150+
151+
// The return type *T must exactly match the argument type T.
152+
// (We formulate it this way--not in terms of the parameter
153+
// type--to support generics.)
154+
var targ types.Type
155+
{
156+
arg := call.Args[0]
157+
tvarg := info.Types[arg]
158+
159+
// Constants: we must work around the type checker
160+
// bug that causes info.Types to wrongly report the
161+
// "typed" type for an untyped constant.
162+
// (See "historical reasons" in issue go.dev/issue/70638.)
163+
//
164+
// We don't have a reliable way to do this but we can attempt
165+
// to re-typecheck the constant expression on its own, in
166+
// the original lexical environment but not as a part of some
167+
// larger expression that implies a conversion to some "typed" type.
168+
// (For the genesis of this idea see (*state).arguments
169+
// in ../../../../internal/refactor/inline/inline.go.)
170+
if tvarg.Value != nil {
171+
info2 := &types.Info{Types: make(map[ast.Expr]types.TypeAndValue)}
172+
if err := types.CheckExpr(token.NewFileSet(), pass.Pkg, token.NoPos, arg, info2); err != nil {
173+
continue // unexpected error
174+
}
175+
tvarg = info2.Types[arg]
176+
}
177+
178+
targ = types.Default(tvarg.Type)
179+
}
180+
if !types.Identical(types.NewPointer(targ), info.TypeOf(call)) {
181+
continue
182+
}
183+
184+
pass.Report(analysis.Diagnostic{
185+
Pos: call.Pos(),
186+
End: call.End(),
187+
Message: fmt.Sprintf("call of %s(x) can be simplified to new(x)", fn.Name()),
188+
SuggestedFixes: []analysis.SuggestedFix{{
189+
Message: fmt.Sprintf("Simplify %s(x) to new(x)", fn.Name()),
190+
TextEdits: []analysis.TextEdit{{
191+
Pos: call.Fun.Pos(),
192+
End: call.Fun.End(),
193+
NewText: []byte("new"),
194+
}},
195+
}},
196+
})
197+
}
198+
}
199+
200+
return nil, nil
201+
}
202+
203+
// A newLike fact records that its associated function is "new-like".
204+
type newLike struct{}
205+
206+
func (*newLike) AFact() {}
207+
func (*newLike) String() string { return "newlike" }

go/analysis/passes/modernize/stringscutprefix.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ func stringscutprefix(pass *analysis.Pass) (any, error) {
184184
lhs := assign.Lhs[0]
185185
obj := typeutil.Callee(info, call)
186186

187-
if obj != stringsTrimPrefix && obj != bytesTrimPrefix && obj != stringsTrimSuffix && obj != bytesTrimSuffix {
187+
if obj == nil ||
188+
obj != stringsTrimPrefix && obj != bytesTrimPrefix && obj != stringsTrimSuffix && obj != bytesTrimSuffix {
188189
continue
189190
}
190191

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//go:build go1.26
2+
3+
package newexpr
4+
5+
// intVar returns a new var whose value is i.
6+
func intVar(i int) *int { return &i } // want `intVar can be an inlinable wrapper around new\(expr\)` intVar:"newlike"
7+
8+
func int64Var(i int64) *int64 { return &i } // want `int64Var can be an inlinable wrapper around new\(expr\)` int64Var:"newlike"
9+
10+
func stringVar(s string) *string { return &s } // want `stringVar can be an inlinable wrapper around new\(expr\)` stringVar:"newlike"
11+
12+
func varOf[T any](x T) *T { return &x } // want `varOf can be an inlinable wrapper around new\(expr\)` varOf:"newlike"
13+
14+
var (
15+
s struct {
16+
int
17+
string
18+
}
19+
_ = intVar(123) // want `call of intVar\(x\) can be simplified to new\(x\)`
20+
_ = int64Var(123) // nope: implicit conversion from untyped int to int64
21+
_ = stringVar("abc") // want `call of stringVar\(x\) can be simplified to new\(x\)`
22+
_ = varOf(s) // want `call of varOf\(x\) can be simplified to new\(x\)`
23+
_ = varOf(123) // want `call of varOf\(x\) can be simplified to new\(x\)`
24+
_ = varOf(int64(123)) // want `call of varOf\(x\) can be simplified to new\(x\)`
25+
_ = varOf[int](123) // want `call of varOf\(x\) can be simplified to new\(x\)`
26+
_ = varOf[int64](123) // nope: implicit conversion from untyped int to int64
27+
_ = varOf( // want `call of varOf\(x\) can be simplified to new\(x\)`
28+
varOf(123)) // want `call of varOf\(x\) can be simplified to new\(x\)`
29+
)
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//go:build go1.26
2+
3+
package newexpr
4+
5+
// intVar returns a new var whose value is i.
6+
func intVar(i int) *int { return new(i) } // want `intVar can be an inlinable wrapper around new\(expr\)` intVar:"newlike"
7+
8+
func int64Var(i int64) *int64 { return new(i) } // want `int64Var can be an inlinable wrapper around new\(expr\)` int64Var:"newlike"
9+
10+
func stringVar(s string) *string { return new(s) } // want `stringVar can be an inlinable wrapper around new\(expr\)` stringVar:"newlike"
11+
12+
func varOf[T any](x T) *T { return new(x) } // want `varOf can be an inlinable wrapper around new\(expr\)` varOf:"newlike"
13+
14+
var (
15+
s struct {
16+
int
17+
string
18+
}
19+
_ = new(123) // want `call of intVar\(x\) can be simplified to new\(x\)`
20+
_ = int64Var(123) // nope: implicit conversion from untyped int to int64
21+
_ = new("abc") // want `call of stringVar\(x\) can be simplified to new\(x\)`
22+
_ = new(s) // want `call of varOf\(x\) can be simplified to new\(x\)`
23+
_ = new(123) // want `call of varOf\(x\) can be simplified to new\(x\)`
24+
_ = new(int64(123)) // want `call of varOf\(x\) can be simplified to new\(x\)`
25+
_ = new(123) // want `call of varOf\(x\) can be simplified to new\(x\)`
26+
_ = varOf[int64](123) // nope: implicit conversion from untyped int to int64
27+
_ = new( // want `call of varOf\(x\) can be simplified to new\(x\)`
28+
new(123)) // want `call of varOf\(x\) can be simplified to new\(x\)`
29+
)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package newexpr

0 commit comments

Comments
 (0)