[tools] internal/refactor/inline: check caller/callee file Go versions

5 views
Skip to first unread message

Alan Donovan (Gerrit)

unread,
Oct 30, 2025, 1:08:26 PM (7 days ago) Oct 30
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Alan Donovan has uploaded the change for review

Commit message

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

+ test
Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce

Change diff

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 2443504..6d2f6b4 100644
--- a/internal/refactor/inline/inline.go
+++ b/internal/refactor/inline/inline.go
@@ -27,6 +27,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,13 @@
 			callee.Name, callee.Unexported[0])
 	}
 
+	// Reject cross-file inlining if callee requires a newer dialect of Go (#75726).
+	callerGoVersion := caller.Info.FileVersions[caller.File]
+	if 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..6cafd69 100644
--- a/internal/refactor/inline/inline_test.go
+++ b/internal/refactor/inline/inline_test.go
@@ -384,6 +384,12 @@
 			`var _ = G[int]{}.f(0)`,
 			`error: generic methods not yet supported`,
 		},
+		{
+			"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`,
+		},
 	})
 }
 
@@ -1807,6 +1813,14 @@
 }
 
 func runTests(t *testing.T, tests []testcase) {
+	addPackageClause := func(src string) string {
+		// Most test cases omit it for brevity.
+		if !strings.Contains(src, "package p") {
+			src = "package p\n" + src
+		}
+		return src
+	}
+
 	for _, test := range tests {
 		t.Run(test.descr, func(t *testing.T) {
 			fset := token.NewFileSet()
@@ -1819,7 +1833,7 @@
 			}
 
 			// 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 +1847,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 +1879,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.

Change information

Files:
  • M internal/refactor/inline/callee.go
  • M internal/refactor/inline/calleefx_test.go
  • M internal/refactor/inline/inline.go
  • M internal/refactor/inline/inline_test.go
  • M internal/refactor/inline/util.go
Change size: M
Delta: 5 files changed, 59 insertions(+), 14 deletions(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: tools
Gerrit-Branch: master
Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
Gerrit-Change-Number: 716560
Gerrit-PatchSet: 1
Gerrit-Owner: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

Alan Donovan (Gerrit)

unread,
Oct 30, 2025, 1:08:38 PM (7 days ago) Oct 30
to goph...@pubsubhelper.golang.org, Robert Findley, Go LUCI, golang-co...@googlegroups.com
Attention needed from Robert Findley

Alan Donovan voted Auto-Submit+1

Auto-Submit+1
Open in Gerrit

Related details

Attention is currently required from:
  • Robert Findley
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: tools
Gerrit-Branch: master
Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
Gerrit-Change-Number: 716560
Gerrit-PatchSet: 1
Gerrit-Owner: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Robert Findley <rfin...@google.com>
Gerrit-Attention: Robert Findley <rfin...@google.com>
Gerrit-Comment-Date: Thu, 30 Oct 2025 17:08:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
unsatisfied_requirement
satisfied_requirement
open
diffy

Robert Findley (Gerrit)

unread,
Oct 30, 2025, 2:20:28 PM (7 days ago) Oct 30
to Alan Donovan, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Attention needed from Alan Donovan

Robert Findley added 1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Robert Findley . unresolved

Help me understand the consequences of this.

This will usually be OK because if my dependency is in another module, my current module must have go version at least that of its dependency? So the only functions that would not be inlinable are those that have a file build constraint?

Open in Gerrit

Related details

Attention is currently required from:
  • Alan Donovan
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
    Gerrit-Change-Number: 716560
    Gerrit-PatchSet: 1
    Gerrit-Owner: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Robert Findley <rfin...@google.com>
    Gerrit-Attention: Alan Donovan <adon...@google.com>
    Gerrit-Comment-Date: Thu, 30 Oct 2025 18:20:24 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Alan Donovan (Gerrit)

    unread,
    Oct 30, 2025, 2:33:21 PM (7 days ago) Oct 30
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
    Attention needed from Alan Donovan

    Alan Donovan uploaded new patchset

    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
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Alan Donovan
    Submit Requirements:
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      • requirement is not satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: newpatchset
      Gerrit-Project: tools
      Gerrit-Branch: master
      Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
      Gerrit-Change-Number: 716560
      Gerrit-PatchSet: 2
      unsatisfied_requirement
      open
      diffy

      Alan Donovan (Gerrit)

      unread,
      Oct 30, 2025, 2:41:00 PM (7 days ago) Oct 30
      to goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, golang-co...@googlegroups.com
      Attention needed from Robert Findley

      Alan Donovan voted and added 1 comment

      Votes added by Alan Donovan

      Auto-Submit+1

      1 comment

      Patchset-level comments
      File-level comment, Patchset 1:
      Robert Findley . resolved

      Help me understand the consequences of this.

      This will usually be OK because if my dependency is in another module, my current module must have go version at least that of its dependency? So the only functions that would not be inlinable are those that have a file build constraint?

      Alan Donovan

      The go command will allow a lower-Go-version module to depend on a higher-versioned one, and so this check could deny users the ability to inline, but such a configuration is untidy and the next go mod tidy operation will update the lower module to the minimum of its dependencies; so I don't expect this denial to be very common.

      I added an integration test of this scenario in gopls; there are no apparent problems from the untidiness, and the check fires as it should.

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Robert Findley
      Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: tools
        Gerrit-Branch: master
        Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
        Gerrit-Change-Number: 716560
        Gerrit-PatchSet: 1
        Gerrit-Owner: Alan Donovan <adon...@google.com>
        Gerrit-Reviewer: Alan Donovan <adon...@google.com>
        Gerrit-Reviewer: Robert Findley <rfin...@google.com>
        Gerrit-Attention: Robert Findley <rfin...@google.com>
        Gerrit-Comment-Date: Thu, 30 Oct 2025 18:40:53 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        Comment-In-Reply-To: Robert Findley <rfin...@google.com>
        unsatisfied_requirement
        satisfied_requirement
        open
        diffy

        Robert Findley (Gerrit)

        unread,
        Oct 30, 2025, 3:58:22 PM (7 days ago) Oct 30
        to Alan Donovan, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
        Attention needed from Alan Donovan

        Robert Findley voted and added 2 comments

        Votes added by Robert Findley

        Code-Review+2

        2 comments

        File internal/refactor/inline/inline.go
        Line 683, Patchset 2 (Latest): if versions.Before(callerGoVersion, callee.GoVersion) {
        Robert Findley . unresolved

        Sorry, I cannot reason about the handling for "", and even if I figured out that it was OK (which, after 1.2 minutes of reading I'm still unclear on), I'd forget again in the future.

        Can we just change this to:
        ```
        if callerGoVersion != "" && callee.GoVersion != "" && versions.Before(callerGoVersion, calleeGoVersion) {
        ```

        So that it's obvious that it fails open?

        File internal/refactor/inline/inline_test.go
        Line 1818, Patchset 2 (Latest): if !strings.Contains(src, "package p") {

        src = "package p\n" + src
        }
        Robert Findley . unresolved

        Yuck. Do, or do not. There is no try.

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Alan Donovan
        Submit Requirements:
        • requirement satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement satisfiedReview-Enforcement
        • requirement satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: tools
        Gerrit-Branch: master
        Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
        Gerrit-Change-Number: 716560
        Gerrit-PatchSet: 2
        Gerrit-Owner: Alan Donovan <adon...@google.com>
        Gerrit-Reviewer: Alan Donovan <adon...@google.com>
        Gerrit-Reviewer: Robert Findley <rfin...@google.com>
        Gerrit-Attention: Alan Donovan <adon...@google.com>
        Gerrit-Comment-Date: Thu, 30 Oct 2025 19:58:18 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Alan Donovan (Gerrit)

        unread,
        Oct 30, 2025, 4:07:17 PM (7 days ago) Oct 30
        to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
        Attention needed from Alan Donovan

        Alan Donovan uploaded new patchset

        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
        Submit Requirements:
          • requirement satisfiedCode-Review
          • requirement is not satisfiedNo-Unresolved-Comments
          • requirement satisfiedReview-Enforcement
          • requirement is not satisfiedTryBots-Pass
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: newpatchset
          Gerrit-Project: tools
          Gerrit-Branch: master
          Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
          Gerrit-Change-Number: 716560
          Gerrit-PatchSet: 3
          satisfied_requirement
          unsatisfied_requirement
          open
          diffy

          Alan Donovan (Gerrit)

          unread,
          Oct 30, 2025, 4:07:52 PM (7 days ago) Oct 30
          to goph...@pubsubhelper.golang.org, Robert Findley, Go LUCI, golang-co...@googlegroups.com
          Attention needed from Robert Findley

          Alan Donovan added 2 comments

          File internal/refactor/inline/inline.go
          Line 683, Patchset 2: if versions.Before(callerGoVersion, callee.GoVersion) {
          Robert Findley . resolved

          Sorry, I cannot reason about the handling for "", and even if I figured out that it was OK (which, after 1.2 minutes of reading I'm still unclear on), I'd forget again in the future.

          Can we just change this to:
          ```
          if callerGoVersion != "" && callee.GoVersion != "" && versions.Before(callerGoVersion, calleeGoVersion) {
          ```

          So that it's obvious that it fails open?

          Alan Donovan

          Yeah, that's fair. It should always be populated in a production analysis driver, but many tests forget to set it. Done.

          File internal/refactor/inline/inline_test.go
          Line 1818, Patchset 2: if !strings.Contains(src, "package p") {

          src = "package p\n" + src
          }
          Robert Findley . unresolved

          Yuck. Do, or do not. There is no try.

          Alan Donovan

          Not sure what you are suggesting. Surely not making hundreds of test cases uglier just because one of them needs to put a comment before the package declaration?

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Robert Findley
          Submit Requirements:
          • requirement satisfiedCode-Review
          • requirement is not satisfiedNo-Unresolved-Comments
          • requirement satisfiedReview-Enforcement
          • requirement is not satisfiedTryBots-Pass
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: comment
          Gerrit-Project: tools
          Gerrit-Branch: master
          Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
          Gerrit-Change-Number: 716560
          Gerrit-PatchSet: 2
          Gerrit-Owner: Alan Donovan <adon...@google.com>
          Gerrit-Reviewer: Alan Donovan <adon...@google.com>
          Gerrit-Reviewer: Robert Findley <rfin...@google.com>
          Gerrit-Attention: Robert Findley <rfin...@google.com>
          Gerrit-Comment-Date: Thu, 30 Oct 2025 20:07:48 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: No
          Comment-In-Reply-To: Robert Findley <rfin...@google.com>
          satisfied_requirement
          unsatisfied_requirement
          open
          diffy

          Robert Findley (Gerrit)

          unread,
          Oct 30, 2025, 4:41:41 PM (7 days ago) Oct 30
          to Alan Donovan, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
          Attention needed from Alan Donovan

          Robert Findley voted and added 1 comment

          Votes added by Robert Findley

          Code-Review+2

          1 comment

          File internal/refactor/inline/inline_test.go
          Line 1818, Patchset 2: if !strings.Contains(src, "package p") {
          src = "package p\n" + src
          }
          Robert Findley . unresolved

          Yuck. Do, or do not. There is no try.

          Alan Donovan

          Not sure what you are suggesting. Surely not making hundreds of test cases uglier just because one of them needs to put a comment before the package declaration?

          Robert Findley

          Sorry for being flippant 😊

          No, I wasn't suggesting that: just than this type of convenient action at a distance should be avoided.

          Alternatives:
          1. `func runTests(t *testing.T, tests []testcase, opts *testOpts)` (and have a field to control the prepending of package clause).
          2. `func testOne(t *testing.T, caller, callee string, want)`, where caller and callee are full source, and call this from a new test function that passes the full source.

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Alan Donovan
          Submit Requirements:
            • requirement satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement satisfiedReview-Enforcement
            • requirement satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: tools
            Gerrit-Branch: master
            Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
            Gerrit-Change-Number: 716560
            Gerrit-PatchSet: 3
            Gerrit-Owner: Alan Donovan <adon...@google.com>
            Gerrit-Reviewer: Alan Donovan <adon...@google.com>
            Gerrit-Reviewer: Robert Findley <rfin...@google.com>
            Gerrit-Attention: Alan Donovan <adon...@google.com>
            Gerrit-Comment-Date: Thu, 30 Oct 2025 20:41:37 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: Yes
            Comment-In-Reply-To: Alan Donovan <adon...@google.com>
            Comment-In-Reply-To: Robert Findley <rfin...@google.com>
            satisfied_requirement
            unsatisfied_requirement
            open
            diffy

            Alan Donovan (Gerrit)

            unread,
            Nov 3, 2025, 5:20:57 PM (3 days ago) Nov 3
            to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
            Attention needed from Alan Donovan

            Alan Donovan uploaded new patchset

            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
            Submit Requirements:
              • requirement satisfiedCode-Review
              • requirement is not satisfiedNo-Unresolved-Comments
              • requirement satisfiedReview-Enforcement
              • requirement is not satisfiedTryBots-Pass
              Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
              Gerrit-MessageType: newpatchset
              Gerrit-Project: tools
              Gerrit-Branch: master
              Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
              Gerrit-Change-Number: 716560
              Gerrit-PatchSet: 4
              satisfied_requirement
              unsatisfied_requirement
              open
              diffy

              Alan Donovan (Gerrit)

              unread,
              Nov 3, 2025, 5:21:23 PM (3 days ago) Nov 3
              to goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, golang-co...@googlegroups.com

              Alan Donovan voted and added 1 comment

              Votes added by Alan Donovan

              Auto-Submit+1
              Commit-Queue+1

              1 comment

              File internal/refactor/inline/inline_test.go
              Line 1818, Patchset 2: if !strings.Contains(src, "package p") {
              src = "package p\n" + src
              }
              Robert Findley . resolved

              Yuck. Do, or do not. There is no try.

              Alan Donovan

              Not sure what you are suggesting. Surely not making hundreds of test cases uglier just because one of them needs to put a comment before the package declaration?

              Robert Findley

              Sorry for being flippant 😊

              No, I wasn't suggesting that: just than this type of convenient action at a distance should be avoided.

              Alternatives:
              1. `func runTests(t *testing.T, tests []testcase, opts *testOpts)` (and have a field to control the prepending of package clause).
              2. `func testOne(t *testing.T, caller, callee string, want)`, where caller and callee are full source, and call this from a new test function that passes the full source.

              Alan Donovan

              OK, I've used the options mechanism of test.descr.

              Open in Gerrit

              Related details

              Attention set is empty
              Submit Requirements:
              • requirement satisfiedCode-Review
              • requirement satisfiedNo-Unresolved-Comments
              • requirement satisfiedReview-Enforcement
              • requirement is not satisfiedTryBots-Pass
              Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
              Gerrit-MessageType: comment
              Gerrit-Project: tools
              Gerrit-Branch: master
              Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce
              Gerrit-Change-Number: 716560
              Gerrit-PatchSet: 3
              Gerrit-Owner: Alan Donovan <adon...@google.com>
              Gerrit-Reviewer: Alan Donovan <adon...@google.com>
              Gerrit-Reviewer: Robert Findley <rfin...@google.com>
              Gerrit-Comment-Date: Mon, 03 Nov 2025 22:21:18 +0000
              satisfied_requirement
              unsatisfied_requirement
              open
              diffy

              Alan Donovan (Gerrit)

              unread,
              Nov 3, 2025, 7:46:43 PM (2 days ago) Nov 3
              to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Robert Findley, golang-co...@googlegroups.com

              Alan Donovan submitted the change with unreviewed changes

              Unreviewed changes

              3 is the latest approved patch-set.
              The change was submitted with unreviewed changes in the following files:

              ```
              The name of the file: internal/refactor/inline/inline_test.go
              Insertions: 9, Deletions: 10.

              @@ -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"
               }
              @@ -385,7 +385,7 @@

               			`error: generic methods not yet supported`,
               		},
               		{
              -			"Can't inline a callee using newer Go to a caller using older Go (#75726).",
              +			"[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`,
              @@ -1813,14 +1813,6 @@

               }
               
               func runTests(t *testing.T, tests []testcase) {
              -	addPackageClause := func(src string) string {
              -		// Most test cases omit it for brevity.
              -		if !strings.Contains(src, "package p") {
              -			src = "package p\n" + src
              -		}
              -		return src
              -	}
              -

               	for _, test := range tests {
               		t.Run(test.descr, func(t *testing.T) {
               			fset := token.NewFileSet()
              @@ -1832,6 +1824,13 @@
               				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 := addPackageClause(test.callee)
               			calleeFile := mustParse("callee.go", calleeContent)
              ```

              Change information

              Commit message:
              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-by: Robert Findley <rfin...@google.com>
              Files:
              • A gopls/internal/test/marker/testdata/codeaction/inline-version.txt
              • M internal/refactor/inline/callee.go
              • M internal/refactor/inline/calleefx_test.go
              • M internal/refactor/inline/inline.go
              • M internal/refactor/inline/inline_test.go
              • M internal/refactor/inline/util.go
                Change size: M
                Delta: 6 files changed, 86 insertions(+), 15 deletions(-)
                Branch: refs/heads/master
                Submit Requirements:
                • requirement satisfiedCode-Review: +2 by Robert Findley
                • requirement satisfiedTryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
                 Open in Gerrit
                Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
                


                 Gerrit-MessageType: merged 
                

                 Gerrit-Project: tools 
                

                 Gerrit-Branch: master 
                

                 Gerrit-Change-Id: I3be25da6fe5eacdb71a340182984a166bcf8d6ce 
                

                 Gerrit-Change-Number: 716560 
                

                 Gerrit-PatchSet: 5 
                open
                diffy
                satisfied_requirement
                Reply all
                Reply to author
                Forward
                0 new messages