gopls/internal/lsp/cache: use typed strings (PackagePath et al) throughout
This changes uses the string types to enforce better hygiene
of conversions. Fishy conversions were flagged with TODO comments.
Perhaps some of these contribute to bugs in our support for vendoring.
Also, the function formerly known as ImportPath is now UnquoteImportPath.
Change-Id: Ia6bf8749908d881fb0a62cb6c98f7dd00563a69e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/449497
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/cache/analysis.go b/gopls/internal/lsp/cache/analysis.go
index e15b43e..cbb6f04 100644
--- a/gopls/internal/lsp/cache/analysis.go
+++ b/gopls/internal/lsp/cache/analysis.go
@@ -24,7 +24,7 @@
"golang.org/x/tools/internal/memoize"
)
-func (s *snapshot) Analyze(ctx context.Context, id string, analyzers []*source.Analyzer) ([]*source.Diagnostic, error) {
+func (s *snapshot) Analyze(ctx context.Context, id PackageID, analyzers []*source.Analyzer) ([]*source.Diagnostic, error) {
// TODO(adonovan): merge these two loops. There's no need to
// construct all the root action handles before beginning
// analysis. Operations should be concurrent (though that first
@@ -35,7 +35,7 @@
if !a.IsEnabled(s.view) {
continue
}
- ah, err := s.actionHandle(ctx, PackageID(id), a.Analyzer)
+ ah, err := s.actionHandle(ctx, id, a.Analyzer)
if err != nil {
return nil, err
}
@@ -416,7 +416,7 @@
for _, diag := range rawDiagnostics {
srcDiags, err := analysisDiagnosticDiagnostics(snapshot, pkg, analyzer, &diag)
if err != nil {
- event.Error(ctx, "unable to compute analysis error position", err, tag.Category.Of(diag.Category), tag.Package.Of(pkg.ID()))
+ event.Error(ctx, "unable to compute analysis error position", err, tag.Category.Of(diag.Category), tag.Package.Of(string(pkg.ID())))
continue
}
diagnostics = append(diagnostics, srcDiags...)
@@ -477,7 +477,7 @@
errorAnalyzerDiag, err = s.Analyze(ctx, pkg.ID(), analyzers)
if err != nil {
// Keep going: analysis failures should not block diagnostics.
- event.Error(ctx, "type error analysis failed", err, tag.Package.Of(pkg.ID()))
+ event.Error(ctx, "type error analysis failed", err, tag.Package.Of(string(pkg.ID())))
}
}
diags := map[span.URI][]*source.Diagnostic{}
diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go
index 17943ef..580ed0c 100644
--- a/gopls/internal/lsp/cache/check.go
+++ b/gopls/internal/lsp/cache/check.go
@@ -378,7 +378,7 @@
for _, e := range m.Errors {
diags, err := goPackagesErrorDiagnostics(snapshot, pkg, e)
if err != nil {
- event.Error(ctx, "unable to compute positions for list errors", err, tag.Package.Of(pkg.ID()))
+ event.Error(ctx, "unable to compute positions for list errors", err, tag.Package.Of(string(pkg.ID())))
continue
}
pkg.diagnostics = append(pkg.diagnostics, diags...)
@@ -400,7 +400,7 @@
for _, e := range pkg.parseErrors {
diags, err := parseErrorDiagnostics(snapshot, pkg, e)
if err != nil {
- event.Error(ctx, "unable to compute positions for parse errors", err, tag.Package.Of(pkg.ID()))
+ event.Error(ctx, "unable to compute positions for parse errors", err, tag.Package.Of(string(pkg.ID())))
continue
}
for _, diag := range diags {
@@ -418,7 +418,7 @@
for _, e := range expandErrors(unexpanded, snapshot.View().Options().RelatedInformationSupported) {
diags, err := typeErrorDiagnostics(snapshot, pkg, e)
if err != nil {
- event.Error(ctx, "unable to compute positions for type errors", err, tag.Package.Of(pkg.ID()))
+ event.Error(ctx, "unable to compute positions for type errors", err, tag.Package.Of(string(pkg.ID())))
continue
}
pkg.typeErrors = append(pkg.typeErrors, e.primary)
@@ -535,7 +535,7 @@
if !ok {
return nil, snapshot.missingPkgError(path)
}
- if !source.IsValidImport(string(m.PkgPath), string(dep.m.PkgPath)) {
+ if !source.IsValidImport(m.PkgPath, dep.m.PkgPath) {
return nil, fmt.Errorf("invalid use of internal package %s", path)
}
depPkg, err := dep.await(ctx, snapshot)
diff --git a/gopls/internal/lsp/cache/graph.go b/gopls/internal/lsp/cache/graph.go
index 1dac076..83336a2 100644
--- a/gopls/internal/lsp/cache/graph.go
+++ b/gopls/internal/lsp/cache/graph.go
@@ -93,8 +93,8 @@
}
// 2. command-line-args packages appear later.
- cli := source.IsCommandLineArguments(string(ids[i]))
- clj := source.IsCommandLineArguments(string(ids[j]))
+ cli := source.IsCommandLineArguments(ids[i])
+ clj := source.IsCommandLineArguments(ids[j])
if cli != clj {
return clj
}
@@ -121,7 +121,7 @@
}
// If we've seen *anything* prior to command-line arguments package, take
// it. Note that ids[0] may itself be command-line-arguments.
- if i > 0 && source.IsCommandLineArguments(string(id)) {
+ if i > 0 && source.IsCommandLineArguments(id) {
g.ids[uri] = ids[:i]
break
}
diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go
index 9cabffc..5250a67 100644
--- a/gopls/internal/lsp/cache/load.go
+++ b/gopls/internal/lsp/cache/load.go
@@ -50,7 +50,9 @@
for _, scope := range scopes {
switch scope := scope.(type) {
case packageLoadScope:
- if source.IsCommandLineArguments(string(scope)) {
+ // TODO(adonovan): is this cast sound?? A
+ // packageLoadScope is really a PackagePath I think.
+ if source.IsCommandLineArguments(PackageID(scope)) {
panic("attempted to load command-line-arguments")
}
// The only time we pass package paths is when we're doing a
@@ -480,10 +482,10 @@
// Allow for multiple ad-hoc packages in the workspace (see #47584).
pkgPath := PackagePath(pkg.PkgPath)
id := PackageID(pkg.ID)
- if source.IsCommandLineArguments(pkg.ID) {
+ if source.IsCommandLineArguments(id) {
suffix := ":" + strings.Join(query, ",")
- id = PackageID(string(id) + suffix)
- pkgPath = PackagePath(string(pkgPath) + suffix)
+ id = PackageID(pkg.ID + suffix)
+ pkgPath = PackagePath(pkg.PkgPath + suffix)
}
if _, ok := updates[id]; ok {
@@ -712,7 +714,7 @@
continue
}
- if source.IsCommandLineArguments(string(m.ID)) {
+ if source.IsCommandLineArguments(m.ID) {
// If all the files contained in m have a real package, we don't need to
// keep m as a workspace package.
if allFilesHaveRealPackages(meta, m) {
@@ -754,7 +756,7 @@
checkURIs:
for _, uri := range append(m.CompiledGoFiles[0:n:n], m.GoFiles...) {
for _, id := range g.ids[uri] {
- if !source.IsCommandLineArguments(string(id)) && (g.metadata[id].Valid || !m.Valid) {
+ if !source.IsCommandLineArguments(id) && (g.metadata[id].Valid || !m.Valid) {
continue checkURIs
}
}
diff --git a/gopls/internal/lsp/cache/metadata.go b/gopls/internal/lsp/cache/metadata.go
index c8b0a53..03037f0 100644
--- a/gopls/internal/lsp/cache/metadata.go
+++ b/gopls/internal/lsp/cache/metadata.go
@@ -8,19 +8,16 @@
"go/types"
"golang.org/x/tools/go/packages"
+ "golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/packagesinternal"
)
-// Declare explicit types for package paths, names, and IDs to ensure that we
-// never use an ID where a path belongs, and vice versa. If we confused these,
-// it would result in confusing errors because package IDs often look like
-// package paths.
type (
- PackageID string // go list's unique identifier for a package (e.g. "vendor/example.com/foo [vendor/example.com/bar.test]")
- PackagePath string // name used to prefix linker symbols (e.g. "vendor/example.com/foo")
- PackageName string // identifier in 'package' declaration (e.g. "foo")
- ImportPath string // path that appears in an import declaration (e.g. "example.com/foo")
+ PackageID = source.PackageID
+ PackagePath = source.PackagePath
+ PackageName = source.PackageName
+ ImportPath = source.ImportPath
)
// Metadata holds package Metadata extracted from a call to packages.Load.
@@ -43,19 +40,13 @@
}
// PackageID implements the source.Metadata interface.
-func (m *Metadata) PackageID() string {
- return string(m.ID)
-}
+func (m *Metadata) PackageID() PackageID { return m.ID }
// Name implements the source.Metadata interface.
-func (m *Metadata) PackageName() string {
- return string(m.Name)
-}
+func (m *Metadata) PackageName() PackageName { return m.Name }
// PkgPath implements the source.Metadata interface.
-func (m *Metadata) PackagePath() string {
- return string(m.PkgPath)
-}
+func (m *Metadata) PackagePath() PackagePath { return m.PkgPath }
// IsIntermediateTestVariant reports whether the given package is an
// intermediate test variant, e.g. "net/http [net/url.test]".
diff --git a/gopls/internal/lsp/cache/pkg.go b/gopls/internal/lsp/cache/pkg.go
index ddfb9ea..ef81778 100644
--- a/gopls/internal/lsp/cache/pkg.go
+++ b/gopls/internal/lsp/cache/pkg.go
@@ -36,7 +36,7 @@
analyses memoize.Store // maps analyzer.Name to Promise[actionResult]
}
-func (p *pkg) String() string { return p.ID() }
+func (p *pkg) String() string { return string(p.ID()) }
// A loadScope defines a package loading scope for use with go/packages.
type loadScope interface {
@@ -56,17 +56,9 @@
func (moduleLoadScope) aScope() {}
func (viewLoadScope) aScope() {}
-func (p *pkg) ID() string {
- return string(p.m.ID)
-}
-
-func (p *pkg) Name() string {
- return string(p.m.Name)
-}
-
-func (p *pkg) PkgPath() string {
- return string(p.m.PkgPath)
-}
+func (p *pkg) ID() PackageID { return p.m.ID }
+func (p *pkg) Name() PackageName { return p.m.Name }
+func (p *pkg) PkgPath() PackagePath { return p.m.PkgPath }
func (p *pkg) ParseMode() source.ParseMode {
return p.mode
@@ -118,8 +110,8 @@
// given its PackagePath. (If you have an ImportPath, e.g. a string
// from an import declaration, use ResolveImportPath instead.
// They may differ in case of vendoring.)
-func (p *pkg) DirectDep(pkgPath string) (source.Package, error) {
- if id, ok := p.m.DepsByPkgPath[PackagePath(pkgPath)]; ok {
+func (p *pkg) DirectDep(pkgPath PackagePath) (source.Package, error) {
+ if id, ok := p.m.DepsByPkgPath[pkgPath]; ok {
if imp := p.deps[id]; imp != nil {
return imp, nil
}
@@ -129,8 +121,8 @@
// ResolveImportPath returns the directly imported dependency of this package,
// given its ImportPath. See also DirectDep.
-func (p *pkg) ResolveImportPath(importPath string) (source.Package, error) {
- if id, ok := p.m.DepsByImpPath[ImportPath(importPath)]; ok && id != "" {
+func (p *pkg) ResolveImportPath(importPath ImportPath) (source.Package, error) {
+ if id, ok := p.m.DepsByImpPath[importPath]; ok && id != "" {
if imp := p.deps[id]; imp != nil {
return imp, nil
}
@@ -138,7 +130,7 @@
return nil, fmt.Errorf("package does not import %s", importPath)
}
-func (p *pkg) MissingDependencies() []string {
+func (p *pkg) MissingDependencies() []ImportPath {
// We don't invalidate metadata for import deletions,
// so check the package imports via the *types.Package.
//
@@ -162,13 +154,14 @@
// should be fast) then we can simply return the blank entries
// in DepsByImpPath. (They are PackageIDs not PackagePaths,
// but the caller only cares whether the set is empty!)
- var missing []string
+ var missing []ImportPath
for _, pkg := range p.types.Imports() {
- if id, ok := p.m.DepsByImpPath[ImportPath(pkg.Path())]; ok && id == "" {
- missing = append(missing, pkg.Path())
+ importPath := ImportPath(pkg.Path())
+ if id, ok := p.m.DepsByImpPath[importPath]; ok && id == "" {
+ missing = append(missing, importPath)
}
}
- sort.Strings(missing)
+ sort.Slice(missing, func(i, j int) bool { return missing[i] < missing[j] })
return missing
}
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index d6599b1..f280304 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -811,17 +811,17 @@
return s.view.goversion >= 13 && s.view.Options().ExperimentalUseInvalidMetadata
}
-func (s *snapshot) GetReverseDependencies(ctx context.Context, id string) ([]source.Package, error) {
+func (s *snapshot) GetReverseDependencies(ctx context.Context, id PackageID) ([]source.Package, error) {
if err := s.awaitLoaded(ctx); err != nil {
return nil, err
}
s.mu.Lock()
meta := s.meta
s.mu.Unlock()
- ids := meta.reverseTransitiveClosure(s.useInvalidMetadata(), PackageID(id))
+ ids := meta.reverseTransitiveClosure(s.useInvalidMetadata(), id)
// Make sure to delete the original package ID from the map.
- delete(ids, PackageID(id))
+ delete(ids, id)
var pkgs []source.Package
for id := range ids {
@@ -1212,12 +1212,11 @@
return meta, nil
}
-func (s *snapshot) WorkspacePackageByID(ctx context.Context, id string) (source.Package, error) {
- packageID := PackageID(id)
- return s.checkedPackage(ctx, packageID, s.workspaceParseMode(packageID))
+func (s *snapshot) WorkspacePackageByID(ctx context.Context, id PackageID) (source.Package, error) {
+ return s.checkedPackage(ctx, id, s.workspaceParseMode(id))
}
-func (s *snapshot) CachedImportPaths(ctx context.Context) (map[string]source.Package, error) {
+func (s *snapshot) CachedImportPaths(ctx context.Context) (map[PackagePath]source.Package, error) {
// Don't reload workspace package metadata.
// This function is meant to only return currently cached information.
s.AwaitInitialized(ctx)
@@ -1225,7 +1224,7 @@
s.mu.Lock()
defer s.mu.Unlock()
- results := map[string]source.Package{}
+ results := map[PackagePath]source.Package{}
s.packages.Range(func(_, v interface{}) {
cachedPkg, err := v.(*packageHandle).cached()
if err != nil {
@@ -1983,7 +1982,7 @@
// For metadata that has been newly invalidated, capture package paths
// requiring reloading in the shouldLoad map.
- if invalidateMetadata && !source.IsCommandLineArguments(string(v.ID)) {
+ if invalidateMetadata && !source.IsCommandLineArguments(v.ID) {
if result.shouldLoad == nil {
result.shouldLoad = make(map[PackageID][]PackagePath)
}
diff --git a/gopls/internal/lsp/command.go b/gopls/internal/lsp/command.go
index 9722b78..3b0d3b8 100644
--- a/gopls/internal/lsp/command.go
+++ b/gopls/internal/lsp/command.go
@@ -735,8 +735,10 @@
progress: "Listing packages",
forURI: args.URI,
}, func(ctx context.Context, deps commandDeps) error {
- var err error
- result.Packages, err = source.KnownPackages(ctx, deps.snapshot, deps.fh)
+ pkgs, err := source.KnownPackages(ctx, deps.snapshot, deps.fh)
+ for _, pkg := range pkgs {
+ result.Packages = append(result.Packages, string(pkg))
+ }
return err
})
return result, err
@@ -765,14 +767,14 @@
name = imp.Name.Name
}
result.Imports = append(result.Imports, command.FileImport{
- Path: source.ImportPath(imp),
+ Path: string(source.UnquoteImportPath(imp)),
Name: name,
})
}
}
for _, imp := range pkg.Imports() {
result.PackageImports = append(result.PackageImports, command.PackageImport{
- Path: imp.PkgPath(), // This might be the vendored path under GOPATH vendoring, in which case it's a bug.
+ Path: string(imp.PkgPath()), // This might be the vendored path under GOPATH vendoring, in which case it's a bug.
})
}
sort.Slice(result.PackageImports, func(i, j int) bool {
diff --git a/gopls/internal/lsp/diagnostics.go b/gopls/internal/lsp/diagnostics.go
index a01898b..bbc57ff 100644
--- a/gopls/internal/lsp/diagnostics.go
+++ b/gopls/internal/lsp/diagnostics.go
@@ -346,7 +346,7 @@
}
func (s *Server) diagnosePkg(ctx context.Context, snapshot source.Snapshot, pkg source.Package, alwaysAnalyze bool) {
- ctx, done := event.Start(ctx, "Server.diagnosePkg", tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(pkg.ID()))
+ ctx, done := event.Start(ctx, "Server.diagnosePkg", tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(string(pkg.ID())))
defer done()
enableDiagnostics := false
includeAnalysis := alwaysAnalyze // only run analyses for packages with open files
@@ -361,7 +361,7 @@
pkgDiagnostics, err := snapshot.DiagnosePackage(ctx, pkg)
if err != nil {
- event.Error(ctx, "warning: diagnosing package", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(pkg.ID()))
+ event.Error(ctx, "warning: diagnosing package", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(string(pkg.ID())))
return
}
for _, cgf := range pkg.CompiledGoFiles() {
@@ -374,7 +374,7 @@
if includeAnalysis && !pkg.HasListOrParseErrors() {
reports, err := source.Analyze(ctx, snapshot, pkg, false)
if err != nil {
- event.Error(ctx, "warning: analyzing package", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(pkg.ID()))
+ event.Error(ctx, "warning: analyzing package", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(string(pkg.ID())))
return
}
for _, cgf := range pkg.CompiledGoFiles() {
@@ -390,7 +390,7 @@
if enableGCDetails {
gcReports, err := source.GCOptimizationDetails(ctx, snapshot, pkg)
if err != nil {
- event.Error(ctx, "warning: gc details", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(pkg.ID()))
+ event.Error(ctx, "warning: gc details", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(string(pkg.ID())))
}
s.gcOptimizationDetailsMu.Lock()
_, enableGCDetails := s.gcOptimizationDetails[pkg.ID()]
diff --git a/gopls/internal/lsp/link.go b/gopls/internal/lsp/link.go
index b26bfb3..7eb561f 100644
--- a/gopls/internal/lsp/link.go
+++ b/gopls/internal/lsp/link.go
@@ -12,7 +12,6 @@
"go/token"
"net/url"
"regexp"
- "strconv"
"strings"
"sync"
@@ -133,21 +132,21 @@
// https://pkg.go.dev.
if view.Options().ImportShortcut.ShowLinks() {
for _, imp := range imports {
- target, err := strconv.Unquote(imp.Path.Value)
- if err != nil {
+ target := source.UnquoteImportPath(imp)
+ if target == "" {
continue
}
// See golang/go#36998: don't link to modules matching GOPRIVATE.
- if view.IsGoPrivatePath(target) {
+ if view.IsGoPrivatePath(string(target)) {
continue
}
if mod, version, ok := moduleAtVersion(target, pkg); ok && strings.ToLower(view.Options().LinkTarget) == "pkg.go.dev" {
- target = strings.Replace(target, mod, mod+"@"+version, 1)
+ target = source.ImportPath(strings.Replace(string(target), mod, mod+"@"+version, 1))
}
// Account for the quotation marks in the positions.
start := imp.Path.Pos() + 1
end := imp.Path.End() - 1
- targetURL := source.BuildLink(view.Options().LinkTarget, target, "")
+ targetURL := source.BuildLink(view.Options().LinkTarget, string(target), "")
l, err := toProtocolLink(pgf.Tok, pgf.Mapper, targetURL, start, end)
if err != nil {
return nil, err
@@ -174,7 +173,7 @@
return links, nil
}
-func moduleAtVersion(targetImportPath string, pkg source.Package) (string, string, bool) {
+func moduleAtVersion(targetImportPath source.ImportPath, pkg source.Package) (string, string, bool) {
impPkg, err := pkg.ResolveImportPath(targetImportPath)
if err != nil {
return "", "", false
diff --git a/gopls/internal/lsp/semantic.go b/gopls/internal/lsp/semantic.go
index a8b38f0..75f88b3 100644
--- a/gopls/internal/lsp/semantic.go
+++ b/gopls/internal/lsp/semantic.go
@@ -15,7 +15,6 @@
"log"
"path/filepath"
"sort"
- "strconv"
"strings"
"time"
@@ -905,8 +904,8 @@
}
return // don't mark anything for . or _
}
- importPath, err := strconv.Unquote(d.Path.Value)
- if err != nil {
+ importPath := source.UnquoteImportPath(d)
+ if importPath == "" {
return
}
// Import strings are implementation defined. Try to match with parse information.
@@ -916,7 +915,7 @@
return
}
// Check whether the original literal contains the package's declared name.
- j := strings.LastIndex(d.Path.Value, imported.Name())
+ j := strings.LastIndex(d.Path.Value, string(imported.Name()))
if j == -1 {
// name doesn't show up, for whatever reason, so nothing to report
return
diff --git a/gopls/internal/lsp/server.go b/gopls/internal/lsp/server.go
index 693afae..9f70591 100644
--- a/gopls/internal/lsp/server.go
+++ b/gopls/internal/lsp/server.go
@@ -27,7 +27,7 @@
session.SetProgressTracker(tracker)
return &Server{
diagnostics: map[span.URI]*fileReports{},
- gcOptimizationDetails: make(map[string]struct{}),
+ gcOptimizationDetails: make(map[source.PackageID]struct{}),
watchedGlobPatterns: make(map[string]struct{}),
changedFiles: make(map[span.URI]struct{}),
session: session,
@@ -97,7 +97,7 @@
// optimization details to be included in the diagnostics. The key is the
// ID of the package.
gcOptimizationDetailsMu sync.Mutex
- gcOptimizationDetails map[string]struct{}
+ gcOptimizationDetails map[source.PackageID]struct{}
// diagnosticsSema limits the concurrency of diagnostics runs, which can be
// expensive.
diff --git a/gopls/internal/lsp/source/completion/completion.go b/gopls/internal/lsp/source/completion/completion.go
index c3b7c2b..1f8dd92 100644
--- a/gopls/internal/lsp/source/completion/completion.go
+++ b/gopls/internal/lsp/source/completion/completion.go
@@ -1089,7 +1089,7 @@
if pkgName, ok := c.pkg.GetTypesInfo().Uses[id].(*types.PkgName); ok {
var pkg source.Package
for _, imp := range c.pkg.Imports() {
- if imp.PkgPath() == pkgName.Imported().Path() {
+ if imp.PkgPath() == source.PackagePath(pkgName.Imported().Path()) {
pkg = imp
}
}
@@ -1140,7 +1140,7 @@
if pkg.GetTypes().Name() != id.Name {
continue
}
- paths = append(paths, path)
+ paths = append(paths, string(path))
}
var relevances map[string]float64
@@ -1158,7 +1158,7 @@
})
for _, path := range paths {
- pkg := known[path]
+ pkg := known[source.PackagePath(path)]
if pkg.GetTypes().Name() != id.Name {
continue
}
@@ -1182,7 +1182,8 @@
add := func(pkgExport imports.PackageExport) {
mu.Lock()
defer mu.Unlock()
- if _, ok := known[pkgExport.Fix.StmtInfo.ImportPath]; ok {
+ // TODO(adonovan): what if the actual package has a vendor/ prefix?
+ if _, ok := known[source.PackagePath(pkgExport.Fix.StmtInfo.ImportPath)]; ok {
return // We got this one above.
}
@@ -1379,7 +1380,8 @@
// Make sure the package name isn't already in use by another
// object, and that this file doesn't import the package yet.
- if _, ok := seen[pkg.Name()]; !ok && pkg != c.pkg.GetTypes() && !alreadyImports(c.file, pkg.Path()) {
+ // TODO(adonovan): what if pkg.Path has vendor/ prefix?
+ if _, ok := seen[pkg.Name()]; !ok && pkg != c.pkg.GetTypes() && !alreadyImports(c.file, source.ImportPath(pkg.Path())) {
seen[pkg.Name()] = struct{}{}
obj := types.NewPkgName(0, nil, pkg.Name(), pkg)
imp := &importInfo{
@@ -1481,12 +1483,12 @@
if err != nil {
return err
}
- var paths []string
+ var paths []string // actually PackagePaths
for path, pkg := range known {
if !strings.HasPrefix(pkg.GetTypes().Name(), prefix) {
continue
}
- paths = append(paths, path)
+ paths = append(paths, string(path))
}
var relevances map[string]float64
@@ -1511,7 +1513,7 @@
})
for _, path := range paths {
- pkg := known[path]
+ pkg := known[source.PackagePath(path)]
if _, ok := seen[pkg.GetTypes().Name()]; ok {
continue
}
@@ -1526,7 +1528,7 @@
}
c.deepState.enqueue(candidate{
// Pass an empty *types.Package to disable deep completions.
- obj: types.NewPkgName(0, nil, pkg.GetTypes().Name(), types.NewPackage(path, pkg.Name())),
+ obj: types.NewPkgName(0, nil, pkg.GetTypes().Name(), types.NewPackage(path, string(pkg.Name()))),
score: unimportedScore(relevances[path]),
imp: imp,
})
@@ -1573,9 +1575,9 @@
}
// alreadyImports reports whether f has an import with the specified path.
-func alreadyImports(f *ast.File, path string) bool {
+func alreadyImports(f *ast.File, path source.ImportPath) bool {
for _, s := range f.Imports {
- if source.ImportPath(s) == path {
+ if source.UnquoteImportPath(s) == path {
return true
}
}
diff --git a/gopls/internal/lsp/source/completion/package.go b/gopls/internal/lsp/source/completion/package.go
index b7fad0f..1b7c731 100644
--- a/gopls/internal/lsp/source/completion/package.go
+++ b/gopls/internal/lsp/source/completion/package.go
@@ -240,7 +240,7 @@
}
pkgName := convertDirNameToPkgName(dirName)
- seenPkgs := make(map[string]struct{})
+ seenPkgs := make(map[source.PackageName]struct{})
// The `go` command by default only allows one package per directory but we
// support multiple package suggestions since gopls is build system agnostic.
@@ -267,30 +267,30 @@
// Add a found package used in current directory as a high relevance
// suggestion and the test package for it as a medium relevance
// suggestion.
- if score := float64(matcher.Score(pkg.Name())); score > 0 {
- packages = append(packages, toCandidate(pkg.Name(), score*highScore))
+ if score := float64(matcher.Score(string(pkg.Name()))); score > 0 {
+ packages = append(packages, toCandidate(string(pkg.Name()), score*highScore))
}
seenPkgs[pkg.Name()] = struct{}{}
testPkgName := pkg.Name() + "_test"
- if _, ok := seenPkgs[testPkgName]; ok || strings.HasSuffix(pkg.Name(), "_test") {
+ if _, ok := seenPkgs[testPkgName]; ok || strings.HasSuffix(string(pkg.Name()), "_test") {
continue
}
- if score := float64(matcher.Score(testPkgName)); score > 0 {
- packages = append(packages, toCandidate(testPkgName, score*stdScore))
+ if score := float64(matcher.Score(string(testPkgName))); score > 0 {
+ packages = append(packages, toCandidate(string(testPkgName), score*stdScore))
}
seenPkgs[testPkgName] = struct{}{}
}
// Add current directory name as a low relevance suggestion.
if _, ok := seenPkgs[pkgName]; !ok {
- if score := float64(matcher.Score(pkgName)); score > 0 {
- packages = append(packages, toCandidate(pkgName, score*lowScore))
+ if score := float64(matcher.Score(string(pkgName))); score > 0 {
+ packages = append(packages, toCandidate(string(pkgName), score*lowScore))
}
testPkgName := pkgName + "_test"
- if score := float64(matcher.Score(testPkgName)); score > 0 {
- packages = append(packages, toCandidate(testPkgName, score*lowScore))
+ if score := float64(matcher.Score(string(testPkgName))); score > 0 {
+ packages = append(packages, toCandidate(string(testPkgName), score*lowScore))
}
}
@@ -330,7 +330,7 @@
// convertDirNameToPkgName converts a valid directory name to a valid package name.
// It leaves only letters and digits. All letters are mapped to lower case.
-func convertDirNameToPkgName(dirName string) string {
+func convertDirNameToPkgName(dirName string) source.PackageName {
var buf bytes.Buffer
for _, ch := range dirName {
switch {
@@ -341,7 +341,7 @@
buf.WriteRune(ch)
}
}
- return buf.String()
+ return source.PackageName(buf.String())
}
// isLetter and isDigit allow only ASCII characters because
diff --git a/gopls/internal/lsp/source/completion/package_test.go b/gopls/internal/lsp/source/completion/package_test.go
index 6436984..614359f 100644
--- a/gopls/internal/lsp/source/completion/package_test.go
+++ b/gopls/internal/lsp/source/completion/package_test.go
@@ -4,7 +4,11 @@
package completion
-import "testing"
+import (
+ "testing"
+
+ "golang.org/x/tools/gopls/internal/lsp/source"
+)
func TestIsValidDirName(t *testing.T) {
tests := []struct {
@@ -51,7 +55,7 @@
func TestConvertDirNameToPkgName(t *testing.T) {
tests := []struct {
dirName string
- pkgName string
+ pkgName source.PackageName
}{
{dirName: "a", pkgName: "a"},
{dirName: "abcdef", pkgName: "abcdef"},
diff --git a/gopls/internal/lsp/source/completion/postfix_snippets.go b/gopls/internal/lsp/source/completion/postfix_snippets.go
index 414db42..3c805a7 100644
--- a/gopls/internal/lsp/source/completion/postfix_snippets.go
+++ b/gopls/internal/lsp/source/completion/postfix_snippets.go
@@ -16,11 +16,11 @@
"sync"
"text/template"
- "golang.org/x/tools/internal/event"
- "golang.org/x/tools/internal/imports"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/snippet"
"golang.org/x/tools/gopls/internal/lsp/source"
+ "golang.org/x/tools/internal/event"
+ "golang.org/x/tools/internal/imports"
)
// Postfix snippets are artificial methods that allow the user to
@@ -442,7 +442,8 @@
// Check if file already imports pkgPath.
for _, s := range c.file.Imports {
- if source.ImportPath(s) == pkgPath {
+ // TODO(adonovan): what if pkgPath has a vendor/ prefix?
+ if source.UnquoteImportPath(s) == source.ImportPath(pkgPath) {
if s.Name == nil {
return defaultName, nil, nil
}
diff --git a/gopls/internal/lsp/source/hover.go b/gopls/internal/lsp/source/hover.go
index 09f7224..0bd5a30 100644
--- a/gopls/internal/lsp/source/hover.go
+++ b/gopls/internal/lsp/source/hover.go
@@ -448,7 +448,7 @@
if strings.ToLower(i.Snapshot.View().Options().LinkTarget) != "pkg.go.dev" {
return "", "", false
}
- impPkg, err := i.pkg.DirectDep(path)
+ impPkg, err := i.pkg.DirectDep(PackagePath(path))
if err != nil {
return "", "", false
}
@@ -539,7 +539,7 @@
if err != nil {
return nil, err
}
- imp, err := pkg.ResolveImportPath(importPath)
+ imp, err := pkg.ResolveImportPath(ImportPath(importPath))
if err != nil {
return nil, err
}
diff --git a/gopls/internal/lsp/source/identifier.go b/gopls/internal/lsp/source/identifier.go
index f11817f..526c108 100644
--- a/gopls/internal/lsp/source/identifier.go
+++ b/gopls/internal/lsp/source/identifier.go
@@ -456,7 +456,7 @@
if err != nil {
return nil, fmt.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err)
}
- imported, err := pkg.ResolveImportPath(importPath)
+ imported, err := pkg.ResolveImportPath(ImportPath(importPath))
if err != nil {
return nil, err
}
diff --git a/gopls/internal/lsp/source/implementation.go b/gopls/internal/lsp/source/implementation.go
index 2da488d..87f4351 100644
--- a/gopls/internal/lsp/source/implementation.go
+++ b/gopls/internal/lsp/source/implementation.go
@@ -323,7 +323,7 @@
// Look up the implicit *types.PkgName.
obj := searchpkg.GetTypesInfo().Implicits[leaf]
if obj == nil {
- return nil, fmt.Errorf("%w for import %q", errNoObjectFound, ImportPath(leaf))
+ return nil, fmt.Errorf("%w for import %s", errNoObjectFound, UnquoteImportPath(leaf))
}
objs = append(objs, obj)
}
diff --git a/gopls/internal/lsp/source/known_packages.go b/gopls/internal/lsp/source/known_packages.go
index e2d950b..efaaee5 100644
--- a/gopls/internal/lsp/source/known_packages.go
+++ b/gopls/internal/lsp/source/known_packages.go
@@ -19,14 +19,15 @@
// KnownPackages returns a list of all known packages
// in the package graph that could potentially be imported
// by the given file.
-func KnownPackages(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle) ([]string, error) {
+func KnownPackages(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle) ([]PackagePath, error) {
pkg, pgf, err := GetParsedFile(ctx, snapshot, fh, NarrowestPackage)
if err != nil {
return nil, fmt.Errorf("GetParsedFile: %w", err)
}
- alreadyImported := map[string]struct{}{}
+ alreadyImported := map[PackagePath]struct{}{}
for _, imp := range pgf.File.Imports {
- alreadyImported[imp.Path.Value] = struct{}{}
+ // TODO(adonovan): the correct PackagePath might need a "vendor/" prefix.
+ alreadyImported[PackagePath(imp.Path.Value)] = struct{}{}
}
// TODO(adonovan): this whole algorithm could be more
// simply expressed in terms of Metadata, not Packages.
@@ -35,8 +36,8 @@
return nil, err
}
var (
- seen = make(map[string]struct{})
- paths []string
+ seen = make(map[PackagePath]struct{})
+ paths []PackagePath
)
for path, knownPkg := range pkgs {
gofiles := knownPkg.CompiledGoFiles()
@@ -79,10 +80,12 @@
return imports.GetAllCandidates(ctx, func(ifix imports.ImportFix) {
mu.Lock()
defer mu.Unlock()
- if _, ok := seen[ifix.StmtInfo.ImportPath]; ok {
+ // TODO(adonovan): what if the actual package path has a vendor/ prefix?
+ path := PackagePath(ifix.StmtInfo.ImportPath)
+ if _, ok := seen[path]; ok {
return
}
- paths = append(paths, ifix.StmtInfo.ImportPath)
+ paths = append(paths, path)
}, "", pgf.URI.Filename(), pkg.GetTypes().Name(), o.Env)
})
if err != nil {
@@ -92,8 +95,8 @@
}
sort.Slice(paths, func(i, j int) bool {
importI, importJ := paths[i], paths[j]
- iHasDot := strings.Contains(importI, ".")
- jHasDot := strings.Contains(importJ, ".")
+ iHasDot := strings.Contains(string(importI), ".")
+ jHasDot := strings.Contains(string(importJ), ".")
if iHasDot && !jHasDot {
return false
}
diff --git a/gopls/internal/lsp/source/references.go b/gopls/internal/lsp/source/references.go
index e26091c..4db716e 100644
--- a/gopls/internal/lsp/source/references.go
+++ b/gopls/internal/lsp/source/references.go
@@ -78,7 +78,7 @@
for _, dep := range rdeps {
for _, f := range dep.CompiledGoFiles() {
for _, imp := range f.File.Imports {
- if path, err := strconv.Unquote(imp.Path.Value); err == nil && path == renamingPkg.PkgPath() {
+ if path, err := strconv.Unquote(imp.Path.Value); err == nil && path == string(renamingPkg.PkgPath()) {
refs = append(refs, &ReferenceInfo{
Name: packageName,
MappedRange: NewMappedRange(f.Tok, f.Mapper, imp.Pos(), imp.End()),
diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go
index 2d4a188..3a8d7cf 100644
--- a/gopls/internal/lsp/source/rename.go
+++ b/gopls/internal/lsp/source/rename.go
@@ -93,7 +93,7 @@
return nil, err, err
}
- if strings.HasSuffix(meta.PackageName(), "_test") {
+ if strings.HasSuffix(string(meta.PackageName()), "_test") {
err := errors.New("can't rename x_test packages")
return nil, err, err
}
@@ -103,7 +103,7 @@
return nil, err, err
}
- if meta.ModuleInfo().Path == meta.PackagePath() {
+ if meta.ModuleInfo().Path == string(meta.PackagePath()) {
err := fmt.Errorf("can't rename package: package path %q is the same as module path %q", meta.PackagePath(), meta.ModuleInfo().Path)
return nil, err, err
}
@@ -113,7 +113,7 @@
err = fmt.Errorf("error building package to rename: %v", err)
return nil, err, err
}
- result, err := computePrepareRenameResp(snapshot, pkg, pgf.File.Name, pkg.Name())
+ result, err := computePrepareRenameResp(snapshot, pkg, pgf.File.Name, string(pkg.Name()))
if err != nil {
return nil, nil, err
}
@@ -202,11 +202,11 @@
// Fix this.
meta := fileMeta[0]
oldPath := meta.PackagePath()
- var modulePath string
+ var modulePath PackagePath
if mi := meta.ModuleInfo(); mi == nil {
return nil, true, fmt.Errorf("cannot rename package: missing module information for package %q", meta.PackagePath())
} else {
- modulePath = mi.Path
+ modulePath = PackagePath(mi.Path)
}
if strings.HasSuffix(newName, "_test") {
@@ -218,7 +218,7 @@
return nil, true, err
}
- renamingEdits, err := renamePackage(ctx, s, modulePath, oldPath, newName, metadata)
+ renamingEdits, err := renamePackage(ctx, s, modulePath, oldPath, PackageName(newName), metadata)
if err != nil {
return nil, true, err
}
@@ -245,12 +245,12 @@
// It updates package clauses and import paths for the renamed package as well
// as any other packages affected by the directory renaming among packages
// described by allMetadata.
-func renamePackage(ctx context.Context, s Snapshot, modulePath, oldPath, newName string, allMetadata []Metadata) (map[span.URI][]protocol.TextEdit, error) {
+func renamePackage(ctx context.Context, s Snapshot, modulePath, oldPath PackagePath, newName PackageName, allMetadata []Metadata) (map[span.URI][]protocol.TextEdit, error) {
if modulePath == oldPath {
return nil, fmt.Errorf("cannot rename package: module path %q is the same as the package path, so renaming the package directory would have no effect", modulePath)
}
- newPathPrefix := path.Join(path.Dir(oldPath), newName)
+ newPathPrefix := path.Join(path.Dir(string(oldPath)), string(newName))
edits := make(map[span.URI][]protocol.TextEdit)
seen := make(seenPackageRename) // track per-file import renaming we've already processed
@@ -272,7 +272,7 @@
// Subtle: check this condition before checking for valid module info
// below, because we should not fail this operation if unrelated packages
// lack module info.
- if !strings.HasPrefix(m.PackagePath()+"/", oldPath+"/") {
+ if !strings.HasPrefix(string(m.PackagePath())+"/", string(oldPath)+"/") {
continue // not affected by the package renaming
}
@@ -280,16 +280,16 @@
return nil, fmt.Errorf("cannot rename package: missing module information for package %q", m.PackagePath())
}
- if modulePath != m.ModuleInfo().Path {
+ if modulePath != PackagePath(m.ModuleInfo().Path) {
continue // don't edit imports if nested package and renaming package have different module paths
}
// Renaming a package consists of changing its import path and package name.
- suffix := strings.TrimPrefix(m.PackagePath(), oldPath)
+ suffix := strings.TrimPrefix(string(m.PackagePath()), string(oldPath))
newPath := newPathPrefix + suffix
pkgName := m.PackageName()
- if m.PackagePath() == oldPath {
+ if m.PackagePath() == PackagePath(oldPath) {
pkgName = newName
if err := renamePackageClause(ctx, m, s, newName, seen, edits); err != nil {
@@ -297,7 +297,8 @@
}
}
- if err := renameImports(ctx, s, m, newPath, pkgName, seen, edits); err != nil {
+ imp := ImportPath(newPath) // TODO(adonovan): what if newPath has vendor/ prefix?
+ if err := renameImports(ctx, s, m, imp, pkgName, seen, edits); err != nil {
return nil, err
}
}
@@ -314,14 +315,14 @@
// However, in all cases the resulting edits will be the same.
type seenPackageRename map[seenPackageKey]bool
type seenPackageKey struct {
- uri span.URI
- importPath string
+ uri span.URI
+ path PackagePath
}
// add reports whether uri and importPath have been seen, and records them as
// seen if not.
-func (s seenPackageRename) add(uri span.URI, importPath string) bool {
- key := seenPackageKey{uri, importPath}
+func (s seenPackageRename) add(uri span.URI, path PackagePath) bool {
+ key := seenPackageKey{uri, path}
seen := s[key]
if !seen {
s[key] = true
@@ -336,7 +337,7 @@
// package clause has already been updated, to prevent duplicate edits.
//
// Edits are written into the edits map.
-func renamePackageClause(ctx context.Context, m Metadata, s Snapshot, newName string, seen seenPackageRename, edits map[span.URI][]protocol.TextEdit) error {
+func renamePackageClause(ctx context.Context, m Metadata, s Snapshot, newName PackageName, seen seenPackageRename, edits map[span.URI][]protocol.TextEdit) error {
pkg, err := s.WorkspacePackageByID(ctx, m.PackageID())
if err != nil {
return err
@@ -358,7 +359,7 @@
}
edits[f.URI] = append(edits[f.URI], protocol.TextEdit{
Range: rng,
- NewText: newName,
+ NewText: string(newName),
})
}
@@ -370,7 +371,7 @@
// newPath and name newName.
//
// Edits are written into the edits map.
-func renameImports(ctx context.Context, s Snapshot, m Metadata, newPath, newName string, seen seenPackageRename, edits map[span.URI][]protocol.TextEdit) error {
+func renameImports(ctx context.Context, s Snapshot, m Metadata, newPath ImportPath, newName PackageName, seen seenPackageRename, edits map[span.URI][]protocol.TextEdit) error {
// TODO(rfindley): we should get reverse dependencies as metadata first,
// rather then building the package immediately. We don't need reverse
// dependencies if they are intermediate test variants.
@@ -399,7 +400,8 @@
}
for _, imp := range f.File.Imports {
- if impPath, _ := strconv.Unquote(imp.Path.Value); impPath != m.PackagePath() {
+ // TODO(adonovan): what if RHS has "vendor/" prefix?
+ if UnquoteImportPath(imp) != ImportPath(m.PackagePath()) {
continue // not the import we're looking for
}
@@ -409,10 +411,9 @@
if err != nil {
return err
}
- newText := strconv.Quote(newPath)
edits[f.URI] = append(edits[f.URI], protocol.TextEdit{
Range: rng,
- NewText: newText,
+ NewText: strconv.Quote(string(newPath)),
})
// If the package name of an import has not changed or if its import
@@ -430,7 +431,7 @@
fileScope := dep.GetTypesInfo().Scopes[f.File]
var changes map[span.URI][]protocol.TextEdit
- localName := newName
+ localName := string(newName)
try := 0
// Keep trying with fresh names until one succeeds.
@@ -446,7 +447,7 @@
// If the chosen local package name matches the package's new name, delete the
// change that would have inserted an explicit local name, which is always
// the lexically first change.
- if localName == newName {
+ if localName == string(newName) {
v := changes[f.URI]
sort.Slice(v, func(i, j int) bool {
return protocol.CompareRange(v[i].Range, v[j].Range) < 0
diff --git a/gopls/internal/lsp/source/rename_check.go b/gopls/internal/lsp/source/rename_check.go
index c4d5709..b81b6f6 100644
--- a/gopls/internal/lsp/source/rename_check.go
+++ b/gopls/internal/lsp/source/rename_check.go
@@ -12,7 +12,6 @@
"go/token"
"go/types"
"reflect"
- "strconv"
"strings"
"unicode"
@@ -834,8 +833,8 @@
if imp == nil {
continue
}
- importPath, err := strconv.Unquote(imp.Path.Value)
- if err != nil {
+ importPath := UnquoteImportPath(imp)
+ if importPath == "" {
continue
}
imported, err := pkg.ResolveImportPath(importPath)
diff --git a/gopls/internal/lsp/source/stub.go b/gopls/internal/lsp/source/stub.go
index 28a2e2b..8e3d856 100644
--- a/gopls/internal/lsp/source/stub.go
+++ b/gopls/internal/lsp/source/stub.go
@@ -194,7 +194,7 @@
return nil, err
}
for _, p := range pkgs {
- if p.PkgPath() == ifaceObj.Pkg().Path() {
+ if p.PkgPath() == PackagePath(ifaceObj.Pkg().Path()) {
return p, nil
}
}
@@ -254,11 +254,11 @@
for i := 0; i < iface.NumEmbeddeds(); i++ {
eiface := iface.Embedded(i).Obj()
depPkg := ifacePkg
- if eiface.Pkg().Path() != ifacePkg.PkgPath() {
+ if path := PackagePath(eiface.Pkg().Path()); path != ifacePkg.PkgPath() {
// TODO(adonovan): I'm not sure what this is trying to do, but it
// looks wrong the in case of type aliases.
var err error
- depPkg, err = ifacePkg.DirectDep(eiface.Pkg().Path())
+ depPkg, err = ifacePkg.DirectDep(path)
if err != nil {
return nil, err
}
diff --git a/gopls/internal/lsp/source/util.go b/gopls/internal/lsp/source/util.go
index c933fba..3ad17c0 100644
--- a/gopls/internal/lsp/source/util.go
+++ b/gopls/internal/lsp/source/util.go
@@ -314,7 +314,7 @@
// findFileInDeps finds uri in pkg or its dependencies.
func findFileInDeps(pkg Package, uri span.URI) (*ParsedGoFile, Package, error) {
queue := []Package{pkg}
- seen := make(map[string]bool)
+ seen := make(map[PackageID]bool)
for len(queue) > 0 {
pkg := queue[0]
@@ -333,14 +333,14 @@
return nil, nil, fmt.Errorf("no file for %s in package %s", uri, pkg.ID())
}
-// ImportPath returns the unquoted import path of s,
+// UnquoteImportPath returns the unquoted import path of s,
// or "" if the path is not properly quoted.
-func ImportPath(s *ast.ImportSpec) string {
- t, err := strconv.Unquote(s.Path.Value)
+func UnquoteImportPath(s *ast.ImportSpec) ImportPath {
+ path, err := strconv.Unquote(s.Path.Value)
if err != nil {
return ""
}
- return t
+ return ImportPath(path)
}
// NodeContains returns true if a node encloses a given position pos.
@@ -532,14 +532,14 @@
// IsValidImport returns whether importPkgPath is importable
// by pkgPath
-func IsValidImport(pkgPath, importPkgPath string) bool {
+func IsValidImport(pkgPath, importPkgPath PackagePath) bool {
i := strings.LastIndex(string(importPkgPath), "/internal/")
if i == -1 {
return true
}
// TODO(rfindley): this looks wrong: IsCommandLineArguments is meant to
// operate on package IDs, not package paths.
- if IsCommandLineArguments(string(pkgPath)) {
+ if IsCommandLineArguments(PackageID(pkgPath)) {
return true
}
// TODO(rfindley): this is wrong. mod.testx/p should not be able to
@@ -551,10 +551,8 @@
// "command-line-arguments" package, which is a package with an unknown ID
// created by the go command. It can have a test variant, which is why callers
// should not check that a value equals "command-line-arguments" directly.
-//
-// TODO(rfindley): this should accept a PackageID.
-func IsCommandLineArguments(s string) bool {
- return strings.Contains(s, "command-line-arguments")
+func IsCommandLineArguments(id PackageID) bool {
+ return strings.Contains(string(id), "command-line-arguments")
}
// RecvIdent returns the type identifier of a method receiver.
diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go
index 54c942f..be2b7f6 100644
--- a/gopls/internal/lsp/source/view.go
+++ b/gopls/internal/lsp/source/view.go
@@ -84,7 +84,7 @@
DiagnosePackage(ctx context.Context, pkg Package) (map[span.URI][]*Diagnostic, error)
// Analyze runs the analyses for the given package at this snapshot.
- Analyze(ctx context.Context, pkgID string, analyzers []*Analyzer) ([]*Diagnostic, error)
+ Analyze(ctx context.Context, pkgID PackageID, analyzers []*Analyzer) ([]*Diagnostic, error)
// RunGoCommandPiped runs the given `go` command, writing its output
// to stdout and stderr. Verb, Args, and WorkingDir must be specified.
@@ -149,12 +149,12 @@
// GetActiveReverseDeps returns the active files belonging to the reverse
// dependencies of this file's package, checked in TypecheckWorkspace mode.
- GetReverseDependencies(ctx context.Context, id string) ([]Package, error)
+ GetReverseDependencies(ctx context.Context, id PackageID) ([]Package, error)
// CachedImportPaths returns all the imported packages loaded in this
// snapshot, indexed by their package path (not import path, despite the name)
// and checked in TypecheckWorkspace mode.
- CachedImportPaths(ctx context.Context) (map[string]Package, error)
+ CachedImportPaths(ctx context.Context) (map[PackagePath]Package, error)
// KnownPackages returns all the packages loaded in this snapshot, checked
// in TypecheckWorkspace mode.
@@ -171,7 +171,7 @@
// WorkspacePackageByID returns the workspace package with id, type checked
// in 'workspace' mode.
- WorkspacePackageByID(ctx context.Context, id string) (Package, error)
+ WorkspacePackageByID(ctx context.Context, id PackageID) (Package, error)
// Symbols returns all symbols in the snapshot.
Symbols(ctx context.Context) map[span.URI][]Symbol
@@ -338,17 +338,15 @@
}
// Metadata represents package metadata retrieved from go/packages.
-//
-// TODO(rfindley): move the strongly typed strings from the cache package here.
type Metadata interface {
// PackageID is the unique package id.
- PackageID() string
+ PackageID() PackageID
// PackageName is the package name.
- PackageName() string
+ PackageName() PackageName
// PackagePath is the package path.
- PackagePath() string
+ PackagePath() PackagePath
// ModuleInfo returns the go/packages module information for the given package.
ModuleInfo() *packages.Module
@@ -593,12 +591,23 @@
return a.Enabled
}
+// Declare explicit types for package paths, names, and IDs to ensure that we
+// never use an ID where a path belongs, and vice versa. If we confused these,
+// it would result in confusing errors because package IDs often look like
+// package paths.
+type (
+ PackageID string // go list's unique identifier for a package (e.g. "vendor/example.com/foo [vendor/example.com/bar.test]")
+ PackagePath string // name used to prefix linker symbols (e.g. "vendor/example.com/foo")
+ PackageName string // identifier in 'package' declaration (e.g. "foo")
+ ImportPath string // path that appears in an import declaration (e.g. "example.com/foo")
+)
+
// Package represents a Go package that has been type-checked. It maintains
// only the relevant fields of a *go/packages.Package.
type Package interface {
- ID() string // logically a cache.PackageID
- Name() string // logically a cache.PackageName
- PkgPath() string // logically a cache.PackagePath
+ ID() PackageID
+ Name() PackageName
+ PkgPath() PackagePath
CompiledGoFiles() []*ParsedGoFile
File(uri span.URI) (*ParsedGoFile, error)
GetSyntax() []*ast.File
@@ -606,9 +615,9 @@
GetTypesInfo() *types.Info
GetTypesSizes() types.Sizes
ForTest() string
- DirectDep(packagePath string) (Package, error) // logically a cache.PackagePath
- ResolveImportPath(importPath string) (Package, error) // logically a cache.ImportPath
- MissingDependencies() []string // unordered; logically cache.ImportPaths
+ DirectDep(path PackagePath) (Package, error)
+ ResolveImportPath(path ImportPath) (Package, error)
+ MissingDependencies() []ImportPath // unordered
Imports() []Package
Version() *module.Version
HasListOrParseErrors() bool
diff --git a/gopls/internal/lsp/source/workspace_symbol.go b/gopls/internal/lsp/source/workspace_symbol.go
index ee4e020..dfb5f39 100644
--- a/gopls/internal/lsp/source/workspace_symbol.go
+++ b/gopls/internal/lsp/source/workspace_symbol.go
@@ -92,7 +92,7 @@
func fullyQualifiedSymbolMatch(space []string, name string, pkg Metadata, matcher matcherFunc) ([]string, float64) {
if _, score := dynamicSymbolMatch(space, name, pkg, matcher); score > 0 {
- return append(space, pkg.PackagePath(), ".", name), score
+ return append(space, string(pkg.PackagePath()), ".", name), score
}
return nil, 0
}
@@ -106,12 +106,12 @@
var score float64
- endsInPkgName := strings.HasSuffix(pkg.PackagePath(), pkg.PackageName())
+ endsInPkgName := strings.HasSuffix(string(pkg.PackagePath()), string(pkg.PackageName()))
// If the package path does not end in the package name, we need to check the
// package-qualified symbol as an extra pass first.
if !endsInPkgName {
- pkgQualified := append(space, pkg.PackageName(), ".", name)
+ pkgQualified := append(space, string(pkg.PackageName()), ".", name)
idx, score := matcher(pkgQualified)
nameStart := len(pkg.PackageName()) + 1
if score > 0 {
@@ -126,7 +126,7 @@
}
// Now try matching the fully qualified symbol.
- fullyQualified := append(space, pkg.PackagePath(), ".", name)
+ fullyQualified := append(space, string(pkg.PackagePath()), ".", name)
idx, score := matcher(fullyQualified)
// As above, check if we matched just the unqualified symbol name.
@@ -141,7 +141,7 @@
if endsInPkgName && idx >= 0 {
pkgStart := len(pkg.PackagePath()) - len(pkg.PackageName())
if idx >= pkgStart {
- return append(space, pkg.PackageName(), ".", name), score
+ return append(space, string(pkg.PackageName()), ".", name), score
}
}
@@ -151,7 +151,7 @@
}
func packageSymbolMatch(space []string, name string, pkg Metadata, matcher matcherFunc) ([]string, float64) {
- qualified := append(space, pkg.PackageName(), ".", name)
+ qualified := append(space, string(pkg.PackageName()), ".", name)
if _, s := matcher(qualified); s > 0 {
return qualified, s
}
@@ -526,7 +526,7 @@
kind: sym.Kind,
uri: i.uri,
rng: sym.Range,
- container: i.md.PackagePath(),
+ container: string(i.md.PackagePath()),
}
store.store(si)
}
