From 8456940f41e685a9d6a108d90b4c4633af840aae Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 29 Oct 2019 18:13:19 -0400 Subject: [PATCH] internal/lsp: remove the pkg.view field, in preparation for CL 204079 This change encompasses the refactorings needed to correctly implement CL 204079. The goal of this CL is to make the actual relevant diffs more clear. Change-Id: I38acfd436e2380be790910e01b6e37d8280e9100 Reviewed-on: https://go-review.googlesource.com/c/tools/+/204139 Run-TryBot: Rebecca Stambler Reviewed-by: Ian Cottrell --- internal/lsp/cache/analysis.go | 2 +- internal/lsp/cache/check.go | 9 ++++---- internal/lsp/cache/errors.go | 9 ++++---- internal/lsp/cache/pkg.go | 29 +++++++++++------------- internal/lsp/cache/view.go | 8 +++++++ internal/lsp/source/completion_format.go | 2 +- internal/lsp/source/identifier.go | 27 +++++++++++----------- internal/lsp/source/references.go | 4 ++-- internal/lsp/source/rename.go | 4 ++-- internal/lsp/source/rename_check.go | 2 +- internal/lsp/source/signature_help.go | 4 ++-- internal/lsp/source/util.go | 22 +++++++++--------- internal/lsp/source/view.go | 15 +++++------- 13 files changed, 69 insertions(+), 68 deletions(-) diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go index e707f7365f..f66388f3c8 100644 --- a/internal/lsp/cache/analysis.go +++ b/internal/lsp/cache/analysis.go @@ -262,7 +262,7 @@ func runAnalysis(ctx context.Context, fset *token.FileSet, act *actionHandle) ([ var errors []*source.Error for _, diag := range diagnostics { - srcErr, err := sourceError(ctx, act.pkg, diag) + srcErr, err := sourceError(ctx, fset, act.pkg, diag) if err != nil { return nil, nil, err } diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index ff0f4120ab..3af794f732 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "go/ast" + "go/token" "go/types" "sort" "sync" @@ -69,6 +70,7 @@ func (s *snapshot) checkPackageHandle(ctx context.Context, id packageID, mode so if err != nil { return nil, err } + fset := s.View().Session().Cache().FileSet() h := s.view.session.cache.store.Bind(string(cph.key), func(ctx context.Context) interface{} { // Begin loading the direct dependencies, in parallel. for _, impID := range cph.imports { @@ -81,7 +83,7 @@ func (s *snapshot) checkPackageHandle(ctx context.Context, id packageID, mode so }(dep) } data := &checkPackageData{} - data.pkg, data.err = s.typeCheck(ctx, cph) + data.pkg, data.err = s.typeCheck(ctx, fset, cph) return data }) cph.handle = h @@ -211,7 +213,7 @@ func (s *snapshot) parseGoHandles(ctx context.Context, m *metadata, mode source. return phs, nil } -func (s *snapshot) typeCheck(ctx context.Context, cph *checkPackageHandle) (*pkg, error) { +func (s *snapshot) typeCheck(ctx context.Context, fset *token.FileSet, cph *checkPackageHandle) (*pkg, error) { ctx, done := trace.StartSpan(ctx, "cache.importer.typeCheck", telemetry.Package.Of(cph.m.id)) defer done() @@ -221,7 +223,6 @@ func (s *snapshot) typeCheck(ctx context.Context, cph *checkPackageHandle) (*pkg } pkg := &pkg{ - view: s.view, id: cph.m.id, mode: cph.mode, pkgPath: cph.m.pkgPath, @@ -305,7 +306,7 @@ func (s *snapshot) typeCheck(ctx context.Context, cph *checkPackageHandle) (*pkg // We don't care about a package's errors unless we have parsed it in full. if cph.mode == source.ParseFull { for _, e := range rawErrors { - srcErr, err := sourceError(ctx, pkg, e) + srcErr, err := sourceError(ctx, fset, pkg, e) if err != nil { return nil, err } diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go index 4a569f2528..315f1574d6 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -18,7 +18,7 @@ import ( errors "golang.org/x/xerrors" ) -func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, error) { +func sourceError(ctx context.Context, fset *token.FileSet, pkg *pkg, e interface{}) (*source.Error, error) { var ( spn span.Span err error @@ -27,7 +27,6 @@ func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, e fixes []source.SuggestedFix related []source.RelatedInformation ) - fset := pkg.view.session.cache.fset switch e := e.(type) { case packages.Error: if e.Pos == "" { @@ -163,7 +162,7 @@ func typeErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, pos toke return span.Span{}, err } posn := fset.Position(pos) - ph, _, err := pkg.FindFile(ctx, span.FileURI(posn.Filename)) + ph, _, err := findFileInPackage(ctx, span.FileURI(posn.Filename), pkg) if err != nil { return spn, nil // ignore errors } @@ -196,7 +195,7 @@ func typeErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, pos toke } func scannerErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, posn token.Position) (span.Span, error) { - ph, _, err := pkg.FindFile(ctx, span.FileURI(posn.Filename)) + ph, _, err := findFileInPackage(ctx, span.FileURI(posn.Filename), pkg) if err != nil { return span.Span{}, err } @@ -224,7 +223,7 @@ func scannerErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, posn // spanToRange converts a span.Span to a protocol.Range, // assuming that the span belongs to the package whose diagnostics are being computed. func spanToRange(ctx context.Context, pkg *pkg, spn span.Span) (protocol.Range, error) { - ph, _, err := pkg.FindFile(ctx, spn.URI()) + ph, _, err := findFileInPackage(ctx, spn.URI(), pkg) if err != nil { return protocol.Range{}, err } diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go index 6d8099acbd..613f8097f9 100644 --- a/internal/lsp/cache/pkg.go +++ b/internal/lsp/cache/pkg.go @@ -17,8 +17,6 @@ import ( // pkg contains the type information needed by the source package. type pkg struct { - view *view - // ID and package path have their own types to avoid being used interchangeably. id packageID pkgPath packagePath @@ -38,10 +36,6 @@ type pkg struct { type packageID string type packagePath string -func (p *pkg) View() source.View { - return p.view -} - func (p *pkg) ID() string { return string(p.id) } @@ -94,7 +88,7 @@ func (p *pkg) IsIllTyped() bool { return p.types == nil || p.typesInfo == nil || p.typesSizes == nil } -func (p *pkg) GetImport(ctx context.Context, pkgPath string) (source.Package, error) { +func (p *pkg) GetImport(pkgPath string) (source.Package, error) { if imp := p.imports[packagePath(pkgPath)]; imp != nil { return imp, nil } @@ -102,6 +96,14 @@ func (p *pkg) GetImport(ctx context.Context, pkgPath string) (source.Package, er return nil, errors.Errorf("no imported package for %s", pkgPath) } +func (p *pkg) Imports() []source.Package { + var result []source.Package + for _, imp := range p.imports { + result = append(result, imp) + } + return result +} + func (s *snapshot) FindAnalysisError(ctx context.Context, id string, diag protocol.Diagnostic) (*source.Error, error) { acts := s.getActionHandles(packageID(id), source.ParseFull) for _, act := range acts { @@ -125,13 +127,8 @@ func (s *snapshot) FindAnalysisError(ctx context.Context, id string, diag protoc return nil, errors.Errorf("no matching diagnostic for %v", diag) } -func (p *pkg) FindFile(ctx context.Context, uri span.URI) (source.ParseGoHandle, source.Package, error) { - // Special case for ignored files. - if p.view.Ignore(uri) { - return p.view.findIgnoredFile(ctx, uri) - } - - queue := []*pkg{p} +func findFileInPackage(ctx context.Context, uri span.URI, pkg source.Package) (source.ParseGoHandle, source.Package, error) { + queue := []source.Package{pkg} seen := make(map[string]bool) for len(queue) > 0 { @@ -139,12 +136,12 @@ func (p *pkg) FindFile(ctx context.Context, uri span.URI) (source.ParseGoHandle, queue = queue[1:] seen[pkg.ID()] = true - for _, ph := range pkg.files { + for _, ph := range pkg.Files() { if ph.File().Identity().URI == uri { return ph, pkg, nil } } - for _, dep := range pkg.imports { + for _, dep := range pkg.Imports() { if !seen[dep.ID()] { queue = append(queue, dep) } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 227166090a..2a5c45e791 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -418,6 +418,14 @@ func (v *view) openFiles(ctx context.Context, uris []span.URI) (results []source return results } +func (v *view) FindFileInPackage(ctx context.Context, uri span.URI, pkg source.Package) (source.ParseGoHandle, source.Package, error) { + // Special case for ignored files. + if v.Ignore(uri) { + return v.findIgnoredFile(ctx, uri) + } + return findFileInPackage(ctx, uri, pkg) +} + type debugView struct{ *view } func (v debugView) ID() string { return v.id } diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index 3123dae6c8..e03857e378 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -125,7 +125,7 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { return item, nil } uri := span.FileURI(pos.Filename) - ph, pkg, err := c.pkg.FindFile(c.ctx, uri) + ph, pkg, err := c.view.FindFileInPackage(c.ctx, uri, c.pkg) if err != nil { return CompletionItem{}, err } diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index ac286afbbb..4addaf3732 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -108,7 +108,7 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F if path == nil { return nil, errors.Errorf("can't find node enclosing position") } - view := pkg.View() + view := snapshot.View() uri := span.FileURI(view.Session().Cache().FileSet().Position(pos).Filename) var ph ParseGoHandle for _, h := range pkg.Files() { @@ -134,7 +134,7 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F } } result.Name = result.ident.Name - if result.mappedRange, err = posToMappedRange(ctx, pkg, result.ident.Pos(), result.ident.End()); err != nil { + if result.mappedRange, err = posToMappedRange(ctx, view, pkg, result.ident.Pos(), result.ident.End()); err != nil { return nil, err } result.Declaration.obj = pkg.GetTypesInfo().ObjectOf(result.ident) @@ -165,7 +165,7 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F return nil, errors.Errorf("no declaration for %s", result.Name) } result.Declaration.node = decl - if result.Declaration.mappedRange, err = nameToMappedRange(ctx, pkg, decl.Pos(), result.Name); err != nil { + if result.Declaration.mappedRange, err = nameToMappedRange(ctx, view, pkg, decl.Pos(), result.Name); err != nil { return nil, err } return result, nil @@ -190,10 +190,10 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F } } - if result.Declaration.mappedRange, err = objToMappedRange(ctx, pkg, result.Declaration.obj); err != nil { + if result.Declaration.mappedRange, err = objToMappedRange(ctx, view, pkg, result.Declaration.obj); err != nil { return nil, err } - if result.Declaration.node, err = objToNode(ctx, pkg, result.Declaration.obj); err != nil { + if result.Declaration.node, err = objToNode(ctx, snapshot.View(), pkg, result.Declaration.obj); err != nil { return nil, err } typ := pkg.GetTypesInfo().TypeOf(result.ident) @@ -207,7 +207,7 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F if hasErrorType(result.Type.Object) { return result, nil } - if result.Type.mappedRange, err = objToMappedRange(ctx, pkg, result.Type.Object); err != nil { + if result.Type.mappedRange, err = objToMappedRange(ctx, view, pkg, result.Type.Object); err != nil { return nil, err } } @@ -241,10 +241,9 @@ func hasErrorType(obj types.Object) bool { return types.IsInterface(obj.Type()) && obj.Pkg() == nil && obj.Name() == "error" } -func objToNode(ctx context.Context, pkg Package, obj types.Object) (ast.Decl, error) { - view := pkg.View() - uri := span.FileURI(view.Session().Cache().FileSet().Position(obj.Pos()).Filename) - ph, _, err := pkg.FindFile(ctx, uri) +func objToNode(ctx context.Context, v View, pkg Package, obj types.Object) (ast.Decl, error) { + uri := span.FileURI(v.Session().Cache().FileSet().Position(obj.Pos()).Filename) + ph, _, err := v.FindFileInPackage(ctx, uri, pkg) if err != nil { return nil, err } @@ -292,7 +291,7 @@ func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F if err != nil { return nil, errors.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err) } - uri := span.FileURI(pkg.View().Session().Cache().FileSet().Position(pos).Filename) + uri := span.FileURI(snapshot.View().Session().Cache().FileSet().Position(pos).Filename) var ph ParseGoHandle for _, h := range pkg.Files() { if h.File().Identity().URI == uri { @@ -305,11 +304,11 @@ func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F Name: importPath, pkg: pkg, } - if result.mappedRange, err = posToMappedRange(ctx, pkg, imp.Path.Pos(), imp.Path.End()); err != nil { + if result.mappedRange, err = posToMappedRange(ctx, snapshot.View(), pkg, imp.Path.Pos(), imp.Path.End()); err != nil { return nil, err } // Consider the "declaration" of an import spec to be the imported package. - importedPkg, err := pkg.GetImport(ctx, importPath) + importedPkg, err := pkg.GetImport(importPath) if err != nil { return nil, err } @@ -326,7 +325,7 @@ func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F if dest == nil { return nil, errors.Errorf("package %q has no files", importPath) } - if result.Declaration.mappedRange, err = posToMappedRange(ctx, pkg, dest.Pos(), dest.End()); err != nil { + if result.Declaration.mappedRange, err = posToMappedRange(ctx, snapshot.View(), pkg, dest.Pos(), dest.End()); err != nil { return nil, err } result.Declaration.node = imp diff --git a/internal/lsp/source/references.go b/internal/lsp/source/references.go index d0c60f5edc..5fd20f822a 100644 --- a/internal/lsp/source/references.go +++ b/internal/lsp/source/references.go @@ -55,7 +55,7 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro if obj == nil || !sameObj(obj, i.Declaration.obj) { continue } - rng, err := posToMappedRange(ctx, i.pkg, ident.Pos(), ident.End()) + rng, err := posToMappedRange(ctx, i.Snapshot.View(), i.pkg, ident.Pos(), ident.End()) if err != nil { return nil, err } @@ -73,7 +73,7 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro if obj == nil || !sameObj(obj, i.Declaration.obj) { continue } - rng, err := posToMappedRange(ctx, i.pkg, ident.Pos(), ident.End()) + rng, err := posToMappedRange(ctx, i.Snapshot.View(), i.pkg, ident.Pos(), ident.End()) if err != nil { return nil, err } diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 69f45ebf6c..d6b50f829a 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -218,10 +218,10 @@ func getPkgNameIdentifier(ctx context.Context, ident *IdentifierInfo, pkgName *t wasImplicit: true, } var err error - if decl.mappedRange, err = objToMappedRange(ctx, ident.pkg, decl.obj); err != nil { + if decl.mappedRange, err = objToMappedRange(ctx, ident.Snapshot.View(), ident.pkg, decl.obj); err != nil { return nil, err } - if decl.node, err = objToNode(ctx, ident.pkg, decl.obj); err != nil { + if decl.node, err = objToNode(ctx, ident.Snapshot.View(), ident.pkg, decl.obj); err != nil { return nil, err } return &IdentifierInfo{ diff --git a/internal/lsp/source/rename_check.go b/internal/lsp/source/rename_check.go index ce79b5824b..320771961e 100644 --- a/internal/lsp/source/rename_check.go +++ b/internal/lsp/source/rename_check.go @@ -850,7 +850,7 @@ func pathEnclosingInterval(ctx context.Context, fset *token.FileSet, pkg Package if err != nil { continue } - importPkg, err := pkg.GetImport(ctx, importPath) + importPkg, err := pkg.GetImport(importPath) if err != nil { return nil, nil, false } diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index 4f190675ae..c884edab9e 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -121,11 +121,11 @@ FindCall: comment *ast.CommentGroup ) if obj != nil { - node, err := objToNode(ctx, pkg, obj) + node, err := objToNode(ctx, view, pkg, obj) if err != nil { return nil, err } - rng, err := objToMappedRange(ctx, pkg, obj) + rng, err := objToMappedRange(ctx, view, pkg, obj) if err != nil { return nil, err } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 6559765d52..e056a6ef50 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -127,7 +127,7 @@ func nodeToProtocolRange(ctx context.Context, view View, m *protocol.ColumnMappe return mrng.Range() } -func objToMappedRange(ctx context.Context, pkg Package, obj types.Object) (mappedRange, error) { +func objToMappedRange(ctx context.Context, v View, pkg Package, obj types.Object) (mappedRange, error) { if pkgName, ok := obj.(*types.PkgName); ok { // An imported Go package has a package-local, unqualified name. // When the name matches the imported package name, there is no @@ -140,26 +140,26 @@ func objToMappedRange(ctx context.Context, pkg Package, obj types.Object) (mappe // When the identifier does not appear in the source, have the range // of the object be the point at the beginning of the declaration. if pkgName.Imported().Name() == pkgName.Name() { - return nameToMappedRange(ctx, pkg, obj.Pos(), "") + return nameToMappedRange(ctx, v, pkg, obj.Pos(), "") } } - return nameToMappedRange(ctx, pkg, obj.Pos(), obj.Name()) + return nameToMappedRange(ctx, v, pkg, obj.Pos(), obj.Name()) } -func nameToMappedRange(ctx context.Context, pkg Package, pos token.Pos, name string) (mappedRange, error) { - return posToMappedRange(ctx, pkg, pos, pos+token.Pos(len(name))) +func nameToMappedRange(ctx context.Context, v View, pkg Package, pos token.Pos, name string) (mappedRange, error) { + return posToMappedRange(ctx, v, pkg, pos, pos+token.Pos(len(name))) } func nodeToMappedRange(ctx context.Context, view View, m *protocol.ColumnMapper, n ast.Node) (mappedRange, error) { return posToRange(ctx, view, m, n.Pos(), n.End()) } -func posToMappedRange(ctx context.Context, pkg Package, pos, end token.Pos) (mappedRange, error) { - m, err := posToMapper(ctx, pkg, pos) +func posToMappedRange(ctx context.Context, v View, pkg Package, pos, end token.Pos) (mappedRange, error) { + m, err := posToMapper(ctx, v, pkg, pos) if err != nil { return mappedRange{}, err } - return posToRange(ctx, pkg.View(), m, pos, end) + return posToRange(ctx, v, m, pos, end) } func posToRange(ctx context.Context, view View, m *protocol.ColumnMapper, pos, end token.Pos) (mappedRange, error) { @@ -175,9 +175,9 @@ func posToRange(ctx context.Context, view View, m *protocol.ColumnMapper, pos, e }, nil } -func posToMapper(ctx context.Context, pkg Package, pos token.Pos) (*protocol.ColumnMapper, error) { - posn := pkg.View().Session().Cache().FileSet().Position(pos) - ph, _, err := pkg.FindFile(ctx, span.FileURI(posn.Filename)) +func posToMapper(ctx context.Context, v View, pkg Package, pos token.Pos) (*protocol.ColumnMapper, error) { + posn := v.Session().Cache().FileSet().Position(pos) + ph, _, err := v.FindFileInPackage(ctx, span.FileURI(posn.Filename), pkg) if err != nil { return nil, err } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index fad38dc68d..8935f334b0 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -249,6 +249,10 @@ type View interface { // dependencies of this file's package. GetActiveReverseDeps(ctx context.Context, f File) []CheckPackageHandle + // FindFileInPackage returns the AST and type information for a file that may + // belong to or be part of a dependency of the given package. + FindFileInPackage(ctx context.Context, uri span.URI, pkg Package) (ParseGoHandle, Package, error) + // Snapshot returns the current snapshot for the view. Snapshot() Snapshot } @@ -292,15 +296,8 @@ type Package interface { GetTypesInfo() *types.Info GetTypesSizes() types.Sizes IsIllTyped() bool - - // GetImport returns the CheckPackageHandle for a package imported by this package. - GetImport(ctx context.Context, pkgPath string) (Package, error) - - // FindFile returns the AST and type information for a file that may - // belong to or be part of a dependency of the given package. - FindFile(ctx context.Context, uri span.URI) (ParseGoHandle, Package, error) - - View() View + GetImport(pkgPath string) (Package, error) + Imports() []Package } type Error struct {