Skip to content

Commit eebe4d7

Browse files
committed
internal/refactor/inline: check caller/callee file Go versions
Inlining a function body declared in a file using a newer Go release into a caller using an older Go release is liable to introducing build errors. (This is particularly a risk for go:fix inline directives introduced by modernizers for newer language features.) Ideally the type checker would record, for each piece of syntax, what minimum Go version it requires, so that we could compute the dependencies precisely for a given callee function body. In the meantime, we use the version of the callee file as a whole, which is quite conservative, and check that the caller file's version is not older. Updates golang/go#75726 + unit test, gopls integration test Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce Reviewed-on: https://go-review.googlesource.com/c/tools/+/716560 Reviewed-by: Robert Findley <rfindley@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent f8a6b84 commit eebe4d7

File tree

6 files changed

+86
-15
lines changed

6 files changed

+86
-15
lines changed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
Test of caller/callee Go version compatibility check (#75726).
2+
3+
-- go.work --
4+
use .
5+
use ./b
6+
7+
-- go.mod --
8+
module example.com
9+
go 1.18
10+
11+
-- a/a.go --
12+
package a
13+
14+
import "example.com/b"
15+
16+
var _ = b.Add(1, 2) //@ codeaction("Add", "refactor.inline.call", end=")", err=`cannot inline call to b.Add (declared using go1.20) into a file using go1.18`)
17+
18+
-- b/go.mod --
19+
module example.com/b
20+
go 1.20
21+
22+
-- b/b.go --
23+
package b
24+
25+
func Add(x, y int) int { return x + y }

internal/refactor/inline/callee.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ type gobCallee struct {
3636
// results of type analysis (does not reach go/types data structures)
3737
PkgPath string // package path of declaring package
3838
Name string // user-friendly name for error messages
39+
GoVersion string // version of Go effective in callee file
3940
Unexported []string // names of free objects that are unexported
4041
FreeRefs []freeRef // locations of references to free objects
4142
FreeObjs []object // descriptions of free objects
@@ -114,6 +115,24 @@ func AnalyzeCallee(logf func(string, ...any), fset *token.FileSet, pkg *types.Pa
114115
return nil, fmt.Errorf("cannot inline function %s as it has no body", name)
115116
}
116117

118+
// Record the file's Go goVersion so that we don't
119+
// inline newer code into file using an older dialect.
120+
//
121+
// Using the file version is overly conservative.
122+
// A more precise solution would be for the type checker to
123+
// record which language features the callee actually needs;
124+
// see https://go.dev/issue/75726.
125+
//
126+
// We don't have the ast.File handy, so instead of a
127+
// lookup we must scan the entire FileVersions map.
128+
var goVersion string
129+
for file, v := range info.FileVersions {
130+
if file.Pos() < decl.Pos() && decl.Pos() < file.End() {
131+
goVersion = v
132+
break
133+
}
134+
}
135+
117136
// Record the location of all free references in the FuncDecl.
118137
// (Parameters are not free by this definition.)
119138
var (
@@ -342,6 +361,7 @@ func AnalyzeCallee(logf func(string, ...any), fset *token.FileSet, pkg *types.Pa
342361
Content: content,
343362
PkgPath: pkg.Path(),
344363
Name: name,
364+
GoVersion: goVersion,
345365
Unexported: unexported,
346366
FreeObjs: freeObjs,
347367
FreeRefs: freeRefs,

internal/refactor/inline/calleefx_test.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,13 @@ func TestCalleeEffects(t *testing.T) {
132132
}
133133

134134
info := &types.Info{
135-
Defs: make(map[*ast.Ident]types.Object),
136-
Uses: make(map[*ast.Ident]types.Object),
137-
Types: make(map[ast.Expr]types.TypeAndValue),
138-
Implicits: make(map[ast.Node]types.Object),
139-
Selections: make(map[*ast.SelectorExpr]*types.Selection),
140-
Scopes: make(map[ast.Node]*types.Scope),
135+
Defs: make(map[*ast.Ident]types.Object),
136+
Uses: make(map[*ast.Ident]types.Object),
137+
Types: make(map[ast.Expr]types.TypeAndValue),
138+
Implicits: make(map[ast.Node]types.Object),
139+
Selections: make(map[*ast.SelectorExpr]*types.Selection),
140+
Scopes: make(map[ast.Node]*types.Scope),
141+
FileVersions: make(map[*ast.File]string),
141142
}
142143
conf := &types.Config{Error: func(err error) { t.Error(err) }}
143144
pkg, err := conf.Check("p", fset, []*ast.File{calleeFile}, info)

internal/refactor/inline/inline.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"golang.org/x/tools/internal/packagepath"
2929
"golang.org/x/tools/internal/typeparams"
3030
"golang.org/x/tools/internal/typesinternal"
31+
"golang.org/x/tools/internal/versions"
3132
)
3233

3334
// A Caller describes the function call and its enclosing context.
@@ -677,6 +678,15 @@ func (st *state) inlineCall() (*inlineCallResult, error) {
677678
callee.Name, callee.Unexported[0])
678679
}
679680

681+
// Reject cross-file inlining if callee requires a newer dialect of Go (#75726).
682+
// (Versions default to types.Config.GoVersion, which is unset in many tests,
683+
// though should be populated by an analysis driver.)
684+
callerGoVersion := caller.Info.FileVersions[caller.File]
685+
if callerGoVersion != "" && callee.GoVersion != "" && versions.Before(callerGoVersion, callee.GoVersion) {
686+
return nil, fmt.Errorf("cannot inline call to %s (declared using %s) into a file using %s",
687+
callee.Name, callee.GoVersion, callerGoVersion)
688+
}
689+
680690
// -- analyze callee's free references in caller context --
681691

682692
// Compute syntax path enclosing Call, innermost first (Path[0]=Call),

internal/refactor/inline/inline_test.go

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ const funcName = "f"
365365
// strategy with the checklist of concerns enumerated in the package
366366
// doc comment.
367367
type testcase struct {
368-
descr string // description; substrings enable options (e.g. "IgnoreEffects")
368+
descr string // description; substrings enable test or inliner options (e.g. "IgnoreEffects", "NoPackageClause")
369369
callee, caller string // Go source files (sans package decl) of caller, callee
370370
want string // expected new portion of caller file, or "error: regexp"
371371
}
@@ -384,6 +384,12 @@ func TestErrors(t *testing.T) {
384384
`var _ = G[int]{}.f(0)`,
385385
`error: generic methods not yet supported`,
386386
},
387+
{
388+
"[NoPackageClause] Can't inline a callee using newer Go to a caller using older Go (#75726).",
389+
"//go:build go1.23\n\npackage p\nfunc f() int { return 0 }",
390+
"//go:build go1.22\n\npackage p\nvar _ = f()",
391+
`error: cannot inline call to p.f \(declared using go1.23\) into a file using go1.22`,
392+
},
387393
})
388394
}
389395

@@ -1818,8 +1824,15 @@ func runTests(t *testing.T, tests []testcase) {
18181824
return f
18191825
}
18201826

1827+
addPackageClause := func(src string) string {
1828+
if !strings.Contains(test.descr, "NoPackageClause") {
1829+
src = "package p\n" + src
1830+
}
1831+
return src
1832+
}
1833+
18211834
// Parse callee file and find first func decl named f.
1822-
calleeContent := "package p\n" + test.callee
1835+
calleeContent := addPackageClause(test.callee)
18231836
calleeFile := mustParse("callee.go", calleeContent)
18241837
var decl *ast.FuncDecl
18251838
for _, d := range calleeFile.Decls {
@@ -1833,7 +1846,7 @@ func runTests(t *testing.T, tests []testcase) {
18331846
}
18341847

18351848
// Parse caller file and find first call to f().
1836-
callerContent := "package p\n" + test.caller
1849+
callerContent := addPackageClause(test.caller)
18371850
callerFile := mustParse("caller.go", callerContent)
18381851
var call *ast.CallExpr
18391852
ast.Inspect(callerFile, func(n ast.Node) bool {
@@ -1865,12 +1878,13 @@ func runTests(t *testing.T, tests []testcase) {
18651878

18661879
// Type check both files as one package.
18671880
info := &types.Info{
1868-
Defs: make(map[*ast.Ident]types.Object),
1869-
Uses: make(map[*ast.Ident]types.Object),
1870-
Types: make(map[ast.Expr]types.TypeAndValue),
1871-
Implicits: make(map[ast.Node]types.Object),
1872-
Selections: make(map[*ast.SelectorExpr]*types.Selection),
1873-
Scopes: make(map[ast.Node]*types.Scope),
1881+
Defs: make(map[*ast.Ident]types.Object),
1882+
Uses: make(map[*ast.Ident]types.Object),
1883+
Types: make(map[ast.Expr]types.TypeAndValue),
1884+
Implicits: make(map[ast.Node]types.Object),
1885+
Selections: make(map[*ast.SelectorExpr]*types.Selection),
1886+
Scopes: make(map[ast.Node]*types.Scope),
1887+
FileVersions: make(map[*ast.File]string),
18741888
}
18751889
conf := &types.Config{Error: func(err error) { t.Error(err) }}
18761890
pkg, err := conf.Check("p", fset, []*ast.File{callerFile, calleeFile}, info)

internal/refactor/inline/util.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ func checkInfoFields(info *types.Info) {
9797
assert(info.Selections != nil, "types.Info.Selections is nil")
9898
assert(info.Types != nil, "types.Info.Types is nil")
9999
assert(info.Uses != nil, "types.Info.Uses is nil")
100+
assert(info.FileVersions != nil, "types.Info.FileVersions is nil")
100101
}
101102

102103
// intersects reports whether the maps' key sets intersect.

0 commit comments

Comments
 (0)