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>
diff --git a/gopls/internal/test/marker/testdata/codeaction/inline-version.txt b/gopls/internal/test/marker/testdata/codeaction/inline-version.txt
new file mode 100644
index 0000000..bd41718
--- /dev/null
+++ b/gopls/internal/test/marker/testdata/codeaction/inline-version.txt
@@ -0,0 +1,25 @@
+Test of caller/callee Go version compatibility check (#75726).
+
+-- go.work --
+use .
+use ./b
+
+-- go.mod --
+module example.com
+go 1.18
+
+-- a/a.go --
+package a
+
+import "example.com/b"
+
+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`)
+
+-- b/go.mod --
+module example.com/b
+go 1.20
+
+-- b/b.go --
+package b
+
+func Add(x, y int) int { return x + y }
diff --git a/internal/refactor/inline/callee.go b/internal/refactor/inline/callee.go
index b46340c..ab2979d 100644
--- a/internal/refactor/inline/callee.go
+++ b/internal/refactor/inline/callee.go
@@ -36,6 +36,7 @@
// results of type analysis (does not reach go/types data structures)
PkgPath string // package path of declaring package
Name string // user-friendly name for error messages
+ GoVersion string // version of Go effective in callee file
Unexported []string // names of free objects that are unexported
FreeRefs []freeRef // locations of references to free objects
FreeObjs []object // descriptions of free objects
@@ -114,6 +115,24 @@
return nil, fmt.Errorf("cannot inline function %s as it has no body", name)
}
+ // Record the file's Go goVersion so that we don't
+ // inline newer code into file using an older dialect.
+ //
+ // Using the file version is overly conservative.
+ // A more precise solution would be for the type checker to
+ // record which language features the callee actually needs;
+ // see https://go.dev/issue/75726.
+ //
+ // We don't have the ast.File handy, so instead of a
+ // lookup we must scan the entire FileVersions map.
+ var goVersion string
+ for file, v := range info.FileVersions {
+ if file.Pos() < decl.Pos() && decl.Pos() < file.End() {
+ goVersion = v
+ break
+ }
+ }
+
// Record the location of all free references in the FuncDecl.
// (Parameters are not free by this definition.)
var (
@@ -342,6 +361,7 @@
Content: content,
PkgPath: pkg.Path(),
Name: name,
+ GoVersion: goVersion,
Unexported: unexported,
FreeObjs: freeObjs,
FreeRefs: freeRefs,
diff --git a/internal/refactor/inline/calleefx_test.go b/internal/refactor/inline/calleefx_test.go
index b643c7a..55a869c 100644
--- a/internal/refactor/inline/calleefx_test.go
+++ b/internal/refactor/inline/calleefx_test.go
@@ -132,12 +132,13 @@
}
info := &types.Info{
- Defs: make(map[*ast.Ident]types.Object),
- Uses: make(map[*ast.Ident]types.Object),
- Types: make(map[ast.Expr]types.TypeAndValue),
- Implicits: make(map[ast.Node]types.Object),
- Selections: make(map[*ast.SelectorExpr]*types.Selection),
- Scopes: make(map[ast.Node]*types.Scope),
+ Defs: make(map[*ast.Ident]types.Object),
+ Uses: make(map[*ast.Ident]types.Object),
+ Types: make(map[ast.Expr]types.TypeAndValue),
+ Implicits: make(map[ast.Node]types.Object),
+ Selections: make(map[*ast.SelectorExpr]*types.Selection),
+ Scopes: make(map[ast.Node]*types.Scope),
+ FileVersions: make(map[*ast.File]string),
}
conf := &types.Config{Error: func(err error) { t.Error(err) }}
pkg, err := conf.Check("p", fset, []*ast.File{calleeFile}, info)
diff --git a/internal/refactor/inline/inline.go b/internal/refactor/inline/inline.go
index c3055be..9975965 100644
--- a/internal/refactor/inline/inline.go
+++ b/internal/refactor/inline/inline.go
@@ -28,6 +28,7 @@
"golang.org/x/tools/internal/packagepath"
"golang.org/x/tools/internal/typeparams"
"golang.org/x/tools/internal/typesinternal"
+ "golang.org/x/tools/internal/versions"
)
// A Caller describes the function call and its enclosing context.
@@ -677,6 +678,15 @@
callee.Name, callee.Unexported[0])
}
+ // Reject cross-file inlining if callee requires a newer dialect of Go (#75726).
+ // (Versions default to types.Config.GoVersion, which is unset in many tests,
+ // though should be populated by an analysis driver.)
+ callerGoVersion := caller.Info.FileVersions[caller.File]
+ if callerGoVersion != "" && callee.GoVersion != "" && versions.Before(callerGoVersion, callee.GoVersion) {
+ return nil, fmt.Errorf("cannot inline call to %s (declared using %s) into a file using %s",
+ callee.Name, callee.GoVersion, callerGoVersion)
+ }
+
// -- analyze callee's free references in caller context --
// Compute syntax path enclosing Call, innermost first (Path[0]=Call),
diff --git a/internal/refactor/inline/inline_test.go b/internal/refactor/inline/inline_test.go
index 93dd262..31e20af 100644
--- a/internal/refactor/inline/inline_test.go
+++ b/internal/refactor/inline/inline_test.go
@@ -365,7 +365,7 @@
// strategy with the checklist of concerns enumerated in the package
// doc comment.
type testcase struct {
- descr string // description; substrings enable options (e.g. "IgnoreEffects")
+ descr string // description; substrings enable test or inliner options (e.g. "IgnoreEffects", "NoPackageClause")
callee, caller string // Go source files (sans package decl) of caller, callee
want string // expected new portion of caller file, or "error: regexp"
}
@@ -384,6 +384,12 @@
`var _ = G[int]{}.f(0)`,
`error: generic methods not yet supported`,
},
+ {
+ "[NoPackageClause] Can't inline a callee using newer Go to a caller using older Go (#75726).",
+ "//go:build go1.23\n\npackage p\nfunc f() int { return 0 }",
+ "//go:build go1.22\n\npackage p\nvar _ = f()",
+ `error: cannot inline call to p.f \(declared using go1.23\) into a file using go1.22`,
+ },
})
}
@@ -1818,8 +1824,15 @@
return f
}
+ addPackageClause := func(src string) string {
+ if !strings.Contains(test.descr, "NoPackageClause") {
+ src = "package p\n" + src
+ }
+ return src
+ }
+
// Parse callee file and find first func decl named f.
- calleeContent := "package p\n" + test.callee
+ calleeContent := addPackageClause(test.callee)
calleeFile := mustParse("callee.go", calleeContent)
var decl *ast.FuncDecl
for _, d := range calleeFile.Decls {
@@ -1833,7 +1846,7 @@
}
// Parse caller file and find first call to f().
- callerContent := "package p\n" + test.caller
+ callerContent := addPackageClause(test.caller)
callerFile := mustParse("caller.go", callerContent)
var call *ast.CallExpr
ast.Inspect(callerFile, func(n ast.Node) bool {
@@ -1865,12 +1878,13 @@
// Type check both files as one package.
info := &types.Info{
- Defs: make(map[*ast.Ident]types.Object),
- Uses: make(map[*ast.Ident]types.Object),
- Types: make(map[ast.Expr]types.TypeAndValue),
- Implicits: make(map[ast.Node]types.Object),
- Selections: make(map[*ast.SelectorExpr]*types.Selection),
- Scopes: make(map[ast.Node]*types.Scope),
+ Defs: make(map[*ast.Ident]types.Object),
+ Uses: make(map[*ast.Ident]types.Object),
+ Types: make(map[ast.Expr]types.TypeAndValue),
+ Implicits: make(map[ast.Node]types.Object),
+ Selections: make(map[*ast.SelectorExpr]*types.Selection),
+ Scopes: make(map[ast.Node]*types.Scope),
+ FileVersions: make(map[*ast.File]string),
}
conf := &types.Config{Error: func(err error) { t.Error(err) }}
pkg, err := conf.Check("p", fset, []*ast.File{callerFile, calleeFile}, info)
diff --git a/internal/refactor/inline/util.go b/internal/refactor/inline/util.go
index 205e5b6..5f895cc 100644
--- a/internal/refactor/inline/util.go
+++ b/internal/refactor/inline/util.go
@@ -97,6 +97,7 @@
assert(info.Selections != nil, "types.Info.Selections is nil")
assert(info.Types != nil, "types.Info.Types is nil")
assert(info.Uses != nil, "types.Info.Uses is nil")
+ assert(info.FileVersions != nil, "types.Info.FileVersions is nil")
}
// intersects reports whether the maps' key sets intersect.
