From 5a7a316cb6563c1dd91f4aabe7c62ab380779a45 Mon Sep 17 00:00:00 2001 From: Gavin Lam Date: Mon, 20 Oct 2025 00:19:29 -0400 Subject: [PATCH] internal/refactor/inline: improve package local name by preserving alias The existing implementation generates package local names by appending a numeric suffix to package name, which reduces readability. This improvement preserves existing import alias when available and fall back to the existing name generation mechanism if a conflict occurs. Updates golang/go#63352 Fixes golang/go#74965 Signed-off-by: Gavin Lam --- internal/refactor/inline/inline.go | 41 +++++++++------ .../refactor/inline/testdata/assignment.txtar | 6 +-- .../testdata/import-preserve-alias.txtar | 50 +++++++++++++++++++ .../refactor/inline/testdata/issue63298.txtar | 4 +- 4 files changed, 81 insertions(+), 20 deletions(-) create mode 100644 internal/refactor/inline/testdata/import-preserve-alias.txtar diff --git a/internal/refactor/inline/inline.go b/internal/refactor/inline/inline.go index 2443504da7a..e23888681de 100644 --- a/internal/refactor/inline/inline.go +++ b/internal/refactor/inline/inline.go @@ -491,14 +491,10 @@ func (i *importState) importName(pkgPath string, shadow shadowMap) string { return "" } -// localName returns the local name for a given imported package path, -// adding one if it doesn't exists. -func (i *importState) localName(pkgPath, pkgName string, shadow shadowMap) string { - // Does an import already exist that works in this shadowing context? - if name := i.importName(pkgPath, shadow); name != "" { - return name - } - +// findNewLocalName returns a new local name to use in a particular shadowing +// context. It considers the existing package alias, or construct a new alias +// based on the package name. +func (i *importState) findNewLocalName(pkgName, pkgAlias string, shadow shadowMap) string { newlyAdded := func(name string) bool { return slices.ContainsFunc(i.newImports, func(n newImport) bool { return n.pkgName == name }) } @@ -516,20 +512,35 @@ func (i *importState) localName(pkgPath, pkgName string, shadow shadowMap) strin // import added by callee // - // Choose local PkgName based on last segment of + // Try to preserve the package alias first. + // + // If that is shadowed, choose local PkgName based on last segment of // package path plus, if needed, a numeric suffix to // ensure uniqueness. // // "init" is not a legal PkgName. - // - // TODO(rfindley): is it worth preserving local package names for callee - // imports? Are they likely to be better or worse than the name we choose - // here? + if shadow[pkgAlias] == 0 && !shadowedInCaller(pkgAlias) && !newlyAdded(pkgAlias) && pkgAlias != "init" { + return pkgAlias + } + base := pkgName name := base for n := 0; shadow[name] != 0 || shadowedInCaller(name) || newlyAdded(name) || name == "init"; n++ { name = fmt.Sprintf("%s%d", base, n) } + + return name +} + +// localName returns the local name for a given imported package path, +// adding one if it doesn't exists. +func (i *importState) localName(pkgPath, pkgName, pkgAlias string, shadow shadowMap) string { + // Does an import already exist that works in this shadowing context? + if name := i.importName(pkgPath, shadow); name != "" { + return name + } + + name := i.findNewLocalName(pkgName, pkgAlias, shadow) i.logf("adding import %s %q", name, pkgPath) spec := &ast.ImportSpec{ Path: &ast.BasicLit{ @@ -1338,7 +1349,7 @@ func (st *state) renameFreeObjs(istate *importState) ([]ast.Expr, error) { var newName ast.Expr if obj.Kind == "pkgname" { // Use locally appropriate import, creating as needed. - n := istate.localName(obj.PkgPath, obj.PkgName, obj.Shadow) + n := istate.localName(obj.PkgPath, obj.PkgName, obj.Name, obj.Shadow) newName = makeIdent(n) // imported package } else if !obj.ValidPos { // Built-in function, type, or value (e.g. nil, zero): @@ -1383,7 +1394,7 @@ func (st *state) renameFreeObjs(istate *importState) ([]ast.Expr, error) { // Form a qualified identifier, pkg.Name. if qualify { - pkgName := istate.localName(obj.PkgPath, obj.PkgName, obj.Shadow) + pkgName := istate.localName(obj.PkgPath, obj.PkgName, obj.PkgName, obj.Shadow) newName = &ast.SelectorExpr{ X: makeIdent(pkgName), Sel: makeIdent(obj.Name), diff --git a/internal/refactor/inline/testdata/assignment.txtar b/internal/refactor/inline/testdata/assignment.txtar index e201d601480..6e7ffc5f2b7 100644 --- a/internal/refactor/inline/testdata/assignment.txtar +++ b/internal/refactor/inline/testdata/assignment.txtar @@ -126,11 +126,11 @@ func _() { package a import ( - "testdata/c" - c0 "testdata/c2" + c1 "testdata/c" + c2 "testdata/c2" ) func _() { - x, y := c.C(0), c0.C(1) //@ inline(re"B", b4) + x, y := c1.C(0), c2.C(1) //@ inline(re"B", b4) _, _ = x, y } diff --git a/internal/refactor/inline/testdata/import-preserve-alias.txtar b/internal/refactor/inline/testdata/import-preserve-alias.txtar new file mode 100644 index 00000000000..a8ec00b3860 --- /dev/null +++ b/internal/refactor/inline/testdata/import-preserve-alias.txtar @@ -0,0 +1,50 @@ +Test of inlining a function and preserve the imported package alias. + +-- go.mod -- +module example.com +go 1.19 + +-- main/main.go -- +package main + +import "example.com/util" + +func main() { + util.A() //@ inline(re"A", result) +} + +-- util/util.go -- +package util + +import stringutil "example.com/string/util" +import urlutil "example.com/url/util" + +func A() { + stringutil.A() + urlutil.A() +} + +-- string/util/util.go -- +package util + +func A() { +} + +-- url/util/util.go -- +package util + +func A() { +} + +-- result -- +package main + +import ( + stringutil "example.com/string/util" + urlutil "example.com/url/util" +) + +func main() { + stringutil.A() + urlutil.A() //@ inline(re"A", result) +} diff --git a/internal/refactor/inline/testdata/issue63298.txtar b/internal/refactor/inline/testdata/issue63298.txtar index e7f36351219..b1b5d0c6a4f 100644 --- a/internal/refactor/inline/testdata/issue63298.txtar +++ b/internal/refactor/inline/testdata/issue63298.txtar @@ -38,11 +38,11 @@ func B() {} package a import ( - b0 "testdata/another/b" + anotherb "testdata/another/b" "testdata/b" ) func _() { b.B() - b0.B() //@ inline(re"a2", result) + anotherb.B() //@ inline(re"a2", result) }