Skip to content

Commit d2096d1

Browse files
adonovangopherbot
authored andcommitted
internal/analysisinternal: move more stuff out
- CanImport, IsStdPackage -> internal/packagepath, a new package for package metadata stuff. - MatchingIdents -> gopls/internal/fillreturns, shared with fillstruct. The remaining decs are all analysis related, but they straddle the driver/analyzer divide, which is not great. (A driver shouldn't per se depend on 'refactor', for example.) I propose to split the remains into internal/analysisdriver and internal/analyzerlib. Change-Id: Ia981b14becedbf874367daed91f55e0a237ae773 Reviewed-on: https://go-review.googlesource.com/c/tools/+/710795 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Alan Donovan <adonovan@google.com>
1 parent 4321df7 commit d2096d1

File tree

12 files changed

+172
-160
lines changed

12 files changed

+172
-160
lines changed

go/analysis/passes/inline/gofix.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"golang.org/x/tools/internal/analysisinternal"
2424
"golang.org/x/tools/internal/astutil"
2525
"golang.org/x/tools/internal/diff"
26+
"golang.org/x/tools/internal/packagepath"
2627
"golang.org/x/tools/internal/refactor"
2728
"golang.org/x/tools/internal/refactor/inline"
2829
"golang.org/x/tools/internal/typesinternal"
@@ -282,7 +283,7 @@ func (a *analyzer) inlineAlias(tn *types.TypeName, curId inspector.Cursor) {
282283
if obj != tn {
283284
return
284285
}
285-
} else if !analysisinternal.CanImport(a.pass.Pkg.Path(), pkgPath) {
286+
} else if !packagepath.CanImport(a.pass.Pkg.Path(), pkgPath) {
286287
// If this package can't see the package of this part of rhs, we can't inline.
287288
return
288289
} else if _, ok := importPrefixes[pkgPath]; !ok {
@@ -450,7 +451,7 @@ func (a *analyzer) inlineConst(con *types.Const, cur inspector.Cursor) {
450451
// "B" means something different here than at the inlinable const's scope.
451452
return
452453
}
453-
} else if !analysisinternal.CanImport(a.pass.Pkg.Path(), incon.RHSPkgPath) {
454+
} else if !packagepath.CanImport(a.pass.Pkg.Path(), incon.RHSPkgPath) {
454455
// If this package can't see the RHS's package, we can't inline.
455456
return
456457
}

go/analysis/passes/modernize/modernize.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ import (
1818
"golang.org/x/tools/go/analysis"
1919
"golang.org/x/tools/go/ast/edge"
2020
"golang.org/x/tools/go/ast/inspector"
21-
"golang.org/x/tools/internal/analysisinternal"
2221
"golang.org/x/tools/internal/analysisinternal/generated"
2322
"golang.org/x/tools/internal/astutil"
2423
"golang.org/x/tools/internal/moreiters"
24+
"golang.org/x/tools/internal/packagepath"
2525
"golang.org/x/tools/internal/stdlib"
2626
"golang.org/x/tools/internal/typesinternal"
2727
"golang.org/x/tools/internal/versions"
@@ -124,7 +124,7 @@ func fileUses(info *types.Info, file *ast.File, version string) bool {
124124
// specified standard packages or their dependencies.
125125
func within(pass *analysis.Pass, pkgs ...string) bool {
126126
path := pass.Pkg.Path()
127-
return analysisinternal.IsStdPackage(path) &&
127+
return packagepath.IsStdPackage(path) &&
128128
moreiters.Contains(stdlib.Dependencies(pkgs...), path)
129129
}
130130

gopls/internal/analysis/fillreturns/fillreturns.go

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"fmt"
1111
"go/ast"
1212
"go/format"
13+
"go/token"
1314
"go/types"
1415
"regexp"
1516
"slices"
@@ -112,7 +113,7 @@ outer:
112113
}
113114

114115
file, _ := cursorutil.FirstEnclosing[*ast.File](curRet)
115-
matches := analysisinternal.MatchingIdents(retTyps, file, ret.Pos(), info, pass.Pkg)
116+
matches := MatchingIdents(retTyps, file, ret.Pos(), info, pass.Pkg)
116117
qual := typesinternal.FileQualifier(file, pass.Pkg)
117118
for i, retTyp := range retTyps {
118119
var match ast.Expr
@@ -242,3 +243,95 @@ func isZeroExpr(expr ast.Expr) bool {
242243
return false
243244
}
244245
}
246+
247+
// MatchingIdents finds the names of all identifiers in 'node' that match any of the given types.
248+
// 'pos' represents the position at which the identifiers may be inserted. 'pos' must be within
249+
// the scope of each of identifier we select. Otherwise, we will insert a variable at 'pos' that
250+
// is unrecognized.
251+
//
252+
// This function is shared with the 'fillstruct' analyzer.
253+
func MatchingIdents(typs []types.Type, node ast.Node, pos token.Pos, info *types.Info, pkg *types.Package) map[types.Type][]string {
254+
255+
// Initialize matches to contain the variable types we are searching for.
256+
matches := make(map[types.Type][]string)
257+
for _, typ := range typs {
258+
if typ == nil {
259+
continue // TODO(adonovan): is this reachable?
260+
}
261+
matches[typ] = nil // create entry
262+
}
263+
264+
seen := map[types.Object]struct{}{}
265+
ast.Inspect(node, func(n ast.Node) bool {
266+
if n == nil {
267+
return false
268+
}
269+
// Prevent circular definitions. If 'pos' is within an assignment statement, do not
270+
// allow any identifiers in that assignment statement to be selected. Otherwise,
271+
// we could do the following, where 'x' satisfies the type of 'f0':
272+
//
273+
// x := fakeStruct{f0: x}
274+
//
275+
if assign, ok := n.(*ast.AssignStmt); ok && pos > assign.Pos() && pos <= assign.End() {
276+
return false
277+
}
278+
if n.End() > pos {
279+
return n.Pos() <= pos
280+
}
281+
ident, ok := n.(*ast.Ident)
282+
if !ok || ident.Name == "_" {
283+
return true
284+
}
285+
obj := info.Defs[ident]
286+
if obj == nil || obj.Type() == nil {
287+
return true
288+
}
289+
if _, ok := obj.(*types.TypeName); ok {
290+
return true
291+
}
292+
// Prevent duplicates in matches' values.
293+
if _, ok = seen[obj]; ok {
294+
return true
295+
}
296+
seen[obj] = struct{}{}
297+
// Find the scope for the given position. Then, check whether the object
298+
// exists within the scope.
299+
innerScope := pkg.Scope().Innermost(pos)
300+
if innerScope == nil {
301+
return true
302+
}
303+
_, foundObj := innerScope.LookupParent(ident.Name, pos)
304+
if foundObj != obj {
305+
return true
306+
}
307+
// The object must match one of the types that we are searching for.
308+
// TODO(adonovan): opt: use typeutil.Map?
309+
if names, ok := matches[obj.Type()]; ok {
310+
matches[obj.Type()] = append(names, ident.Name)
311+
} else {
312+
// If the object type does not exactly match
313+
// any of the target types, greedily find the first
314+
// target type that the object type can satisfy.
315+
for typ := range matches {
316+
if equivalentTypes(obj.Type(), typ) {
317+
matches[typ] = append(matches[typ], ident.Name)
318+
}
319+
}
320+
}
321+
return true
322+
})
323+
return matches
324+
}
325+
326+
func equivalentTypes(want, got types.Type) bool {
327+
if types.Identical(want, got) {
328+
return true
329+
}
330+
// Code segment to help check for untyped equality from (golang/go#32146).
331+
if rhs, ok := want.(*types.Basic); ok && rhs.Info()&types.IsUntyped > 0 {
332+
if lhs, ok := got.Underlying().(*types.Basic); ok {
333+
return rhs.Info()&types.IsConstType == lhs.Info()&types.IsConstType
334+
}
335+
}
336+
return types.AssignableTo(want, got)
337+
}

gopls/internal/analysis/fillstruct/fillstruct.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ import (
2424
"unicode"
2525

2626
"golang.org/x/tools/go/analysis"
27+
"golang.org/x/tools/gopls/internal/analysis/fillreturns"
2728
"golang.org/x/tools/gopls/internal/cache"
2829
"golang.org/x/tools/gopls/internal/cache/parsego"
2930
"golang.org/x/tools/gopls/internal/fuzzy"
3031
"golang.org/x/tools/gopls/internal/util/cursorutil"
3132
"golang.org/x/tools/gopls/internal/util/safetoken"
32-
"golang.org/x/tools/internal/analysisinternal"
3333
"golang.org/x/tools/internal/typeparams"
3434
"golang.org/x/tools/internal/typesinternal"
3535
)
@@ -183,7 +183,7 @@ func SuggestedFix(cpkg *cache.Package, pgf *parsego.File, start, end token.Pos)
183183
}
184184
fieldTyps = append(fieldTyps, field.Type())
185185
}
186-
matches := analysisinternal.MatchingIdents(fieldTyps, file, start, info, pkg)
186+
matches := fillreturns.MatchingIdents(fieldTyps, file, start, info, pkg)
187187
qual := typesinternal.FileQualifier(file, pkg)
188188

189189
for i, fieldTyp := range fieldTyps {

gopls/internal/analysis/unusedfunc/unusedfunc.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"golang.org/x/tools/internal/analysisinternal"
2020
typeindexanalyzer "golang.org/x/tools/internal/analysisinternal/typeindex"
2121
"golang.org/x/tools/internal/astutil"
22+
"golang.org/x/tools/internal/packagepath"
2223
"golang.org/x/tools/internal/typesinternal/typeindex"
2324
)
2425

@@ -70,7 +71,7 @@ var Analyzer = &analysis.Analyzer{
7071
func run(pass *analysis.Pass) (any, error) {
7172
// The standard library makes heavy use of intrinsics, linknames, etc,
7273
// that confuse this algorithm; so skip it (#74130).
73-
if analysisinternal.IsStdPackage(pass.Pkg.Path()) {
74+
if packagepath.IsStdPackage(pass.Pkg.Path()) {
7475
return nil, nil
7576
}
7677

gopls/internal/golang/completion/deep_completion.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"time"
1212

1313
"golang.org/x/tools/gopls/internal/util/typesutil"
14-
"golang.org/x/tools/internal/analysisinternal"
14+
"golang.org/x/tools/internal/packagepath"
1515
)
1616

1717
// MaxDeepCompletions limits deep completion results because in most cases
@@ -162,7 +162,7 @@ func (c *completer) deepSearch(ctx context.Context, minDepth int, deadline *time
162162
continue
163163
}
164164

165-
if cand.imp != nil && !analysisinternal.CanImport(string(c.pkg.Metadata().PkgPath), cand.imp.importPath) {
165+
if cand.imp != nil && !packagepath.CanImport(string(c.pkg.Metadata().PkgPath), cand.imp.importPath) {
166166
continue // inaccessible internal package
167167
}
168168

gopls/internal/mcp/context.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import (
2323
"golang.org/x/tools/gopls/internal/file"
2424
"golang.org/x/tools/gopls/internal/golang"
2525
"golang.org/x/tools/gopls/internal/protocol"
26-
"golang.org/x/tools/internal/analysisinternal"
2726
"golang.org/x/tools/internal/astutil"
27+
"golang.org/x/tools/internal/packagepath"
2828
)
2929

3030
type ContextParams struct {
@@ -108,7 +108,7 @@ func (h *handler) contextHandler(ctx context.Context, req *mcp.CallToolRequest,
108108
// Skip the standard library to reduce token usage, operating on
109109
// the assumption that the LLM is already familiar with its
110110
// symbols and documentation.
111-
if analysisinternal.IsStdPackage(spec.Path.Value) {
111+
if packagepath.IsStdPackage(spec.Path.Value) {
112112
continue
113113
}
114114
toSummarize = append(toSummarize, spec)

0 commit comments

Comments
 (0)