Skip to content

Commit

Permalink
internal/typesinternal: determine package name using qualifier
Browse files Browse the repository at this point in the history
- 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]>
  • Loading branch information
h9jiang authored and gopherbot committed Dec 27, 2024
1 parent a304b37 commit a039694
Show file tree
Hide file tree
Showing 22 changed files with 186 additions and 187 deletions.
3 changes: 2 additions & 1 deletion gopls/internal/analysis/fillreturns/fillreturns.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ outer:
retTyps = append(retTyps, retTyp)
}
matches := analysisinternal.MatchingIdents(retTyps, file, ret.Pos(), info, pass.Pkg)
qual := typesinternal.FileQualifier(file, pass.Pkg)
for i, retTyp := range retTyps {
var match ast.Expr
var idx int
Expand Down Expand Up @@ -184,7 +185,7 @@ outer:
// If no identifier matches the pattern, generate a zero value.
if best := fuzzy.BestMatch(retTyp.String(), names); best != "" {
fixed[i] = ast.NewIdent(best)
} else if zero, isValid := typesinternal.ZeroExpr(file, pass.Pkg, retTyp); isValid {
} else if zero, isValid := typesinternal.ZeroExpr(retTyp, qual); isValid {
fixed[i] = zero
} else {
return nil, nil
Expand Down
24 changes: 14 additions & 10 deletions gopls/internal/analysis/fillstruct/fillstruct.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ func SuggestedFix(fset *token.FileSet, start, end token.Pos, content []byte, fil
fieldTyps = append(fieldTyps, field.Type())
}
matches := analysisinternal.MatchingIdents(fieldTyps, file, start, info, pkg)
qual := typesinternal.FileQualifier(file, pkg)
var elts []ast.Expr
for i, fieldTyp := range fieldTyps {
if fieldTyp == nil {
Expand Down Expand Up @@ -232,7 +233,7 @@ func SuggestedFix(fset *token.FileSet, start, end token.Pos, content []byte, fil
// NOTE: We currently match on the name of the field key rather than the field type.
if best := fuzzy.BestMatch(fieldName, names); best != "" {
kv.Value = ast.NewIdent(best)
} else if expr, isValid := populateValue(file, pkg, fieldTyp); isValid {
} else if expr, isValid := populateValue(fieldTyp, qual); isValid {
kv.Value = expr
} else {
return nil, nil, nil // no fix to suggest
Expand Down Expand Up @@ -328,32 +329,35 @@ func indent(str, ind []byte) []byte {
//
// The reasoning here is that users will call fillstruct with the intention of
// initializing the struct, in which case setting these fields to nil has no effect.
func populateValue(f *ast.File, pkg *types.Package, typ types.Type) (_ ast.Expr, isValid bool) {
//
// If the input contains an invalid type, populateValue may panic or return
// expression that may not compile.
func populateValue(typ types.Type, qual types.Qualifier) (_ ast.Expr, isValid bool) {
switch t := typ.(type) {
case *types.TypeParam, *types.Interface, *types.Struct, *types.Basic:
return typesinternal.ZeroExpr(f, pkg, t)
return typesinternal.ZeroExpr(t, qual)

case *types.Alias, *types.Named:
switch t.Underlying().(type) {
// Avoid typesinternal.ZeroExpr here as we don't want to return nil.
case *types.Map, *types.Slice:
return &ast.CompositeLit{
Type: typesinternal.TypeExpr(f, pkg, t),
Type: typesinternal.TypeExpr(t, qual),
}, true
default:
return typesinternal.ZeroExpr(f, pkg, t)
return typesinternal.ZeroExpr(t, qual)
}

// Avoid typesinternal.ZeroExpr here as we don't want to return nil.
case *types.Map, *types.Slice:
return &ast.CompositeLit{
Type: typesinternal.TypeExpr(f, pkg, t),
Type: typesinternal.TypeExpr(t, qual),
}, true

case *types.Array:
return &ast.CompositeLit{
Type: &ast.ArrayType{
Elt: typesinternal.TypeExpr(f, pkg, t.Elem()),
Elt: typesinternal.TypeExpr(t.Elem(), qual),
Len: &ast.BasicLit{
Kind: token.INT, Value: fmt.Sprintf("%v", t.Len()),
},
Expand All @@ -370,14 +374,14 @@ func populateValue(f *ast.File, pkg *types.Package, typ types.Type) (_ ast.Expr,
Args: []ast.Expr{
&ast.ChanType{
Dir: dir,
Value: typesinternal.TypeExpr(f, pkg, t.Elem()),
Value: typesinternal.TypeExpr(t.Elem(), qual),
},
},
}, true

case *types.Signature:
return &ast.FuncLit{
Type: typesinternal.TypeExpr(f, pkg, t).(*ast.FuncType),
Type: typesinternal.TypeExpr(t, qual).(*ast.FuncType),
// The body of the function literal contains a panic statement to
// avoid type errors.
Body: &ast.BlockStmt{
Expand Down Expand Up @@ -425,7 +429,7 @@ func populateValue(f *ast.File, pkg *types.Package, typ types.Type) (_ ast.Expr,
default:
// TODO(hxjiang): & prefix only works if populateValue returns a
// composite literal T{} or the expression new(T).
expr, isValid := populateValue(f, pkg, t.Elem())
expr, isValid := populateValue(t.Elem(), qual)
return &ast.UnaryExpr{
Op: token.AND,
X: expr,
Expand Down
26 changes: 13 additions & 13 deletions gopls/internal/golang/addtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,13 +418,13 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
}
}

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

data := testInfo{
TestingPackageName: qf(types.NewPackage("testing", "testing")),
PackageName: qf(pkg.Types()),
TestingPackageName: qual(types.NewPackage("testing", "testing")),
PackageName: qual(pkg.Types()),
TestFuncName: testName,
Func: function{
Name: fn.Name(),
Expand All @@ -493,11 +493,11 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
for i := range sig.Params().Len() {
param := sig.Params().At(i)
name, typ := param.Name(), param.Type()
f := field{Type: types.TypeString(typ, qf)}
f := field{Type: types.TypeString(typ, qual)}
if i == 0 && isContextType(typ) {
f.Value = qf(types.NewPackage("context", "context")) + ".Background()"
f.Value = qual(types.NewPackage("context", "context")) + ".Background()"
} else if name == "" || name == "_" {
f.Value, _ = typesinternal.ZeroString(typ, qf)
f.Value, _ = typesinternal.ZeroString(typ, qual)
} else {
f.Name = name
}
Expand All @@ -516,7 +516,7 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
}
data.Func.Results = append(data.Func.Results, field{
Name: name,
Type: types.TypeString(typ, qf),
Type: types.TypeString(typ, qual),
})
}

Expand Down Expand Up @@ -575,7 +575,7 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
data.Receiver = &receiver{
Var: field{
Name: varName,
Type: types.TypeString(recvType, qf),
Type: types.TypeString(recvType, qual),
},
}

Expand Down Expand Up @@ -627,11 +627,11 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
for i := range constructor.Signature().Params().Len() {
param := constructor.Signature().Params().At(i)
name, typ := param.Name(), param.Type()
f := field{Type: types.TypeString(typ, qf)}
f := field{Type: types.TypeString(typ, qual)}
if i == 0 && isContextType(typ) {
f.Value = qf(types.NewPackage("context", "context")) + ".Background()"
f.Value = qual(types.NewPackage("context", "context")) + ".Background()"
} else if name == "" || name == "_" {
f.Value, _ = typesinternal.ZeroString(typ, qf)
f.Value, _ = typesinternal.ZeroString(typ, qual)
} else {
f.Name = name
}
Expand All @@ -653,7 +653,7 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
}
data.Receiver.Constructor.Results = append(data.Receiver.Constructor.Results, field{
Name: name,
Type: types.TypeString(typ, qf),
Type: types.TypeString(typ, qual),
})
}
}
Expand Down
5 changes: 2 additions & 3 deletions gopls/internal/golang/codeaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/protocol/command"
"golang.org/x/tools/gopls/internal/settings"
"golang.org/x/tools/gopls/internal/util/typesutil"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/typesinternal"
Expand Down Expand Up @@ -318,8 +317,8 @@ func quickFix(ctx context.Context, req *codeActionsRequest) error {
path, _ := astutil.PathEnclosingInterval(req.pgf.File, start, end)
si := stubmethods.GetIfaceStubInfo(req.pkg.FileSet(), info, path, start)
if si != nil {
qf := typesutil.FileQualifier(req.pgf.File, si.Concrete.Obj().Pkg(), info)
iface := types.TypeString(si.Interface.Type(), qf)
qual := typesinternal.FileQualifier(req.pgf.File, si.Concrete.Obj().Pkg())
iface := types.TypeString(si.Interface.Type(), qual)
msg := fmt.Sprintf("Declare missing methods of %s", iface)
req.addApplyFixAction(msg, fixMissingInterfaceMethods, req.loc)
}
Expand Down
7 changes: 3 additions & 4 deletions gopls/internal/golang/completion/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import (
goplsastutil "golang.org/x/tools/gopls/internal/util/astutil"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/gopls/internal/util/safetoken"
"golang.org/x/tools/gopls/internal/util/typesutil"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/stdlib"
Expand Down Expand Up @@ -205,7 +204,7 @@ func (ipm insensitivePrefixMatcher) Score(candidateLabel string) float32 {
type completer struct {
snapshot *cache.Snapshot
pkg *cache.Package
qf types.Qualifier // for qualifying typed expressions
qual types.Qualifier // for qualifying typed expressions
mq golang.MetadataQualifier // for syntactic qualifying
opts *completionOptions

Expand Down Expand Up @@ -602,7 +601,7 @@ func Completion(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p
c := &completer{
pkg: pkg,
snapshot: snapshot,
qf: typesutil.FileQualifier(pgf.File, pkg.Types(), info),
qual: typesinternal.FileQualifier(pgf.File, pkg.Types()),
mq: golang.MetadataQualifierForFile(snapshot, pgf.File, pkg.Metadata()),
completionContext: completionContext{
triggerCharacter: protoContext.TriggerCharacter,
Expand Down Expand Up @@ -1768,7 +1767,7 @@ func (c *completer) injectType(ctx context.Context, t types.Type) {
// a named type whose name is literally "[]int". This allows
// us to reuse our object based completion machinery.
fakeNamedType := candidate{
obj: types.NewTypeName(token.NoPos, nil, types.TypeString(t, c.qf), t),
obj: types.NewTypeName(token.NoPos, nil, types.TypeString(t, c.qual), t),
score: stdScore,
}
// Make sure the type name matches before considering
Expand Down
12 changes: 6 additions & 6 deletions gopls/internal/golang/completion/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e

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

switch obj := obj.(type) {
case *types.TypeName:
detail, kind = golang.FormatType(obj.Type(), c.qf)
detail, kind = golang.FormatType(obj.Type(), c.qual)
case *types.Const:
kind = protocol.ConstantCompletion
case *types.Var:
if _, ok := obj.Type().(*types.Struct); ok {
detail = "struct{...}" // for anonymous unaliased struct types
} else if obj.IsField() {
var err error
detail, err = golang.FormatVarType(ctx, c.snapshot, c.pkg, obj, c.qf, c.mq)
detail, err = golang.FormatVarType(ctx, c.snapshot, c.pkg, obj, c.qual, c.mq)
if err != nil {
return CompletionItem{}, err
}
Expand Down Expand Up @@ -128,7 +128,7 @@ Suffixes:
switch mod {
case invoke:
if sig, ok := funcType.Underlying().(*types.Signature); ok {
s, err := golang.NewSignature(ctx, c.snapshot, c.pkg, sig, nil, c.qf, c.mq)
s, err := golang.NewSignature(ctx, c.snapshot, c.pkg, sig, nil, c.qual, c.mq)
if err != nil {
return CompletionItem{}, err
}
Expand Down Expand Up @@ -288,7 +288,7 @@ func (c *completer) formatConversion(convertTo types.Type) conversionEdits {
return conversionEdits{}
}

typeName := types.TypeString(convertTo, c.qf)
typeName := types.TypeString(convertTo, c.qual)
switch t := convertTo.(type) {
// We need extra parens when casting to these types. For example,
// we need "(*int)(foo)", not "*int(foo)".
Expand All @@ -299,7 +299,7 @@ func (c *completer) formatConversion(convertTo types.Type) conversionEdits {
// must need a conversion here. However, if the target type is untyped,
// don't suggest converting to e.g. "untyped float" (golang/go#62141).
if t.Info()&types.IsUntyped != 0 {
typeName = types.TypeString(types.Default(convertTo), c.qf)
typeName = types.TypeString(types.Default(convertTo), c.qual)
}
}
return conversionEdits{prefix: typeName + "(", suffix: ")"}
Expand Down
22 changes: 11 additions & 11 deletions gopls/internal/golang/completion/literal.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (c *completer) literal(ctx context.Context, literalType types.Type, imp *im
}

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

snip, typeName := c.typeNameSnippet(literalType, qf)
snip, typeName := c.typeNameSnippet(literalType, qual)

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

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

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

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

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

Expand All @@ -540,7 +540,7 @@ func (c *completer) typeNameSnippet(literalType types.Type, qf types.Qualifier)
snip.WriteText(", ")
}
snip.WritePlaceholder(func(snip *snippet.Builder) {
snip.WriteText(types.TypeString(tparams.At(i), qf))
snip.WriteText(types.TypeString(tparams.At(i), qual))
})
}
} else {
Expand All @@ -550,7 +550,7 @@ func (c *completer) typeNameSnippet(literalType types.Type, qf types.Qualifier)
typeName += "[...]"
} else {
// We don't have unspecified type params so use default type formatting.
typeName = types.TypeString(literalType, qf)
typeName = types.TypeString(literalType, qual)
snip.WriteText(typeName)
}

Expand Down
Loading

0 comments on commit a039694

Please sign in to comment.