internal/lsp/source: fix cached package name matching

I made a silly mistake and checked the prefix on the import path rather
than the package name, which obviously breaks everything other than
top-level stdlib packages.

Fix that, then tweak the ranking a bit. We now get deep completions, which
is nice, but filled up the results too fast. Now instead of 5 results of
any kind, we give up after 5 packages searched.

Change-Id: I15b293f68f17531077da9ffe791a38ccc0e129f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231617
Run-TryBot: Heschi Kreinick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>

Author: heschi

internal/lsp/source/completion.go

@@ -862,7 +862,7 @@ func (c *completer) unimportedMembers(ctx context.Context, id *ast.Ident) error
 		if imports.ImportPathToAssumedName(path) != pkg.GetTypes().Name() {
 			imp.name = pkg.GetTypes().Name()
 		}
-		c.packageMembers(ctx, pkg.GetTypes(), stdScore+.1*float64(relevance), imp)
+		c.packageMembers(ctx, pkg.GetTypes(), unimportedScore(relevance), imp)
 		if len(c.items) >= unimportedMemberTarget {
 			return nil
 		}
@@ -882,7 +882,7 @@ func (c *completer) unimportedMembers(ctx context.Context, id *ast.Ident) error
 		// Continue with untyped proposals.
 		pkg := types.NewPackage(pkgExport.Fix.StmtInfo.ImportPath, pkgExport.Fix.IdentName)
 		for _, export := range pkgExport.Exports {
-			score := stdScore + 0.1*float64(pkgExport.Fix.Relevance)
+			score := unimportedScore(pkgExport.Fix.Relevance)
 			c.found(ctx, candidate{
 				obj:   types.NewVar(0, pkg, export, nil),
 				score: score,
@@ -901,6 +901,12 @@ func (c *completer) unimportedMembers(ctx context.Context, id *ast.Ident) error
 	})
 }
 
+// unimportedScore returns a score for an unimported package that is generally
+// lower than other candidates.
+func unimportedScore(relevance int) float64 {
+	return (stdScore + .1*float64(relevance)) / 2
+}
+
 func (c *completer) packageMembers(ctx context.Context, pkg *types.Package, score float64, imp *importInfo) {
 	scope := pkg.Scope()
 	for _, name := range scope.Names() {
@@ -1107,15 +1113,15 @@ func (c *completer) unimportedPackages(ctx context.Context, seen map[string]stru
 	if c.surrounding != nil {
 		prefix = c.surrounding.Prefix()
 	}
-	initialItemCount := len(c.items)
+	count := 0
 
 	known, err := c.snapshot.CachedImportPaths(ctx)
 	if err != nil {
 		return err
 	}
 	var paths []string
-	for path := range known {
-		if !strings.HasPrefix(path, prefix) {
+	for path, pkg := range known {
+		if !strings.HasPrefix(pkg.GetTypes().Name(), prefix) {
 			continue
 		}
 		paths = append(paths, path)
@@ -1131,22 +1137,25 @@ func (c *completer) unimportedPackages(ctx context.Context, seen map[string]stru
 
 	for path, relevance := range relevances {
 		pkg := known[path]
+		if _, ok := seen[pkg.GetTypes().Name()]; ok {
+			continue
+		}
 		imp := &importInfo{
 			importPath: path,
 			pkg:        pkg,
 		}
 		if imports.ImportPathToAssumedName(path) != pkg.GetTypes().Name() {
 			imp.name = pkg.GetTypes().Name()
 		}
-		score := 0.01 * float64(relevance)
+		if count >= maxUnimportedPackageNames {
+			return nil
+		}
 		c.found(ctx, candidate{
 			obj:   types.NewPkgName(0, nil, pkg.GetTypes().Name(), pkg.GetTypes()),
-			score: score,
+			score: unimportedScore(relevance),
 			imp:   imp,
 		})
-		if len(c.items)-initialItemCount >= maxUnimportedPackageNames {
-			return nil
-		}
+		count++
 	}
 
 	ctx, cancel := context.WithCancel(ctx)
@@ -1159,26 +1168,28 @@ func (c *completer) unimportedPackages(ctx context.Context, seen map[string]stru
 		if _, ok := seen[pkg.IdentName]; ok {
 			return
 		}
+		if _, ok := relevances[pkg.StmtInfo.ImportPath]; ok {
+			return
+		}
 
-		if len(c.items)-initialItemCount >= maxUnimportedPackageNames {
+		if count >= maxUnimportedPackageNames {
 			cancel()
 			return
 		}
-		// Rank unimported packages significantly lower than other results.
-		score := 0.01 * float64(pkg.Relevance)
 
 		// Do not add the unimported packages to seen, since we can have
 		// multiple packages of the same name as completion suggestions, since
 		// only one will be chosen.
 		obj := types.NewPkgName(0, nil, pkg.IdentName, types.NewPackage(pkg.StmtInfo.ImportPath, pkg.IdentName))
 		c.found(ctx, candidate{
 			obj:   obj,
-			score: score,
+			score: unimportedScore(pkg.Relevance),
 			imp: &importInfo{
 				importPath: pkg.StmtInfo.ImportPath,
 				name:       pkg.StmtInfo.Name,
 			},
 		})
+		count++
 	}
 	return c.snapshot.View().RunProcessEnvFunc(ctx, func(opts *imports.Options) error {
 		return imports.GetAllCandidates(ctx, add, prefix, c.filename, c.pkg.GetTypes().Name(), opts)

internal/lsp/testdata/lsp/primarymod/unimported/unimported.go.in

@@ -1,7 +1,7 @@
 package unimported
 
 func _() {
-	http //@unimported("p", nethttp, nethttptest)
+	http //@unimported("p", nethttp)
 	pkg  //@unimported("g", externalpackage)
 	// container/ring is extremely unlikely to be imported by anything, so shouldn't have type information.
 	ring.Ring     //@unimported("Ring", ringring)
@@ -12,7 +12,7 @@ func _() {
 
 // Create markers for unimported std lib packages. Only for use by this test.
 /* http */ //@item(nethttp, "http", "\"net/http\"", "package")
-/* httptest */ //@item(nethttptest, "httptest", "\"net/http/httptest\"", "package")
+
 /* pkg */ //@item(externalpackage, "pkg", "\"example.com/extramodule/pkg\"", "package")
 
 /* ring.Ring */ //@item(ringring, "Ring", "(from \"container/ring\")", "var")