From 0bbdbb2ef6097bd84ae01b7ab1f762caaea8a680 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 18 Oct 2019 14:22:18 -0400 Subject: [PATCH] internal/lsp: associate package with its snapshot A package really should always be associated with its snapshot rather than its view. This eliminates some extra parameters in a few utility functions. Change-Id: I60f9b7286e0072d3268602f6bd32052a3d2e5559 Reviewed-on: https://go-review.googlesource.com/c/tools/+/202039 Run-TryBot: Rebecca Stambler Reviewed-by: Heschi Kreinick TryBot-Result: Gobot Gobot --- internal/lsp/cache/check.go | 2 +- internal/lsp/cache/pkg.go | 10 ++-- internal/lsp/source/completion_format.go | 2 +- internal/lsp/source/hover.go | 2 +- internal/lsp/source/identifier.go | 64 ++++++++++++------------ internal/lsp/source/references.go | 4 +- internal/lsp/source/rename.go | 8 ++- internal/lsp/source/signature_help.go | 4 +- internal/lsp/source/util.go | 20 ++++---- internal/lsp/source/view.go | 1 + 10 files changed, 60 insertions(+), 57 deletions(-) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 084f3cd929..b872ce9985 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -266,7 +266,7 @@ func (imp *importer) typeCheck(ctx context.Context, cph *checkPackageHandle) (*p defer done() pkg := &pkg{ - view: imp.snapshot.view, + snapshot: imp.snapshot, id: cph.m.id, mode: cph.mode, pkgPath: cph.m.pkgPath, diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go index d570cbaf46..e967c4125d 100644 --- a/internal/lsp/cache/pkg.go +++ b/internal/lsp/cache/pkg.go @@ -20,7 +20,7 @@ import ( // pkg contains the type information needed by the source package. type pkg struct { - view *view + snapshot *snapshot // ID and package path have their own types to avoid being used interchangeably. id packageID @@ -44,6 +44,10 @@ type pkg struct { type packageID string type packagePath string +func (p *pkg) Snapshot() source.Snapshot { + return p.snapshot +} + func (p *pkg) ID() string { return string(p.id) } @@ -136,8 +140,8 @@ func (p *pkg) FindDiagnostic(pdiag protocol.Diagnostic) (*source.Diagnostic, err 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) + if p.snapshot.view.Ignore(uri) { + return p.snapshot.view.findIgnoredFile(ctx, uri) } queue := []*pkg{p} diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index 80b1c4f8e6..514cf04b0d 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -136,7 +136,7 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { if !(file.Pos() <= obj.Pos() && obj.Pos() <= file.End()) { return CompletionItem{}, errors.Errorf("no file for %s", obj.Name()) } - ident, err := findIdentifier(c.ctx, c.view, c.snapshot, pkg, file, obj.Pos()) + ident, err := findIdentifier(c.ctx, pkg, file, obj.Pos()) if err != nil { return CompletionItem{}, err } diff --git a/internal/lsp/source/hover.go b/internal/lsp/source/hover.go index 6a127d7ac6..c7a3f112d8 100644 --- a/internal/lsp/source/hover.go +++ b/internal/lsp/source/hover.go @@ -47,7 +47,7 @@ func (i *IdentifierInfo) Hover(ctx context.Context) (*HoverInformation, error) { switch x := h.source.(type) { case ast.Node: var b strings.Builder - if err := format.Node(&b, i.View.Session().Cache().FileSet(), x); err != nil { + if err := format.Node(&b, i.Snapshot().View().Session().Cache().FileSet(), x); err != nil { return nil, err } h.Signature = b.String() diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index baed832e99..531f833e5a 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -20,10 +20,8 @@ import ( // IdentifierInfo holds information about an identifier in Go source. type IdentifierInfo struct { - Name string - View View - snapshot Snapshot - File ParseGoHandle + Name string + File ParseGoHandle mappedRange Type struct { @@ -39,6 +37,10 @@ type IdentifierInfo struct { qf types.Qualifier } +func (i *IdentifierInfo) Snapshot() Snapshot { + return i.pkg.Snapshot() +} + type Declaration struct { mappedRange node ast.Node @@ -49,7 +51,7 @@ type Declaration struct { // Identifier returns identifier information for a position // in a file, accounting for a potentially incomplete selector. func Identifier(ctx context.Context, view View, f File, pos protocol.Position) (*IdentifierInfo, error) { - snapshot, cphs, err := view.CheckPackageHandles(ctx, f) + _, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, err } @@ -77,17 +79,17 @@ func Identifier(ctx context.Context, view View, f File, pos protocol.Position) ( if err != nil { return nil, err } - return findIdentifier(ctx, view, snapshot, pkg, file, rng.Start) + return findIdentifier(ctx, pkg, file, rng.Start) } -func findIdentifier(ctx context.Context, view View, snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { - if result, err := identifier(ctx, view, snapshot, pkg, file, pos); err != nil || result != nil { +func findIdentifier(ctx context.Context, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { + if result, err := identifier(ctx, pkg, file, pos); err != nil || result != nil { return result, err } // If the position is not an identifier but immediately follows // an identifier or selector period (as is common when // requesting a completion), use the path to the preceding node. - ident, err := identifier(ctx, view, snapshot, pkg, file, pos-1) + ident, err := identifier(ctx, pkg, file, pos-1) if ident == nil && err == nil { err = errors.New("no identifier found") } @@ -95,20 +97,21 @@ func findIdentifier(ctx context.Context, view View, snapshot Snapshot, pkg Packa } // identifier checks a single position for a potential identifier. -func identifier(ctx context.Context, view View, snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { +func identifier(ctx context.Context, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { ctx, done := trace.StartSpan(ctx, "source.identifier") defer done() var err error // Handle import specs separately, as there is no formal position for a package declaration. - if result, err := importSpec(ctx, view, snapshot, file, pkg, pos); result != nil || err != nil { + if result, err := importSpec(ctx, pkg, file, pos); result != nil || err != nil { return result, err } path, _ := astutil.PathEnclosingInterval(file, pos, pos) if path == nil { return nil, errors.Errorf("can't find node enclosing position") } + view := pkg.Snapshot().View() uri := span.FileURI(view.Session().Cache().FileSet().Position(pos).Filename) var ph ParseGoHandle for _, h := range pkg.Files() { @@ -117,12 +120,10 @@ func identifier(ctx context.Context, view View, snapshot Snapshot, pkg Package, } } result := &IdentifierInfo{ - View: view, - snapshot: snapshot, - File: ph, - qf: qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()), - pkg: pkg, - ident: searchForIdent(path[0]), + File: ph, + qf: qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()), + pkg: pkg, + ident: searchForIdent(path[0]), } // No identifier at the given position. if result.ident == nil { @@ -135,7 +136,7 @@ func identifier(ctx context.Context, view View, snapshot Snapshot, pkg Package, } } result.Name = result.ident.Name - if result.mappedRange, err = posToMappedRange(ctx, view, pkg, result.ident.Pos(), result.ident.End()); err != nil { + if result.mappedRange, err = posToMappedRange(ctx, pkg, result.ident.Pos(), result.ident.End()); err != nil { return nil, err } result.Declaration.obj = pkg.GetTypesInfo().ObjectOf(result.ident) @@ -166,7 +167,7 @@ func identifier(ctx context.Context, view View, snapshot Snapshot, pkg Package, return nil, errors.Errorf("no declaration for %s", result.Name) } result.Declaration.node = decl - if result.Declaration.mappedRange, err = nameToMappedRange(ctx, view, pkg, decl.Pos(), result.Name); err != nil { + if result.Declaration.mappedRange, err = nameToMappedRange(ctx, pkg, decl.Pos(), result.Name); err != nil { return nil, err } return result, nil @@ -191,10 +192,10 @@ func identifier(ctx context.Context, view View, snapshot Snapshot, pkg Package, } } - if result.Declaration.mappedRange, err = objToMappedRange(ctx, view, pkg, result.Declaration.obj); err != nil { + if result.Declaration.mappedRange, err = objToMappedRange(ctx, pkg, result.Declaration.obj); err != nil { return nil, err } - if result.Declaration.node, err = objToNode(ctx, view, pkg, result.Declaration.obj); err != nil { + if result.Declaration.node, err = objToNode(ctx, pkg, result.Declaration.obj); err != nil { return nil, err } typ := pkg.GetTypesInfo().TypeOf(result.ident) @@ -208,7 +209,7 @@ func identifier(ctx context.Context, view View, snapshot Snapshot, pkg Package, if hasErrorType(result.Type.Object) { return result, nil } - if result.Type.mappedRange, err = objToMappedRange(ctx, view, pkg, result.Type.Object); err != nil { + if result.Type.mappedRange, err = objToMappedRange(ctx, pkg, result.Type.Object); err != nil { return nil, err } } @@ -242,7 +243,8 @@ func hasErrorType(obj types.Object) bool { return types.IsInterface(obj.Type()) && obj.Pkg() == nil && obj.Name() == "error" } -func objToNode(ctx context.Context, view View, pkg Package, obj types.Object) (ast.Decl, error) { +func objToNode(ctx context.Context, pkg Package, obj types.Object) (ast.Decl, error) { + view := pkg.Snapshot().View() uri := span.FileURI(view.Session().Cache().FileSet().Position(obj.Pos()).Filename) ph, _, err := pkg.FindFile(ctx, uri) if err != nil { @@ -278,7 +280,7 @@ func objToNode(ctx context.Context, view View, pkg Package, obj types.Object) (a } // importSpec handles positions inside of an *ast.ImportSpec. -func importSpec(ctx context.Context, view View, snapshot Snapshot, file *ast.File, pkg Package, pos token.Pos) (*IdentifierInfo, error) { +func importSpec(ctx context.Context, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { var imp *ast.ImportSpec for _, spec := range file.Imports { if spec.Path.Pos() <= pos && pos < spec.Path.End() { @@ -292,7 +294,7 @@ func importSpec(ctx context.Context, view View, snapshot Snapshot, file *ast.Fil if err != nil { return nil, errors.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err) } - uri := span.FileURI(view.Session().Cache().FileSet().Position(pos).Filename) + uri := span.FileURI(pkg.Snapshot().View().Session().Cache().FileSet().Position(pos).Filename) var ph ParseGoHandle for _, h := range pkg.Files() { if h.File().Identity().URI == uri { @@ -300,13 +302,11 @@ func importSpec(ctx context.Context, view View, snapshot Snapshot, file *ast.Fil } } result := &IdentifierInfo{ - View: view, - snapshot: snapshot, - File: ph, - Name: importPath, - pkg: pkg, + File: ph, + Name: importPath, + pkg: pkg, } - if result.mappedRange, err = posToMappedRange(ctx, view, pkg, imp.Path.Pos(), imp.Path.End()); err != nil { + if result.mappedRange, err = posToMappedRange(ctx, pkg, imp.Path.Pos(), imp.Path.End()); err != nil { return nil, err } // Consider the "declaration" of an import spec to be the imported package. @@ -327,7 +327,7 @@ func importSpec(ctx context.Context, view View, snapshot Snapshot, file *ast.Fil if dest == nil { return nil, errors.Errorf("package %q has no files", importPath) } - if result.Declaration.mappedRange, err = posToMappedRange(ctx, view, pkg, dest.Pos(), dest.End()); err != nil { + if result.Declaration.mappedRange, err = posToMappedRange(ctx, 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 84c9d4d650..d0c60f5edc 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.View, i.pkg, ident.Pos(), ident.End()) + rng, err := posToMappedRange(ctx, 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.View, i.pkg, ident.Pos(), ident.End()) + rng, err := posToMappedRange(ctx, 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 100b55c488..1ccdb7a8f7 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -151,7 +151,7 @@ func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string) if err != nil { return nil, err } - fh := i.snapshot.Handle(ctx, f) + fh := i.Snapshot().Handle(ctx, f) data, _, err := fh.Read(ctx) if err != nil { return nil, err @@ -218,16 +218,14 @@ func getPkgNameIdentifier(ctx context.Context, ident *IdentifierInfo, pkgName *t wasImplicit: true, } var err error - if decl.mappedRange, err = objToMappedRange(ctx, ident.View, ident.pkg, decl.obj); err != nil { + if decl.mappedRange, err = objToMappedRange(ctx, ident.pkg, decl.obj); err != nil { return nil, err } - if decl.node, err = objToNode(ctx, ident.View, ident.pkg, decl.obj); err != nil { + if decl.node, err = objToNode(ctx, ident.pkg, decl.obj); err != nil { return nil, err } return &IdentifierInfo{ Name: pkgName.Name(), - View: ident.View, - snapshot: ident.snapshot, mappedRange: decl.mappedRange, File: ident.File, Declaration: decl, diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index 0f87de489f..848cd52d27 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, view, pkg, obj) + node, err := objToNode(ctx, pkg, obj) if err != nil { return nil, err } - rng, err := objToMappedRange(ctx, view, pkg, obj) + rng, err := objToMappedRange(ctx, pkg, obj) if err != nil { return nil, err } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 1e6d736320..f74e1a15f6 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, view View, pkg Package, obj types.Object) (mappedRange, error) { +func objToMappedRange(ctx context.Context, 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, view View, pkg Package, obj types.Obj // 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, view, pkg, obj.Pos(), "") + return nameToMappedRange(ctx, pkg, obj.Pos(), "") } } - return nameToMappedRange(ctx, view, pkg, obj.Pos(), obj.Name()) + return nameToMappedRange(ctx, pkg, obj.Pos(), obj.Name()) } -func nameToMappedRange(ctx context.Context, view View, pkg Package, pos token.Pos, name string) (mappedRange, error) { - return posToMappedRange(ctx, view, pkg, pos, pos+token.Pos(len(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 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, view View, pkg Package, pos, end token.Pos) (mappedRange, error) { - m, err := posToMapper(ctx, view, pkg, pos) +func posToMappedRange(ctx context.Context, pkg Package, pos, end token.Pos) (mappedRange, error) { + m, err := posToMapper(ctx, pkg, pos) if err != nil { return mappedRange{}, err } - return posToRange(ctx, view, m, pos, end) + return posToRange(ctx, pkg.Snapshot().View(), m, pos, end) } func posToRange(ctx context.Context, view View, m *protocol.ColumnMapper, pos, end token.Pos) (mappedRange, error) { @@ -175,8 +175,8 @@ func posToRange(ctx context.Context, view View, m *protocol.ColumnMapper, pos, e }, nil } -func posToMapper(ctx context.Context, view View, pkg Package, pos token.Pos) (*protocol.ColumnMapper, error) { - posn := view.Session().Cache().FileSet().Position(pos) +func posToMapper(ctx context.Context, pkg Package, pos token.Pos) (*protocol.ColumnMapper, error) { + posn := pkg.Snapshot().View().Session().Cache().FileSet().Position(pos) ph, _, err := pkg.FindFile(ctx, span.FileURI(posn.Filename)) if err != nil { return nil, err diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index f4a2346c2f..b437b461d6 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -274,6 +274,7 @@ type File interface { // Package represents a Go package that has been type-checked. It maintains // only the relevant fields of a *go/packages.Package. type Package interface { + Snapshot() Snapshot ID() string PkgPath() string Files() []ParseGoHandle