[tools] gopls/internal/analysis/modernize: pass to use go1.26 new(x)
9 views
Skip to first unread message
Alan Donovan (Gerrit)
Sep 19, 2025, 6:21:45 PM (12 days ago) Sep 19
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Alan Donovan has uploaded the change for review![]( "Open in Gerrit")
Commit message
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:
//go:fix inline
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.)
Also:
- factor analysisinternal.EnclosingScope out of gopls' Rename.
- fix a trivial panic recently introduced to stringscutprefix.
+ test, doc
Change-Id: Ic5fa36515c7da4377c01f7e155c622fca992adda
Change diff
diff --git a/gopls/internal/analysis/modernize/any.go b/gopls/internal/analysis/modernize/any.go
index 68bb38e..66055f6 100644
--- a/gopls/internal/analysis/modernize/any.go
+++ b/gopls/internal/analysis/modernize/any.go
@@ -32,16 +32,12 @@
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
for curFile := range filesUsing(inspect, pass.TypesInfo, "go1.18") {
- file := curFile.Node().(*ast.File)
-
for curIface := range curFile.Preorder((*ast.InterfaceType)(nil)) {
iface := curIface.Node().(*ast.InterfaceType)
if iface.Methods.NumFields() == 0 {
// Check that 'any' is not shadowed.
- // TODO(adonovan): find scope using only local Cursor operations.
- scope := pass.TypesInfo.Scopes[file].Innermost(iface.Pos())
- if _, obj := scope.LookupParent("any", iface.Pos()); obj == builtinAny {
+ if lookup(pass.TypesInfo, curIface, "any") == builtinAny {
pass.Report(analysis.Diagnostic{
Pos: iface.Pos(),
End: iface.End(),
diff --git a/gopls/internal/analysis/modernize/doc.go b/gopls/internal/analysis/modernize/doc.go
index 217b106..8281576 100644
--- a/gopls/internal/analysis/modernize/doc.go
+++ b/gopls/internal/analysis/modernize/doc.go
@@ -143,6 +143,36 @@
as the behavior of `min` and `max` with NaN values can differ from
the original if/else statement.
+# Analyzer newexpr
+
+newexpr: simplify code by using go1.26's new(expr)
+
+This analyzer finds declarations of functions of this form:
+
+ func varOf(x int) *int { return &x }
+
+and suggests a fix to turn them into inlinable wrappers around
+go1.26's built-in new(expr) function:
+
+ //go:fix inline
+ func varOf(x int) *int { return new(x) }
+
+(The directive comment causes the inline analyzer to suggest
+that calls to such functions are inlined.)
+
+In addition, this analyzer suggests a fix for each call
+to one of the functions before it is transformed, so that
+
+ use(varOf(123))
+
+is replaced by:
+
+ use(new(123))
+
+(Wrapper functions such as varOf are common when working with Go
+serialization packages such as for JSON or protobuf, where pointers
+are often used to express optionality.)
+
# Analyzer omitzero
omitzero: suggest replacing omitempty with omitzero for struct fields
diff --git a/gopls/internal/analysis/modernize/minmax.go b/gopls/internal/analysis/modernize/minmax.go
index bfc167e..76b95da 100644
--- a/gopls/internal/analysis/modernize/minmax.go
+++ b/gopls/internal/analysis/modernize/minmax.go
@@ -59,7 +59,6 @@
b = compare.Y
lhs = tassign.Lhs[0]
rhs = tassign.Rhs[0]
- scope = pass.TypesInfo.Scopes[ifStmt.Body]
sign = isInequality(compare.Op)
// callArg formats a call argument, preserving comments from [start-end).
@@ -93,7 +92,7 @@
sym := cond(sign < 0, "min", "max")
- if _, obj := scope.LookupParent(sym, ifStmt.Pos()); !is[*types.Builtin](obj) {
+ if !is[*types.Builtin](lookup(pass.TypesInfo, curIfStmt, sym)) {
return // min/max function is shadowed
}
@@ -149,7 +148,7 @@
}
sym := cond(sign < 0, "min", "max")
- if _, obj := scope.LookupParent(sym, ifStmt.Pos()); !is[*types.Builtin](obj) {
+ if !is[*types.Builtin](lookup(pass.TypesInfo, curIfStmt, sym)) {
return // min/max function is shadowed
}
diff --git a/gopls/internal/analysis/modernize/modernize.go b/gopls/internal/analysis/modernize/modernize.go
index ed7613f..63809ff 100644
--- a/gopls/internal/analysis/modernize/modernize.go
+++ b/gopls/internal/analysis/modernize/modernize.go
@@ -37,6 +37,7 @@
ForVarAnalyzer,
MapsLoopAnalyzer,
MinMaxAnalyzer,
+ NewExprAnalyzer,
OmitZeroAnalyzer,
RangeIntAnalyzer,
SlicesContainsAnalyzer,
@@ -143,6 +144,7 @@
builtinFalse = types.Universe.Lookup("false")
builtinLen = types.Universe.Lookup("len")
builtinMake = types.Universe.Lookup("make")
+ builtinNew = types.Universe.Lookup("new")
builtinNil = types.Universe.Lookup("nil")
builtinString = types.Universe.Lookup("string")
builtinTrue = types.Universe.Lookup("true")
@@ -187,3 +189,10 @@
})
return noEffects
}
+
+// lookup returns the symbol denoted by name at the position of the cursor.
+func lookup(info *types.Info, cur inspector.Cursor, name string) types.Object {
+ scope := analysisinternal.EnclosingScope(info, cur)
+ _, obj := scope.LookupParent(name, cur.Node().Pos())
+ return obj
+}
diff --git a/gopls/internal/analysis/modernize/modernize_test.go b/gopls/internal/analysis/modernize/modernize_test.go
index 77e7622..bf7e4ff 100644
--- a/gopls/internal/analysis/modernize/modernize_test.go
+++ b/gopls/internal/analysis/modernize/modernize_test.go
@@ -42,6 +42,10 @@
RunWithSuggestedFixes(t, TestData(), modernize.MinMaxAnalyzer, "minmax")
}
+func TestNewExpr(t *testing.T) {
+ RunWithSuggestedFixes(t, TestData(), modernize.NewExprAnalyzer, "newexpr")
+}
+
func TestOmitZero(t *testing.T) {
RunWithSuggestedFixes(t, TestData(), modernize.OmitZeroAnalyzer, "omitzero")
}
diff --git a/gopls/internal/analysis/modernize/newexpr.go b/gopls/internal/analysis/modernize/newexpr.go
new file mode 100644
index 0000000..90f931a
--- /dev/null
+++ b/gopls/internal/analysis/modernize/newexpr.go
@@ -0,0 +1,173 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package modernize
+
+import (
+ _ "embed"
+ "go/ast"
+ "go/token"
+ "go/types"
+ "strings"
+
+ "fmt"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/inspect"
+ "golang.org/x/tools/go/ast/inspector"
+ "golang.org/x/tools/go/types/typeutil"
+ "golang.org/x/tools/internal/analysisinternal"
+)
+
+var NewExprAnalyzer = &analysis.Analyzer{
+ Name: "newexpr",
+ Doc: analysisinternal.MustExtractDoc(doc, "newexpr"),
+ URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/newexpr",
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ Run: run,
+ FactTypes: []analysis.Fact{&newLike{}},
+}
+
+func run(pass *analysis.Pass) (any, error) {
+ var (
+ inspect = pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+ info = pass.TypesInfo
+ )
+
+ // Detect functions that are new-like, i.e. have the form:
+ //
+ // func f(x T) *T { return &x }
+ //
+ // meaning that it is equivalent to new(x), if x has type T.
+ for curFuncDecl := range inspect.Root().Preorder((*ast.FuncDecl)(nil)) {
+ decl := curFuncDecl.Node().(*ast.FuncDecl)
+ fn := info.Defs[decl.Name].(*types.Func)
+ if decl.Body != nil && len(decl.Body.List) == 1 {
+ if ret, ok := decl.Body.List[0].(*ast.ReturnStmt); ok && len(ret.Results) == 1 {
+ if unary, ok := ret.Results[0].(*ast.UnaryExpr); ok && unary.Op == token.AND {
+ if id, ok := unary.X.(*ast.Ident); ok {
+ if v, ok := info.Uses[id].(*types.Var); ok {
+ sig := fn.Signature()
+ if sig.Results().Len() == 1 &&
+ is[*types.Pointer](sig.Results().At(0).Type()) && // => no iface conversion
+ sig.Params().Len() == 1 &&
+ sig.Params().At(0) == v {
+
+ // Export a fact for each one.
+ pass.ExportObjectFact(fn, &newLike{})
+
+ // Check file version.
+ file := enclosingFile(curFuncDecl)
+ if !fileUses(info, file, "go1.26") {
+ continue // new(expr) not available in this file
+ }
+
+ var edits []analysis.TextEdit
+
+ // If 'new' is not shadowed, replace func body: &x -> new(x).
+ // This makes it safely and cleanly inlinable.
+ curRet, _ := curFuncDecl.FindNode(ret)
+ if lookup(info, curRet, "new") == builtinNew {
+ edits = []analysis.TextEdit{
+ // return &x
+ // ---- -
+ // return new(x)
+ {
+ Pos: unary.OpPos,
+ End: unary.OpPos + token.Pos(len("&")),
+ NewText: []byte("new("),
+ },
+ {
+ Pos: unary.X.End(),
+ End: unary.X.End(),
+ NewText: []byte(")"),
+ },
+ }
+ }
+
+ // Add a //go:fix inline annotation, if not already present.
+ // TODO(adonovan): use ast.ParseDirective when go1.26 is assured.
+ if !strings.Contains(decl.Doc.Text(), "go:fix inline") {
+ edits = append(edits, analysis.TextEdit{
+ Pos: decl.Pos(),
+ End: decl.Pos(),
+ NewText: []byte("//go:fix inline\n"),
+ })
+ }
+
+ if len(edits) > 0 {
+ pass.Report(analysis.Diagnostic{
+ Pos: decl.Name.Pos(),
+ End: decl.Name.End(),
+ Message: fmt.Sprintf("%s can be an inlinable wrapper around new(expr)", decl.Name),
+ SuggestedFixes: []analysis.SuggestedFix{
+ {
+ Message: "Make %s an inlinable wrapper around new(expr)",
+ TextEdits: edits,
+ },
+ },
+ })
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
+ // Report and transform calls, when safe.
+ // In effect, this is inlining the new-like function
+ // even before we have marked the callee with //go:fix inline.
+ for curCall := range inspect.Root().Preorder((*ast.CallExpr)(nil)) {
+ call := curCall.Node().(*ast.CallExpr)
+ var fact newLike
+ if fn, ok := typeutil.Callee(info, call).(*types.Func); ok &&
+ pass.ImportObjectFact(fn, &fact) {
+
+ // Check file version.
+ file := enclosingFile(curCall)
+ if !fileUses(info, file, "go1.26") {
+ continue // new(expr) not available in this file
+ }
+
+ // Check new is not shadowed.
+ if lookup(info, curCall, "new") != builtinNew {
+ continue
+ }
+
+ // Argument type must exactly match parameter type.
+ // TODO(adonovan): support generic varOf (see suppressed test case).
+ var (
+ targ = types.Default(info.TypeOf(call.Args[0]))
+ tparam = fn.Signature().Params().At(0).Type()
+ )
+ if !types.Identical(targ, tparam) {
+ continue
+ }
+
+ pass.Report(analysis.Diagnostic{
+ Pos: call.Pos(),
+ End: call.End(),
+ Message: fmt.Sprintf("call of %s(x) can be simplified to new(x)", fn.Name()),
+ SuggestedFixes: []analysis.SuggestedFix{{
+ Message: fmt.Sprintf("Simplify %s(x) to new(x)", fn.Name()),
+ TextEdits: []analysis.TextEdit{{
+ Pos: call.Fun.Pos(),
+ End: call.Fun.End(),
+ NewText: []byte("new"),
+ }},
+ }},
+ })
+ }
+ }
+
+ return nil, nil
+}
+
+// A newLike fact records that its associated function is "new-like".
+type newLike struct{}
+
+func (*newLike) AFact() {}
+func (*newLike) String() string { return "newlike" }
diff --git a/gopls/internal/analysis/modernize/stringscutprefix.go b/gopls/internal/analysis/modernize/stringscutprefix.go
index e614065..8bb6bbc 100644
--- a/gopls/internal/analysis/modernize/stringscutprefix.go
+++ b/gopls/internal/analysis/modernize/stringscutprefix.go
@@ -184,7 +184,8 @@
lhs := assign.Lhs[0]
obj := typeutil.Callee(info, call)
- if obj != stringsTrimPrefix && obj != bytesTrimPrefix && obj != stringsTrimSuffix && obj != bytesTrimSuffix {
+ if obj == nil ||
+ obj != stringsTrimPrefix && obj != bytesTrimPrefix && obj != stringsTrimSuffix && obj != bytesTrimSuffix {
continue
}
diff --git a/gopls/internal/analysis/modernize/testdata/src/newexpr/newexpr.go b/gopls/internal/analysis/modernize/testdata/src/newexpr/newexpr.go
new file mode 100644
index 0000000..d3ed055
--- /dev/null
+++ b/gopls/internal/analysis/modernize/testdata/src/newexpr/newexpr.go
@@ -0,0 +1,21 @@
+//go:build go1.26
+
+package newexpr
+
+// intVar returns a new var whose value is i.
+func intVar(i int) *int { return &i } // want `intVar can be an inlinable wrapper around new\(expr\)` intVar:"newlike"
+
+func stringVar(s string) *string { return &s } // want `stringVar can be an inlinable wrapper around new\(expr\)` stringVar:"newlike"
+
+func varOf[T any](x T) *T { return &x } // want `varOf can be an inlinable wrapper around new\(expr\)` varOf:"newlike"
+
+var (
+ s struct {
+ int
+ string
+ }
+ _ = intVar(123) // want `call of intVar\(x\) can be simplified to new\(x\)`
+ _ = stringVar("abc") // want `call of stringVar\(x\) can be simplified to new\(x\)`
+ _ = varOf(s) // --want `call of varOf\(x\) can be simplified to new\(x\)`
+ _ = varOf(123) // --want `call of varOf\(x\) can be simplified to new\(x\)`
+)
diff --git a/gopls/internal/analysis/modernize/testdata/src/newexpr/newexpr.go.golden b/gopls/internal/analysis/modernize/testdata/src/newexpr/newexpr.go.golden
new file mode 100644
index 0000000..9e659dd
--- /dev/null
+++ b/gopls/internal/analysis/modernize/testdata/src/newexpr/newexpr.go.golden
@@ -0,0 +1,25 @@
+//go:build go1.26
+
+package newexpr
+
+// intVar returns a new var whose value is i.
+//
+//go:fix inline
+func intVar(i int) *int { return new(i) } // want `intVar can be an inlinable wrapper around new\(expr\)` intVar:"newlike"
+
+//go:fix inline
+func stringVar(s string) *string { return new(s) } // want `stringVar can be an inlinable wrapper around new\(expr\)` stringVar:"newlike"
+
+//go:fix inline
+func varOf[T any](x T) *T { return new(x) } // want `varOf can be an inlinable wrapper around new\(expr\)` varOf:"newlike"
+
+var (
+ s struct {
+ int
+ string
+ }
+ _ = new(123) // want `call of intVar\(x\) can be simplified to new\(x\)`
+ _ = new("abc") // want `call of stringVar\(x\) can be simplified to new\(x\)`
+ _ = varOf(s) // --want `call of varOf\(x\) can be simplified to new\(x\)`
+ _ = varOf(123) // --want `call of varOf\(x\) can be simplified to new\(x\)`
+)
diff --git a/gopls/internal/golang/rename_check.go b/gopls/internal/golang/rename_check.go
index 1af4619..70a4acf 100644
--- a/gopls/internal/golang/rename_check.go
+++ b/gopls/internal/golang/rename_check.go
@@ -354,7 +354,7 @@
switch n := cur.Node().(type) {
case *ast.Ident:
if pkg.TypesInfo().Uses[n] == obj {
- block := enclosingBlock(pkg.TypesInfo(), cur)
+ block := analysisinternal.EnclosingBlock(pkg.TypesInfo(), cur)
if !fn(n, block) {
ok = false
}
@@ -399,27 +399,6 @@
return ok
}
-// enclosingBlock returns the innermost block logically enclosing the
-// AST node (an ast.Ident), specified as a Cursor.
-func enclosingBlock(info *types.Info, curId inspector.Cursor) *types.Scope {
- for cur := range curId.Enclosing() {
- n := cur.Node()
- // For some reason, go/types always associates a
- // function's scope with its FuncType.
- // See comments about scope above.
- switch f := n.(type) {
- case *ast.FuncDecl:
- n = f.Type
- case *ast.FuncLit:
- n = f.Type
- }
- if b := info.Scopes[n]; b != nil {
- return b
- }
- }
- panic("no Scope for *ast.File")
-}
-
func (r *renamer) checkLabel(label *types.Label) {
// Check there are no identical labels in the function's label block.
// (Label blocks don't nest, so this is easy.)
diff --git a/internal/analysisinternal/analysis.go b/internal/analysisinternal/analysis.go
index bc7f998..b0958aa 100644
--- a/internal/analysisinternal/analysis.go
+++ b/internal/analysisinternal/analysis.go
@@ -664,3 +664,23 @@
c, _ = moreiters.First(c.Enclosing((*ast.File)(nil)))
return c.Node().(*ast.File)
}
+
+// EnclosingScope returns the innermost block logically enclosing the cursor.
+func EnclosingScope(info *types.Info, cur inspector.Cursor) *types.Scope {
+ for cur := range cur.Enclosing() {
+ n := cur.Node()
+ // For some reason, go/types always associates a
+ // function's scope with its FuncType.
+ // See comments about scope above.
+ switch f := n.(type) {
+ case *ast.FuncDecl:
+ n = f.Type
+ case *ast.FuncLit:
+ n = f.Type
+ }
+ if b := info.Scopes[n]; b != nil {
+ return b
+ }
+ }
+ panic("no Scope for *ast.File")
+}
Change information
Files:
- M gopls/internal/analysis/modernize/any.go
- M gopls/internal/analysis/modernize/doc.go
- M gopls/internal/analysis/modernize/minmax.go
- M gopls/internal/analysis/modernize/modernize.go
- M gopls/internal/analysis/modernize/modernize_test.go
- A gopls/internal/analysis/modernize/newexpr.go
- M gopls/internal/analysis/modernize/stringscutprefix.go
- A gopls/internal/analysis/modernize/testdata/src/newexpr/newexpr.go
- A gopls/internal/analysis/modernize/testdata/src/newexpr/newexpr.go.golden
- M gopls/internal/golang/rename_check.go
- M internal/analysisinternal/analysis.go
Change size: L
Delta: 11 files changed, 288 insertions(+), 31 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Sep 19, 2025, 6:25:42 PM (12 days ago) Sep 19
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
Related details
Attention is currently required from:
- Alan Donovan
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Sep 22, 2025, 12:42:30 PM (9 days ago) Sep 22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Robert Findley
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #3 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
Related details
Attention is currently required from:
- Alan Donovan
- Robert Findley
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Sep 22, 2025, 12:50:40 PM (9 days ago) Sep 22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Robert Findley
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #4 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
Related details
Attention is currently required from:
- Alan Donovan
- Robert Findley
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Sep 23, 2025, 1:24:43 PM (8 days ago) Sep 23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Robert Findley
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #5 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Alan Donovan
- Robert Findley
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Sep 23, 2025, 2:11:00 PM (8 days ago) Sep 23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Robert Findley
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #7 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Alan Donovan
- Robert Findley
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Sep 23, 2025, 2:13:51 PM (8 days ago) Sep 23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Robert Findley
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #8 to this change.
Related details
Attention is currently required from:
- Alan Donovan
- Robert Findley
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Robert Findley (Gerrit)
Sep 30, 2025, 2:36:02 PM (yesterday) Sep 30
to Alan Donovan, goph...@pubsubhelper.golang.org, Madeline Kalil, Go LUCI, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Madeline Kalil
Robert Findley added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Alan Donovan
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ingo Oeser (Gerrit)
Sep 30, 2025, 7:47:22 PM (23 hours ago) Sep 30
to Alan Donovan, goph...@pubsubhelper.golang.org, Madeline Kalil, Go LUCI, Robert Findley, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Madeline Kalil
Ingo Oeser added 2 comments![]( "Open in Gerrit")
Patchset-level comments
Ingo Oeser . resolved
Another interesting variant.
File gopls/internal/analysis/modernize/testdata/src/newexpr/newexpr.go
Line 26, Patchset 8 (Latest):
)Ingo Oeser . unresolved
Another interesting case:
_ = varOf(int64(123))
Which is use a lot from people who prefer type inference over explicit type parameters.
Related details
Attention is currently required from:
- Alan Donovan
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
10:04 AM (8 hours ago) 10:04 AM
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Madeline Kalil
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #9 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Alan Donovan
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
10:06 AM (8 hours ago) 10:06 AM
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Madeline Kalil
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #10 to this change.
Related details
Attention is currently required from:
- Alan Donovan
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
10:06 AM (8 hours ago) 10:06 AM
to goph...@pubsubhelper.golang.org, Go LUCI, Ingo Oeser, Madeline Kalil, Robert Findley, golang-co...@googlegroups.com
Attention needed from Ingo Oeser and Madeline Kalil
Alan Donovan added 1 comment![]( "Open in Gerrit")
File gopls/internal/analysis/modernize/testdata/src/newexpr/newexpr.go
_ = varOf(int64(123))
Which is use a lot from people who prefer type inference over explicit type parameters.
Attention is currently required from:
- Ingo Oeser
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
11:10 AM (7 hours ago) 11:10 AM
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan, Ingo Oeser and Madeline Kalil
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #11 to this change.
Following approvals got outdated and were removed:
Attention is currently required from:
- Alan Donovan
- Ingo Oeser
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
4:47 PM (2 hours ago) 4:47 PM
to Alan Donovan, goph...@pubsubhelper.golang.org, Go LUCI, Ingo Oeser, Robert Findley, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Ingo Oeser
Madeline Kalil added 2 comments![]( "Open in Gerrit")
File go/analysis/passes/modernize/newexpr.go
Line 43, Patchset 11 (Latest):
for curFuncDecl := range inspect.Root().Preorder((*ast.FuncDecl)(nil)) {Madeline Kalil . unresolved
I'm curious about starting at inspect.Root() versus inspecting all files using 1.26 and then looking at FuncDecls -- is it just so we can export an object fact for each matching instance regardless of the file version?
File go/analysis/passes/modernize/testdata/src/newexpr/newexpr_go125.go
Related details
Attention is currently required from:
- Alan Donovan
- Ingo Oeser
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
6:11 PM (22 minutes ago) 6:11 PM
to goph...@pubsubhelper.golang.org, Go LUCI, Ingo Oeser, Madeline Kalil, Robert Findley, golang-co...@googlegroups.com
Attention needed from Ingo Oeser and Madeline Kalil
Alan Donovan added 2 comments![]( "Open in Gerrit")
File go/analysis/passes/modernize/newexpr.go
Line 43, Patchset 11 (Latest):
for curFuncDecl := range inspect.Root().Preorder((*ast.FuncDecl)(nil)) {Madeline Kalil . resolved
I'm curious about starting at inspect.Root() versus inspecting all files using 1.26 and then looking at FuncDecls -- is it just so we can export an object fact for each matching instance regardless of the file version?
Alan Donovan
Good question. The version filter is not expected to be very selective, at least in the long run, so it's better to locate the problems first using the most efficient approach, then reject based on version after. Having made that realization, I have been slowly chipping away at uses of filesUsing and not using in new code.
But in this case, as you allude, we still need to export object facts for new-like functions in older code so that calls to them from newer code can be checked, and thus it wouldn't be correct to use filesUsing.
File go/analysis/passes/modernize/testdata/src/newexpr/newexpr_go125.go
Madeline Kalil . resolved
What is this file for?
Alan Donovan
It's to prevent the package from having no .go files (an error) under some build configurations.
Related details
Attention is currently required from:
- Ingo Oeser
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages