Skip to content

Commit ed067b1

Browse files
adonovangopherbot
authored andcommitted
internal/astutil/free: move inline.freeishNames into its own package
We'll need it in the analysis driver to remove unneeded imports as part of the attached issue. For golang/go#72035 Change-Id: I980aa9d5684558a84cb762b6ba1f5f614f994424 Reviewed-on: https://go-review.googlesource.com/c/tools/+/716641 Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent f0b71b2 commit ed067b1

File tree

3 files changed

+32
-22
lines changed

3 files changed

+32
-22
lines changed

internal/refactor/inline/free.go renamed to internal/astutil/free/free.go

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,22 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
// Copied, with considerable changes, from go/parser/resolver.go
6-
// at af53bd2c03.
7-
8-
package inline
5+
// Package free defines utilities for computing the free variables of
6+
// a syntax tree without type information. This is inherently
7+
// heuristic because of the T{f: x} ambiguity, in which f may or may
8+
// not be a lexical reference depending on whether T is a struct type.
9+
package free
910

1011
import (
1112
"go/ast"
1213
"go/token"
1314
)
1415

15-
// freeishNames computes an approximation to the free names of the AST
16-
// at node n based solely on syntax, inserting values into the map.
16+
// Copied, with considerable changes, from go/parser/resolver.go
17+
// at af53bd2c03.
18+
19+
// Names computes an approximation to the set of free names of the AST
20+
// at node n based solely on syntax.
1721
//
1822
// In the absence of composite literals, the set of free names is exact. Composite
1923
// literals introduce an ambiguity that can only be resolved with type information:
@@ -33,16 +37,22 @@ import (
3337
// - Labels are ignored: they do not refer to values.
3438
// - This is never called on FuncDecls or ImportSpecs, so the function
3539
// panics if it sees one.
36-
func freeishNames(free map[string]bool, n ast.Node, includeComplitIdents bool) {
37-
v := &freeVisitor{free: free, includeComplitIdents: includeComplitIdents}
40+
func Names(n ast.Node, includeComplitIdents bool) map[string]bool {
41+
v := &freeVisitor{
42+
free: make(map[string]bool),
43+
includeComplitIdents: includeComplitIdents,
44+
}
3845
// Begin with a scope, even though n might not be a form that establishes a scope.
3946
// For example, n might be:
4047
// x := ...
4148
// Then we need to add the first x to some scope.
4249
v.openScope()
4350
ast.Walk(v, n)
4451
v.closeScope()
45-
assert(v.scope == nil, "unbalanced scopes")
52+
if v.scope != nil {
53+
panic("unbalanced scopes")
54+
}
55+
return v.free
4656
}
4757

4858
// A freeVisitor holds state for a free-name analysis.

internal/refactor/inline/free_test.go renamed to internal/astutil/free/free_test.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
package inline
5+
package free_test
66

77
import (
88
"fmt"
@@ -13,9 +13,11 @@ import (
1313
"slices"
1414
"strings"
1515
"testing"
16+
17+
"golang.org/x/tools/internal/astutil/free"
1618
)
1719

18-
func TestFreeishNames(t *testing.T) {
20+
func TestNames(t *testing.T) {
1921
elems := func(m map[string]bool) string {
2022
return strings.Join(slices.Sorted(maps.Keys(m)), " ")
2123
}
@@ -213,13 +215,12 @@ func TestFreeishNames(t *testing.T) {
213215
for _, test := range tc.cases {
214216
_, f := mustParse(t, "free.go", `package p; func _() {`+test.code+`}`)
215217
n := f.Decls[0].(*ast.FuncDecl).Body
216-
got := map[string]bool{}
217218
want := map[string]bool{}
218219
for n := range strings.FieldsSeq(test.want) {
219220
want[n] = true
220221
}
221222

222-
freeishNames(got, n, tc.includeComplitIdents)
223+
got := free.Names(n, tc.includeComplitIdents)
223224

224225
if !maps.Equal(got, want) {
225226
t.Errorf("\ncode %s\ngot %v\nwant %v", test.code, elems(got), elems(want))
@@ -229,12 +230,12 @@ func TestFreeishNames(t *testing.T) {
229230
}
230231
}
231232

232-
func TestFreeishNamesScope(t *testing.T) {
233+
func TestFreeNames_scope(t *testing.T) {
233234
// Verify that inputs that don't start a scope don't crash.
234235
_, f := mustParse(t, "free.go", `package p; func _() { x := 1; _ = x }`)
235236
// Select the short var decl, not the entire function body.
236237
n := f.Decls[0].(*ast.FuncDecl).Body.List[0]
237-
freeishNames(map[string]bool{}, n, false)
238+
_ = free.Names(n, false)
238239
}
239240

240241
func mustParse(t *testing.T, filename string, content any) (*token.FileSet, *ast.File) {

internal/refactor/inline/inline.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"golang.org/x/tools/go/ast/astutil"
2525
"golang.org/x/tools/go/types/typeutil"
2626
internalastutil "golang.org/x/tools/internal/astutil"
27+
"golang.org/x/tools/internal/astutil/free"
2728
"golang.org/x/tools/internal/packagepath"
2829
"golang.org/x/tools/internal/typeparams"
2930
"golang.org/x/tools/internal/typesinternal"
@@ -575,9 +576,8 @@ func (i *importState) localName(pkgPath, pkgName string, shadow shadowMap) strin
575576
// Since they are not relevant to removing unused imports, we instruct
576577
// freeishNames to omit composite-literal keys that are identifiers.
577578
func trimNewImports(newImports []newImport, new ast.Node) []newImport {
578-
free := map[string]bool{}
579579
const omitComplitIdents = false
580-
freeishNames(free, new, omitComplitIdents)
580+
free := free.Names(new, omitComplitIdents)
581581
var res []newImport
582582
for _, ni := range newImports {
583583
if free[ni.pkgName] {
@@ -2388,14 +2388,13 @@ func createBindingDecl(logf logger, caller *Caller, args []*argument, calleeDecl
23882388
// (caller syntax), so we can use type info.
23892389
// But Type is the untyped callee syntax,
23902390
// so we have to use a syntax-only algorithm.
2391-
free := make(map[string]bool)
2391+
const includeComplitIdents = true
2392+
free := free.Names(spec.Type, includeComplitIdents)
23922393
for _, value := range spec.Values {
23932394
for name := range freeVars(caller.Info, value) {
23942395
free[name] = true
23952396
}
23962397
}
2397-
const includeComplitIdents = true
2398-
freeishNames(free, spec.Type, includeComplitIdents)
23992398
for name := range free {
24002399
if names[name] {
24012400
logf("binding decl would shadow free name %q", name)
@@ -3456,7 +3455,7 @@ func (st *state) assignStmts(callerStmt *ast.AssignStmt, returnOperands []ast.Ex
34563455
assert(callIdx == -1, "malformed (duplicative) AST")
34573456
callIdx = i
34583457
for j, returnOperand := range returnOperands {
3459-
freeishNames(freeNames, returnOperand, includeComplitIdents)
3458+
maps.Copy(freeNames, free.Names(returnOperand, includeComplitIdents))
34603459
rhs = append(rhs, returnOperand)
34613460
if resultInfo[j]&nonTrivialResult != 0 {
34623461
nonTrivial[i+j] = true
@@ -3469,7 +3468,7 @@ func (st *state) assignStmts(callerStmt *ast.AssignStmt, returnOperands []ast.Ex
34693468
// We must clone before clearing positions, since e came from the caller.
34703469
expr = internalastutil.CloneNode(expr)
34713470
clearPositions(expr)
3472-
freeishNames(freeNames, expr, includeComplitIdents)
3471+
maps.Copy(freeNames, free.Names(expr, includeComplitIdents))
34733472
rhs = append(rhs, expr)
34743473
}
34753474
}

0 commit comments

Comments
 (0)