From 69a2705782a05011d654f89087fdc771b4c6f776 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Wed, 13 Feb 2019 13:23:28 -0500 Subject: [PATCH] internal/lsp: remove error return values from interface "field" accessors we don't really use them, only generate them in cases where the failure is way more fundamental, and then also fail to remember them for the next call to the same accessor. Better to not have them. Change-Id: I0e8abeda688f5cc2a932ed95a80d89225c399f93 Reviewed-on: https://go-review.googlesource.com/c/162399 Reviewed-by: Rebecca Stambler --- internal/lsp/cache/file.go | 35 +++++++++------------------ internal/lsp/cmd/definition.go | 10 ++------ internal/lsp/format.go | 5 +--- internal/lsp/imports.go | 5 +--- internal/lsp/position.go | 5 +--- internal/lsp/server.go | 25 ++++--------------- internal/lsp/source/completion.go | 10 ++------ internal/lsp/source/definition.go | 21 ++++------------ internal/lsp/source/diagnostics.go | 10 ++------ internal/lsp/source/format.go | 15 +++--------- internal/lsp/source/signature_help.go | 10 ++------ internal/lsp/source/view.go | 8 +++--- 12 files changed, 39 insertions(+), 120 deletions(-) diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go index cded8720..1a12a383 100644 --- a/internal/lsp/cache/file.go +++ b/internal/lsp/cache/file.go @@ -5,7 +5,6 @@ package cache import ( - "fmt" "go/ast" "go/token" "io/ioutil" @@ -32,53 +31,41 @@ func (f *File) Read() ([]byte, error) { return f.read() } -func (f *File) GetFileSet() (*token.FileSet, error) { - if f.view.Config.Fset == nil { - return nil, fmt.Errorf("no fileset for file view config") - } - return f.view.Config.Fset, nil +func (f *File) GetFileSet() *token.FileSet { + return f.view.Config.Fset } -func (f *File) GetToken() (*token.File, error) { +func (f *File) GetToken() *token.File { f.view.mu.Lock() defer f.view.mu.Unlock() if f.token == nil { if err := f.view.parse(f.URI); err != nil { - return nil, err - } - if f.token == nil { - return nil, fmt.Errorf("failed to find or parse %v", f.URI) + return nil } } - return f.token, nil + return f.token } -func (f *File) GetAST() (*ast.File, error) { +func (f *File) GetAST() *ast.File { f.view.mu.Lock() defer f.view.mu.Unlock() if f.ast == nil { if err := f.view.parse(f.URI); err != nil { - return nil, err - } - if f.ast == nil { - return nil, fmt.Errorf("failed to find or parse %v", f.URI) + return nil } } - return f.ast, nil + return f.ast } -func (f *File) GetPackage() (*packages.Package, error) { +func (f *File) GetPackage() *packages.Package { f.view.mu.Lock() defer f.view.mu.Unlock() if f.pkg == nil { if err := f.view.parse(f.URI); err != nil { - return nil, err - } - if f.pkg == nil { - return nil, fmt.Errorf("failed to find or parse %v", f.URI) + return nil } } - return f.pkg, nil + return f.pkg } // read is the internal part of Read that presumes the lock is already held diff --git a/internal/lsp/cmd/definition.go b/internal/lsp/cmd/definition.go index 861de838..29202e88 100644 --- a/internal/lsp/cmd/definition.go +++ b/internal/lsp/cmd/definition.go @@ -65,10 +65,7 @@ func (d *definition) Run(ctx context.Context, args ...string) error { if err != nil { return err } - tok, err := f.GetToken() - if err != nil { - return err - } + tok := f.GetToken() pos := tok.Pos(from.Start.Offset) ident, err := source.Identifier(ctx, view, f, pos) if err != nil { @@ -118,10 +115,7 @@ func buildDefinition(view source.View, ident *source.IdentifierInfo) (*Definitio func buildGuruDefinition(view source.View, ident *source.IdentifierInfo) (*guru.Definition, error) { loc := newLocation(view.FileSet(), ident.Declaration.Range) - pkg, err := ident.File.GetPackage() - if err != nil { - return nil, err - } + pkg := ident.File.GetPackage() // guru does not support ranges loc.End = loc.Start // Behavior that attempts to match the expected output for guru. For an example diff --git a/internal/lsp/format.go b/internal/lsp/format.go index ec0db161..650f72e1 100644 --- a/internal/lsp/format.go +++ b/internal/lsp/format.go @@ -18,10 +18,7 @@ func formatRange(ctx context.Context, v source.View, uri protocol.DocumentURI, r if err != nil { return nil, err } - tok, err := f.GetToken() - if err != nil { - return nil, err - } + tok := f.GetToken() var r source.Range if rng == nil { r.Start = tok.Pos(0) diff --git a/internal/lsp/imports.go b/internal/lsp/imports.go index 2f8f7c1c..3b57db3b 100644 --- a/internal/lsp/imports.go +++ b/internal/lsp/imports.go @@ -20,10 +20,7 @@ func organizeImports(ctx context.Context, v source.View, uri protocol.DocumentUR if err != nil { return nil, err } - tok, err := f.GetToken() - if err != nil { - return nil, err - } + tok := f.GetToken() r := source.Range{ Start: tok.Pos(0), End: tok.Pos(tok.Size()), diff --git a/internal/lsp/position.go b/internal/lsp/position.go index 4b19c784..fb0d9bdb 100644 --- a/internal/lsp/position.go +++ b/internal/lsp/position.go @@ -36,10 +36,7 @@ func fromProtocolLocation(ctx context.Context, v *cache.View, loc protocol.Locat if err != nil { return source.Range{}, err } - tok, err := f.GetToken() - if err != nil { - return source.Range{}, err - } + tok := f.GetToken() return fromProtocolRange(tok, loc.Range), nil } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 46d03e6e..3db22fff 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -220,10 +220,7 @@ func (s *server) Completion(ctx context.Context, params *protocol.CompletionPara if err != nil { return nil, err } - tok, err := f.GetToken() - if err != nil { - return nil, err - } + tok := f.GetToken() pos := fromProtocolPosition(tok, params.Position) items, prefix, err := source.Completion(ctx, f, pos) if err != nil { @@ -248,10 +245,7 @@ func (s *server) Hover(ctx context.Context, params *protocol.TextDocumentPositio if err != nil { return nil, err } - tok, err := f.GetToken() - if err != nil { - return nil, err - } + tok := f.GetToken() pos := fromProtocolPosition(tok, params.Position) ident, err := source.Identifier(ctx, s.view, f, pos) if err != nil { @@ -280,10 +274,7 @@ func (s *server) SignatureHelp(ctx context.Context, params *protocol.TextDocumen if err != nil { return nil, err } - tok, err := f.GetToken() - if err != nil { - return nil, err - } + tok := f.GetToken() pos := fromProtocolPosition(tok, params.Position) info, err := source.SignatureHelp(ctx, f, pos) if err != nil { @@ -301,10 +292,7 @@ func (s *server) Definition(ctx context.Context, params *protocol.TextDocumentPo if err != nil { return nil, err } - tok, err := f.GetToken() - if err != nil { - return nil, err - } + tok := f.GetToken() pos := fromProtocolPosition(tok, params.Position) ident, err := source.Identifier(ctx, s.view, f, pos) if err != nil { @@ -322,10 +310,7 @@ func (s *server) TypeDefinition(ctx context.Context, params *protocol.TextDocume if err != nil { return nil, err } - tok, err := f.GetToken() - if err != nil { - return nil, err - } + tok := f.GetToken() pos := fromProtocolPosition(tok, params.Position) ident, err := source.Identifier(ctx, s.view, f, pos) if err != nil { diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index a748a175..a6799ad2 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -47,14 +47,8 @@ type finder func(types.Object, float64, []CompletionItem) []CompletionItem // completion. For instance, some clients may tolerate imperfect matches as // valid completion results, since users may make typos. func Completion(ctx context.Context, f File, pos token.Pos) (items []CompletionItem, prefix string, err error) { - file, err := f.GetAST() - if err != nil { - return nil, "", err - } - pkg, err := f.GetPackage() - if err != nil { - return nil, "", err - } + file := f.GetAST() + pkg := f.GetPackage() path, _ := astutil.PathEnclosingInterval(file, pos, pos) if path == nil { return nil, "", fmt.Errorf("cannot find node enclosing position") diff --git a/internal/lsp/source/definition.go b/internal/lsp/source/definition.go index bac247be..bc233c0c 100644 --- a/internal/lsp/source/definition.go +++ b/internal/lsp/source/definition.go @@ -50,14 +50,8 @@ func Identifier(ctx context.Context, v View, f File, pos token.Pos) (*Identifier func (i *IdentifierInfo) Hover(q types.Qualifier) (string, error) { if q == nil { - fAST, err := i.File.GetAST() - if err != nil { - return "", err - } - pkg, err := i.File.GetPackage() - if err != nil { - return "", err - } + fAST := i.File.GetAST() + pkg := i.File.GetPackage() q = qualifier(fAST, pkg.Types, pkg.TypesInfo) } return types.ObjectString(i.Declaration.Object, q), nil @@ -65,14 +59,8 @@ func (i *IdentifierInfo) Hover(q types.Qualifier) (string, error) { // identifier checks a single position for a potential identifier. func identifier(ctx context.Context, v View, f File, pos token.Pos) (*IdentifierInfo, error) { - fAST, err := f.GetAST() - if err != nil { - return nil, err - } - pkg, err := f.GetPackage() - if err != nil { - return nil, err - } + fAST := f.GetAST() + pkg := f.GetPackage() path, _ := astutil.PathEnclosingInterval(fAST, pos, pos) result := &IdentifierInfo{ File: f, @@ -109,6 +97,7 @@ func identifier(ctx context.Context, v View, f File, pos token.Pos) (*Identifier } } } + var err error if result.Declaration.Range, err = objToRange(ctx, v, result.Declaration.Object); err != nil { return nil, err } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 27f5bad2..9fa26b7a 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -50,10 +50,7 @@ func Diagnostics(ctx context.Context, v View, uri URI) (map[string][]Diagnostic, if err != nil { return nil, err } - pkg, err := f.GetPackage() - if err != nil { - return nil, err - } + pkg := f.GetPackage() // Prepare the reports we will send for this package. reports := make(map[string][]Diagnostic) for _, filename := range pkg.GoFiles { @@ -82,10 +79,7 @@ func Diagnostics(ctx context.Context, v View, uri URI) (map[string][]Diagnostic, if err != nil { continue } - diagTok, err := diagFile.GetToken() - if err != nil { - continue - } + diagTok := diagFile.GetToken() content, err := diagFile.Read() if err != nil { continue diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 78aaab71..ac5a304c 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -21,10 +21,7 @@ import ( // Format formats a file with a given range. func Format(ctx context.Context, f File, rng Range) ([]TextEdit, error) { - fAST, err := f.GetAST() - if err != nil { - return nil, err - } + fAST := f.GetAST() path, exact := astutil.PathEnclosingInterval(fAST, rng.Start, rng.End) if !exact || len(path) == 0 { return nil, fmt.Errorf("no exact AST node matching the specified range") @@ -50,10 +47,7 @@ func Format(ctx context.Context, f File, rng Range) ([]TextEdit, error) { // of Go used to build the LSP server will determine how it formats code. // This should be acceptable for all users, who likely be prompted to rebuild // the LSP server on each Go release. - fset, err := f.GetFileSet() - if err != nil { - return nil, err - } + fset := f.GetFileSet() buf := &bytes.Buffer{} if err := format.Node(buf, fset, node); err != nil { return nil, err @@ -62,10 +56,7 @@ func Format(ctx context.Context, f File, rng Range) ([]TextEdit, error) { if err != nil { return nil, err } - tok, err := f.GetToken() - if err != nil { - return nil, err - } + tok := f.GetToken() return computeTextEdits(rng, tok, string(content), buf.String()), nil } diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index db78b126..4478fed2 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -25,14 +25,8 @@ type ParameterInformation struct { } func SignatureHelp(ctx context.Context, f File, pos token.Pos) (*SignatureInformation, error) { - fAST, err := f.GetAST() - if err != nil { - return nil, err - } - pkg, err := f.GetPackage() - if err != nil { - return nil, err - } + fAST := f.GetAST() + pkg := f.GetPackage() // Find a call expression surrounding the query position. var callExpr *ast.CallExpr diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 49538270..eaac87dd 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -27,10 +27,10 @@ type View interface { // building blocks for most queries. Users of the source package can abstract // the loading of packages into their own caching systems. type File interface { - GetAST() (*ast.File, error) - GetFileSet() (*token.FileSet, error) - GetPackage() (*packages.Package, error) - GetToken() (*token.File, error) + GetAST() *ast.File + GetFileSet() *token.FileSet + GetPackage() *packages.Package + GetToken() *token.File Read() ([]byte, error) }