Skip to content

Commit a039694

Browse files
h9jianggopherbot
h9jiang
authored and
gopherbot
committed
internal/typesinternal: determine package name using qualifier
- typesinternal.TypeExpr and typesinternal.ZeroExpr accept qualifier to determine the package name. - FileQualifier is moved from typesutil to typesinternal due to visibility issue. - typesinternal.FileQualifier no longer require types.Info as input. FileQualifier used to use type.Info.PkgName to map from an identifier to PkgName(Obj) and store the Obj pointer as key. Looping through ast.ImportSpec can achieve the same goal by store the pkg path as key. Change-Id: I98f2bbf9821986a9e1ed220322c86f245b91ae4c Reviewed-on: https://go-review.googlesource.com/c/tools/+/632916 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]> Auto-Submit: Hongxiang Jiang <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent a304b37 commit a039694

File tree

22 files changed

+186
-187
lines changed

22 files changed

+186
-187
lines changed

‎gopls/internal/analysis/fillreturns/fillreturns.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ outer:
155155
retTyps = append(retTyps, retTyp)
156156
}
157157
matches := analysisinternal.MatchingIdents(retTyps, file, ret.Pos(), info, pass.Pkg)
158+
qual := typesinternal.FileQualifier(file, pass.Pkg)
158159
for i, retTyp := range retTyps {
159160
var match ast.Expr
160161
var idx int
@@ -184,7 +185,7 @@ outer:
184185
// If no identifier matches the pattern, generate a zero value.
185186
if best := fuzzy.BestMatch(retTyp.String(), names); best != "" {
186187
fixed[i] = ast.NewIdent(best)
187-
} else if zero, isValid := typesinternal.ZeroExpr(file, pass.Pkg, retTyp); isValid {
188+
} else if zero, isValid := typesinternal.ZeroExpr(retTyp, qual); isValid {
188189
fixed[i] = zero
189190
} else {
190191
return nil, nil

‎gopls/internal/analysis/fillstruct/fillstruct.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ func SuggestedFix(fset *token.FileSet, start, end token.Pos, content []byte, fil
199199
fieldTyps = append(fieldTyps, field.Type())
200200
}
201201
matches := analysisinternal.MatchingIdents(fieldTyps, file, start, info, pkg)
202+
qual := typesinternal.FileQualifier(file, pkg)
202203
var elts []ast.Expr
203204
for i, fieldTyp := range fieldTyps {
204205
if fieldTyp == nil {
@@ -232,7 +233,7 @@ func SuggestedFix(fset *token.FileSet, start, end token.Pos, content []byte, fil
232233
// NOTE: We currently match on the name of the field key rather than the field type.
233234
if best := fuzzy.BestMatch(fieldName, names); best != "" {
234235
kv.Value = ast.NewIdent(best)
235-
} else if expr, isValid := populateValue(file, pkg, fieldTyp); isValid {
236+
} else if expr, isValid := populateValue(fieldTyp, qual); isValid {
236237
kv.Value = expr
237238
} else {
238239
return nil, nil, nil // no fix to suggest
@@ -328,32 +329,35 @@ func indent(str, ind []byte) []byte {
328329
//
329330
// The reasoning here is that users will call fillstruct with the intention of
330331
// initializing the struct, in which case setting these fields to nil has no effect.
331-
func populateValue(f *ast.File, pkg *types.Package, typ types.Type) (_ ast.Expr, isValid bool) {
332+
//
333+
// If the input contains an invalid type, populateValue may panic or return
334+
// expression that may not compile.
335+
func populateValue(typ types.Type, qual types.Qualifier) (_ ast.Expr, isValid bool) {
332336
switch t := typ.(type) {
333337
case *types.TypeParam, *types.Interface, *types.Struct, *types.Basic:
334-
return typesinternal.ZeroExpr(f, pkg, t)
338+
return typesinternal.ZeroExpr(t, qual)
335339

336340
case *types.Alias, *types.Named:
337341
switch t.Underlying().(type) {
338342
// Avoid typesinternal.ZeroExpr here as we don't want to return nil.
339343
case *types.Map, *types.Slice:
340344
return &ast.CompositeLit{
341-
Type: typesinternal.TypeExpr(f, pkg, t),
345+
Type: typesinternal.TypeExpr(t, qual),
342346
}, true
343347
default:
344-
return typesinternal.ZeroExpr(f, pkg, t)
348+
return typesinternal.ZeroExpr(t, qual)
345349
}
346350

347351
// Avoid typesinternal.ZeroExpr here as we don't want to return nil.
348352
case *types.Map, *types.Slice:
349353
return &ast.CompositeLit{
350-
Type: typesinternal.TypeExpr(f, pkg, t),
354+
Type: typesinternal.TypeExpr(t, qual),
351355
}, true
352356

353357
case *types.Array:
354358
return &ast.CompositeLit{
355359
Type: &ast.ArrayType{
356-
Elt: typesinternal.TypeExpr(f, pkg, t.Elem()),
360+
Elt: typesinternal.TypeExpr(t.Elem(), qual),
357361
Len: &ast.BasicLit{
358362
Kind: token.INT, Value: fmt.Sprintf("%v", t.Len()),
359363
},
@@ -370,14 +374,14 @@ func populateValue(f *ast.File, pkg *types.Package, typ types.Type) (_ ast.Expr,
370374
Args: []ast.Expr{
371375
&ast.ChanType{
372376
Dir: dir,
373-
Value: typesinternal.TypeExpr(f, pkg, t.Elem()),
377+
Value: typesinternal.TypeExpr(t.Elem(), qual),
374378
},
375379
},
376380
}, true
377381

378382
case *types.Signature:
379383
return &ast.FuncLit{
380-
Type: typesinternal.TypeExpr(f, pkg, t).(*ast.FuncType),
384+
Type: typesinternal.TypeExpr(t, qual).(*ast.FuncType),
381385
// The body of the function literal contains a panic statement to
382386
// avoid type errors.
383387
Body: &ast.BlockStmt{
@@ -425,7 +429,7 @@ func populateValue(f *ast.File, pkg *types.Package, typ types.Type) (_ ast.Expr,
425429
default:
426430
// TODO(hxjiang): & prefix only works if populateValue returns a
427431
// composite literal T{} or the expression new(T).
428-
expr, isValid := populateValue(f, pkg, t.Elem())
432+
expr, isValid := populateValue(t.Elem(), qual)
429433
return &ast.UnaryExpr{
430434
Op: token.AND,
431435
X: expr,

‎gopls/internal/golang/addtest.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -418,13 +418,13 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
418418
}
419419
}
420420

421-
// qf qualifier determines the correct package name to use for a type in
421+
// qual qualifier determines the correct package name to use for a type in
422422
// foo_test.go. It does this by:
423423
// - Consult imports map from test file foo_test.go.
424424
// - If not found, consult imports map from original file foo.go.
425425
// If the package is not imported in test file foo_test.go, it is added to
426426
// extraImports map.
427-
qf := func(p *types.Package) string {
427+
qual := func(p *types.Package) string {
428428
// References from an in-package test should not be qualified.
429429
if !xtest && p == pkg.Types() {
430430
return ""
@@ -472,8 +472,8 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
472472
}
473473

474474
data := testInfo{
475-
TestingPackageName: qf(types.NewPackage("testing", "testing")),
476-
PackageName: qf(pkg.Types()),
475+
TestingPackageName: qual(types.NewPackage("testing", "testing")),
476+
PackageName: qual(pkg.Types()),
477477
TestFuncName: testName,
478478
Func: function{
479479
Name: fn.Name(),
@@ -493,11 +493,11 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
493493
for i := range sig.Params().Len() {
494494
param := sig.Params().At(i)
495495
name, typ := param.Name(), param.Type()
496-
f := field{Type: types.TypeString(typ, qf)}
496+
f := field{Type: types.TypeString(typ, qual)}
497497
if i == 0 && isContextType(typ) {
498-
f.Value = qf(types.NewPackage("context", "context")) + ".Background()"
498+
f.Value = qual(types.NewPackage("context", "context")) + ".Background()"
499499
} else if name == "" || name == "_" {
500-
f.Value, _ = typesinternal.ZeroString(typ, qf)
500+
f.Value, _ = typesinternal.ZeroString(typ, qual)
501501
} else {
502502
f.Name = name
503503
}
@@ -516,7 +516,7 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
516516
}
517517
data.Func.Results = append(data.Func.Results, field{
518518
Name: name,
519-
Type: types.TypeString(typ, qf),
519+
Type: types.TypeString(typ, qual),
520520
})
521521
}
522522

@@ -575,7 +575,7 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
575575
data.Receiver = &receiver{
576576
Var: field{
577577
Name: varName,
578-
Type: types.TypeString(recvType, qf),
578+
Type: types.TypeString(recvType, qual),
579579
},
580580
}
581581

@@ -627,11 +627,11 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
627627
for i := range constructor.Signature().Params().Len() {
628628
param := constructor.Signature().Params().At(i)
629629
name, typ := param.Name(), param.Type()
630-
f := field{Type: types.TypeString(typ, qf)}
630+
f := field{Type: types.TypeString(typ, qual)}
631631
if i == 0 && isContextType(typ) {
632-
f.Value = qf(types.NewPackage("context", "context")) + ".Background()"
632+
f.Value = qual(types.NewPackage("context", "context")) + ".Background()"
633633
} else if name == "" || name == "_" {
634-
f.Value, _ = typesinternal.ZeroString(typ, qf)
634+
f.Value, _ = typesinternal.ZeroString(typ, qual)
635635
} else {
636636
f.Name = name
637637
}
@@ -653,7 +653,7 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
653653
}
654654
data.Receiver.Constructor.Results = append(data.Receiver.Constructor.Results, field{
655655
Name: name,
656-
Type: types.TypeString(typ, qf),
656+
Type: types.TypeString(typ, qual),
657657
})
658658
}
659659
}

‎gopls/internal/golang/codeaction.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"golang.org/x/tools/gopls/internal/protocol"
2828
"golang.org/x/tools/gopls/internal/protocol/command"
2929
"golang.org/x/tools/gopls/internal/settings"
30-
"golang.org/x/tools/gopls/internal/util/typesutil"
3130
"golang.org/x/tools/internal/event"
3231
"golang.org/x/tools/internal/imports"
3332
"golang.org/x/tools/internal/typesinternal"
@@ -318,8 +317,8 @@ func quickFix(ctx context.Context, req *codeActionsRequest) error {
318317
path, _ := astutil.PathEnclosingInterval(req.pgf.File, start, end)
319318
si := stubmethods.GetIfaceStubInfo(req.pkg.FileSet(), info, path, start)
320319
if si != nil {
321-
qf := typesutil.FileQualifier(req.pgf.File, si.Concrete.Obj().Pkg(), info)
322-
iface := types.TypeString(si.Interface.Type(), qf)
320+
qual := typesinternal.FileQualifier(req.pgf.File, si.Concrete.Obj().Pkg())
321+
iface := types.TypeString(si.Interface.Type(), qual)
323322
msg := fmt.Sprintf("Declare missing methods of %s", iface)
324323
req.addApplyFixAction(msg, fixMissingInterfaceMethods, req.loc)
325324
}

‎gopls/internal/golang/completion/completion.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ import (
4040
goplsastutil "golang.org/x/tools/gopls/internal/util/astutil"
4141
"golang.org/x/tools/gopls/internal/util/bug"
4242
"golang.org/x/tools/gopls/internal/util/safetoken"
43-
"golang.org/x/tools/gopls/internal/util/typesutil"
4443
"golang.org/x/tools/internal/event"
4544
"golang.org/x/tools/internal/imports"
4645
"golang.org/x/tools/internal/stdlib"
@@ -205,7 +204,7 @@ func (ipm insensitivePrefixMatcher) Score(candidateLabel string) float32 {
205204
type completer struct {
206205
snapshot *cache.Snapshot
207206
pkg *cache.Package
208-
qf types.Qualifier // for qualifying typed expressions
207+
qual types.Qualifier // for qualifying typed expressions
209208
mq golang.MetadataQualifier // for syntactic qualifying
210209
opts *completionOptions
211210

@@ -602,7 +601,7 @@ func Completion(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p
602601
c := &completer{
603602
pkg: pkg,
604603
snapshot: snapshot,
605-
qf: typesutil.FileQualifier(pgf.File, pkg.Types(), info),
604+
qual: typesinternal.FileQualifier(pgf.File, pkg.Types()),
606605
mq: golang.MetadataQualifierForFile(snapshot, pgf.File, pkg.Metadata()),
607606
completionContext: completionContext{
608607
triggerCharacter: protoContext.TriggerCharacter,
@@ -1768,7 +1767,7 @@ func (c *completer) injectType(ctx context.Context, t types.Type) {
17681767
// a named type whose name is literally "[]int". This allows
17691768
// us to reuse our object based completion machinery.
17701769
fakeNamedType := candidate{
1771-
obj: types.NewTypeName(token.NoPos, nil, types.TypeString(t, c.qf), t),
1770+
obj: types.NewTypeName(token.NoPos, nil, types.TypeString(t, c.qual), t),
17721771
score: stdScore,
17731772
}
17741773
// Make sure the type name matches before considering

‎gopls/internal/golang/completion/format.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e
5151

5252
var (
5353
label = cand.name
54-
detail = types.TypeString(obj.Type(), c.qf)
54+
detail = types.TypeString(obj.Type(), c.qual)
5555
insert = label
5656
kind = protocol.TextCompletion
5757
snip snippet.Builder
@@ -71,15 +71,15 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e
7171

7272
switch obj := obj.(type) {
7373
case *types.TypeName:
74-
detail, kind = golang.FormatType(obj.Type(), c.qf)
74+
detail, kind = golang.FormatType(obj.Type(), c.qual)
7575
case *types.Const:
7676
kind = protocol.ConstantCompletion
7777
case *types.Var:
7878
if _, ok := obj.Type().(*types.Struct); ok {
7979
detail = "struct{...}" // for anonymous unaliased struct types
8080
} else if obj.IsField() {
8181
var err error
82-
detail, err = golang.FormatVarType(ctx, c.snapshot, c.pkg, obj, c.qf, c.mq)
82+
detail, err = golang.FormatVarType(ctx, c.snapshot, c.pkg, obj, c.qual, c.mq)
8383
if err != nil {
8484
return CompletionItem{}, err
8585
}
@@ -128,7 +128,7 @@ Suffixes:
128128
switch mod {
129129
case invoke:
130130
if sig, ok := funcType.Underlying().(*types.Signature); ok {
131-
s, err := golang.NewSignature(ctx, c.snapshot, c.pkg, sig, nil, c.qf, c.mq)
131+
s, err := golang.NewSignature(ctx, c.snapshot, c.pkg, sig, nil, c.qual, c.mq)
132132
if err != nil {
133133
return CompletionItem{}, err
134134
}
@@ -288,7 +288,7 @@ func (c *completer) formatConversion(convertTo types.Type) conversionEdits {
288288
return conversionEdits{}
289289
}
290290

291-
typeName := types.TypeString(convertTo, c.qf)
291+
typeName := types.TypeString(convertTo, c.qual)
292292
switch t := convertTo.(type) {
293293
// We need extra parens when casting to these types. For example,
294294
// we need "(*int)(foo)", not "*int(foo)".
@@ -299,7 +299,7 @@ func (c *completer) formatConversion(convertTo types.Type) conversionEdits {
299299
// must need a conversion here. However, if the target type is untyped,
300300
// don't suggest converting to e.g. "untyped float" (golang/go#62141).
301301
if t.Info()&types.IsUntyped != 0 {
302-
typeName = types.TypeString(types.Default(convertTo), c.qf)
302+
typeName = types.TypeString(types.Default(convertTo), c.qual)
303303
}
304304
}
305305
return conversionEdits{prefix: typeName + "(", suffix: ")"}

‎gopls/internal/golang/completion/literal.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func (c *completer) literal(ctx context.Context, literalType types.Type, imp *im
7979
}
8080

8181
var (
82-
qf = c.qf
82+
qual = c.qual
8383
sel = enclosingSelector(c.path, c.pos)
8484
conversion conversionEdits
8585
)
@@ -91,20 +91,20 @@ func (c *completer) literal(ctx context.Context, literalType types.Type, imp *im
9191
// Don't qualify the type name if we are in a selector expression
9292
// since the package name is already present.
9393
if sel != nil {
94-
qf = func(_ *types.Package) string { return "" }
94+
qual = func(_ *types.Package) string { return "" }
9595
}
9696

97-
snip, typeName := c.typeNameSnippet(literalType, qf)
97+
snip, typeName := c.typeNameSnippet(literalType, qual)
9898

9999
// A type name of "[]int" doesn't work very will with the matcher
100100
// since "[" isn't a valid identifier prefix. Here we strip off the
101101
// slice (and array) prefix yielding just "int".
102102
matchName := typeName
103103
switch t := literalType.(type) {
104104
case *types.Slice:
105-
matchName = types.TypeString(t.Elem(), qf)
105+
matchName = types.TypeString(t.Elem(), qual)
106106
case *types.Array:
107-
matchName = types.TypeString(t.Elem(), qf)
107+
matchName = types.TypeString(t.Elem(), qual)
108108
}
109109

110110
addlEdits, err := c.importEdits(imp)
@@ -298,7 +298,7 @@ func (c *completer) functionLiteral(ctx context.Context, sig *types.Signature, m
298298
// of "i int, j int".
299299
if i == sig.Params().Len()-1 || !types.Identical(p.Type(), sig.Params().At(i+1).Type()) {
300300
snip.WriteText(" ")
301-
typeStr, err := golang.FormatVarType(ctx, c.snapshot, c.pkg, p, c.qf, c.mq)
301+
typeStr, err := golang.FormatVarType(ctx, c.snapshot, c.pkg, p, c.qual, c.mq)
302302
if err != nil {
303303
// In general, the only error we should encounter while formatting is
304304
// context cancellation.
@@ -356,7 +356,7 @@ func (c *completer) functionLiteral(ctx context.Context, sig *types.Signature, m
356356
snip.WriteText(name + " ")
357357
}
358358

359-
text, err := golang.FormatVarType(ctx, c.snapshot, c.pkg, r, c.qf, c.mq)
359+
text, err := golang.FormatVarType(ctx, c.snapshot, c.pkg, r, c.qual, c.mq)
360360
if err != nil {
361361
// In general, the only error we should encounter while formatting is
362362
// context cancellation.
@@ -513,7 +513,7 @@ func (c *completer) makeCall(snip *snippet.Builder, typeName string, secondArg s
513513
}
514514

515515
// Create a snippet for a type name where type params become placeholders.
516-
func (c *completer) typeNameSnippet(literalType types.Type, qf types.Qualifier) (*snippet.Builder, string) {
516+
func (c *completer) typeNameSnippet(literalType types.Type, qual types.Qualifier) (*snippet.Builder, string) {
517517
var (
518518
snip snippet.Builder
519519
typeName string
@@ -526,7 +526,7 @@ func (c *completer) typeNameSnippet(literalType types.Type, qf types.Qualifier)
526526
// Inv: pnt is not "error" or "unsafe.Pointer", so pnt.Obj() != nil and has a Pkg().
527527

528528
// We are not "fully instantiated" meaning we have type params that must be specified.
529-
if pkg := qf(pnt.Obj().Pkg()); pkg != "" {
529+
if pkg := qual(pnt.Obj().Pkg()); pkg != "" {
530530
typeName = pkg + "."
531531
}
532532

@@ -540,7 +540,7 @@ func (c *completer) typeNameSnippet(literalType types.Type, qf types.Qualifier)
540540
snip.WriteText(", ")
541541
}
542542
snip.WritePlaceholder(func(snip *snippet.Builder) {
543-
snip.WriteText(types.TypeString(tparams.At(i), qf))
543+
snip.WriteText(types.TypeString(tparams.At(i), qual))
544544
})
545545
}
546546
} else {
@@ -550,7 +550,7 @@ func (c *completer) typeNameSnippet(literalType types.Type, qf types.Qualifier)
550550
typeName += "[...]"
551551
} else {
552552
// We don't have unspecified type params so use default type formatting.
553-
typeName = types.TypeString(literalType, qf)
553+
typeName = types.TypeString(literalType, qual)
554554
snip.WriteText(typeName)
555555
}
556556

0 commit comments

Comments
 (0)