go/analysis/passes/modernize: newexpr: add //go:fix inline directives

This CL causes the newexpr suggested fix to add "inline" directives
on the function equivalent to new(expr) so that calls to it are
eventually melted away. This is safe because the inliner now checks
that the caller's Go version is at least as new as the callee's.

Updates golang/go#45624
Updates golang/go#75726

Change-Id: I36d27c7fa92965340da739ab71ed0cddff0d03ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/716561
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

Author: adonovan

go/analysis/passes/modernize/doc.go

@@ -176,8 +176,12 @@ This analyzer finds declarations of functions of this form:
 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
 
@@ -187,9 +191,9 @@ is replaced by:
 
 	use(new(123))
 
-(Wrapper functions such as varOf are common when working with Go
+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.)
+are often used to express optionality.
 
 # Analyzer omitzero
 

go/analysis/passes/modernize/newexpr.go

@@ -87,25 +87,18 @@ func run(pass *analysis.Pass) (any, error) {
 									}
 								}
 
-								// Disabled until we resolve https://go.dev/issue/75726
-								// (Go version skew between caller and callee in inliner.)
-								// TODO(adonovan): fix and reenable.
+								// Add a //go:fix inline annotation, if not already present.
 								//
-								// Also, restore these lines to our section of doc.go:
-								// 	//go:fix inline
-								//	...
-								// 	(The directive comment causes the inline analyzer to suggest
-								// 	that calls to such functions are inlined.)
-								if false {
-									// 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"),
-										})
-									}
+								// The inliner will not inline a newer callee body into an
+								// older Go file; see https://go.dev/issue/75726.
+								//
+								// 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 {

go/analysis/passes/modernize/testdata/src/newexpr/newexpr.go.golden

@@ -3,12 +3,17 @@
 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 int64Var(i int64) *int64 { return new(i) } // want `int64Var can be an inlinable wrapper around new\(expr\)` int64Var:"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 (

gopls/doc/analyzers.md

@@ -3469,8 +3469,11 @@ This analyzer finds declarations of functions of this form:
 
 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))
@@ -3479,7 +3482,7 @@ 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.)
+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.
 
 
 Default: on.

gopls/internal/doc/api.json

@@ -1546,7 +1546,7 @@
 						},
 						{
 							"Name": "\"newexpr\"",
-							"Doc": "simplify code by using go1.26's new(expr)\n\nThis analyzer finds declarations of functions of this form:\n\n\tfunc varOf(x int) *int { return \u0026x }\n\nand suggests a fix to turn them into inlinable wrappers around\ngo1.26's built-in new(expr) function:\n\n\tfunc varOf(x int) *int { return new(x) }\n\nIn addition, this analyzer suggests a fix for each call\nto one of the functions before it is transformed, so that\n\n\tuse(varOf(123))\n\nis replaced by:\n\n\tuse(new(123))\n\n(Wrapper functions such as varOf are common when working with Go\nserialization packages such as for JSON or protobuf, where pointers\nare often used to express optionality.)",
+							"Doc": "simplify code by using go1.26's new(expr)\n\nThis analyzer finds declarations of functions of this form:\n\n\tfunc varOf(x int) *int { return \u0026x }\n\nand suggests a fix to turn them into inlinable wrappers around\ngo1.26's built-in new(expr) function:\n\n\t//go:fix inline\n\tfunc varOf(x int) *int { return new(x) }\n\n(The directive comment causes the 'inline' analyzer to suggest\nthat calls to such functions are inlined.)\n\nIn addition, this analyzer suggests a fix for each call\nto one of the functions before it is transformed, so that\n\n\tuse(varOf(123))\n\nis replaced by:\n\n\tuse(new(123))\n\nWrapper functions such as varOf are common when working with Go\nserialization packages such as for JSON or protobuf, where pointers\nare often used to express optionality.",
 							"Default": "true",
 							"Status": ""
 						},
@@ -3449,7 +3449,7 @@
 		},
 		{
 			"Name": "newexpr",
-			"Doc": "simplify code by using go1.26's new(expr)\n\nThis analyzer finds declarations of functions of this form:\n\n\tfunc varOf(x int) *int { return \u0026x }\n\nand suggests a fix to turn them into inlinable wrappers around\ngo1.26's built-in new(expr) function:\n\n\tfunc varOf(x int) *int { return new(x) }\n\nIn addition, this analyzer suggests a fix for each call\nto one of the functions before it is transformed, so that\n\n\tuse(varOf(123))\n\nis replaced by:\n\n\tuse(new(123))\n\n(Wrapper functions such as varOf are common when working with Go\nserialization packages such as for JSON or protobuf, where pointers\nare often used to express optionality.)",
+			"Doc": "simplify code by using go1.26's new(expr)\n\nThis analyzer finds declarations of functions of this form:\n\n\tfunc varOf(x int) *int { return \u0026x }\n\nand suggests a fix to turn them into inlinable wrappers around\ngo1.26's built-in new(expr) function:\n\n\t//go:fix inline\n\tfunc varOf(x int) *int { return new(x) }\n\n(The directive comment causes the 'inline' analyzer to suggest\nthat calls to such functions are inlined.)\n\nIn addition, this analyzer suggests a fix for each call\nto one of the functions before it is transformed, so that\n\n\tuse(varOf(123))\n\nis replaced by:\n\n\tuse(new(123))\n\nWrapper functions such as varOf are common when working with Go\nserialization packages such as for JSON or protobuf, where pointers\nare often used to express optionality.",
 			"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize#newexpr",
 			"Default": true
 		},