cybercyst/go
1
0
Fork 0
mirror of https://github.com/golang/go.git synced 2025-05-05 15:43:04 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

internal/lsp: distinguish parse errors from actual errors

Browse Source 
Parse errors need to be treated separately from actual errors when
parsing a file. Parse errors are treated more like values, whereas
actual errors should not be propagated to the user. This enables us to
delete some of the special handling for context.Canceled errors.

Change-Id: I93a02f22b3f54beccbd6bcf26f04bb8da0202c25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195997
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This commit is contained in:
Rebecca Stambler 2019-09-17 11:19:11 -04:00
parent d997db1b28
commit 1cc9451822
20 changed files with 142 additions and 170 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
internal/lsp/cache/builtin.go vendored
View File

@ -43,8 +43,8 @@ func (view *view) buildBuiltinPackage(ctx context.Context) error {
fh := view.session.GetFile(span.FileURI(filename))
ph := view.session.cache.ParseGoHandle(fh, source.ParseFull)
view.builtin.files = append(view.builtin.files, ph)
file, _, err := ph.Parse(ctx)
if file == nil {
file, _, _, err := ph.Parse(ctx)
if err != nil {
return err
}
files[filename] = file

11
internal/lsp/cache/check.go vendored
View File

@ -258,15 +258,12 @@ func (imp *importer) typeCheck(ctx context.Context, cph *checkPackageHandle, m *
go func(i int, ph source.ParseGoHandle) {
defer wg.Done()
files[i], _, parseErrors[i] = ph.Parse(ctx)
files[i], _, parseErrors[i], _ = ph.Parse(ctx)
}(i, ph)
}
wg.Wait()
for _, err := range parseErrors {
if err == context.Canceled {
return nil, errors.Errorf("parsing files for %s: %v", m.pkgPath, err)
}
if err != nil {
imp.view.session.cache.appendPkgError(pkg, err)
}
@ -350,9 +347,9 @@ func (imp *importer) cachePerFile(ctx context.Context, gof *goFile, ph source.Pa
}
gof.cphs[cph.m.id] = cph
file, _, err := ph.Parse(ctx)
if file == nil {
return errors.Errorf("no AST for %s: %v", ph.File().Identity().URI, err)
file, _, _, err := ph.Parse(ctx)
if err != nil {
return err
}
gof.imports = file.Imports
return nil

42
internal/lsp/cache/load.go vendored
View File

@ -88,25 +88,33 @@ func (view *view) load(ctx context.Context, f *goFile, fh source.FileHandle) ([]
// checkMetadata determines if we should run go/packages.Load for this file.
// If yes, update the metadata for the file and its package.
func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandle) ([]*metadata, error) {
func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandle) (metadata []*metadata, err error) {
// Check if we need to re-run go/packages before loading the package.
f.mu.Lock()
runGopackages := v.shouldRunGopackages(ctx, f, fh)
metadata := f.metadata()
f.mu.Unlock()
var runGopackages bool
func() {
f.mu.Lock()
defer f.mu.Unlock()
runGopackages, err = v.shouldRunGopackages(ctx, f, fh)
metadata = f.metadata()
}()
if err != nil {
return nil, err
}
// The package metadata is correct as-is, so just return it.
if !runGopackages {
return metadata, nil
}
// Check if the context has been canceled before calling packages.Load.
// Don't bother running go/packages if the context has been canceled.
if ctx.Err() != nil {
return nil, ctx.Err()
}
ctx, done := trace.StartSpan(ctx, "packages.Load", telemetry.File.Of(f.filename()))
defer done()
pkgs, err := packages.Load(v.Config(ctx), fmt.Sprintf("file=%s", f.filename()))
if len(pkgs) == 0 {
if err == nil {
@ -185,18 +193,14 @@ func sameSet(x, y map[packagePath]struct{}) bool {
// shouldRunGopackages reparses a file's package and import declarations to
// determine if they have changed.
// It assumes that the caller holds the lock on the f.mu lock.
func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, fh source.FileHandle) (result bool) {
func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, fh source.FileHandle) (result bool, err error) {
if len(f.meta) == 0 || len(f.missingImports) > 0 {
return true
return true, nil
}
// Get file content in case we don't already have it.
parsed, _, err := v.session.cache.ParseGoHandle(fh, source.ParseHeader).Parse(ctx)
if err == context.Canceled {
log.Error(ctx, "parsing file header", err, tag.Of("file", f.URI()))
return false
}
if parsed == nil {
return true
parsed, _, _, err := v.session.cache.ParseGoHandle(fh, source.ParseHeader).Parse(ctx)
if err != nil {
return false, err
}
// Check if the package's name has changed, by checking if this is a filename
// we already know about, and if so, check if its package name has changed.
@ -204,21 +208,21 @@ func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, fh source.Fil
for _, uri := range m.files {
if span.CompareURI(uri, f.URI()) == 0 {
if m.name != parsed.Name.Name {
return true
return true, nil
}
}
}
}
// If the package's imports have changed, re-run `go list`.
if len(f.imports) != len(parsed.Imports) {
return true
return true, nil
}
for i, importSpec := range f.imports {
if importSpec.Path.Value != parsed.Imports[i].Path.Value {
return true
return true, nil
}
}
return false
return false, nil
}
type importGraph struct {

75
internal/lsp/cache/parse.go vendored
View File

@ -41,9 +41,10 @@ type parseGoHandle struct {
type parseGoData struct {
memoize.NoCopy
ast *ast.File
mapper *protocol.ColumnMapper
err error
ast *ast.File
parseError error // errors associated with parsing the file
mapper *protocol.ColumnMapper
err error
}
func (c *cache) ParseGoHandle(fh source.FileHandle, mode source.ParseMode) source.ParseGoHandle {
@ -53,18 +54,7 @@ func (c *cache) ParseGoHandle(fh source.FileHandle, mode source.ParseMode) sourc
}
h := c.store.Bind(key, func(ctx context.Context) interface{} {
data := &parseGoData{}
data.ast, data.err = parseGo(ctx, c, fh, mode)
tok := c.FileSet().File(data.ast.Pos())
if tok == nil {
return data
}
uri := fh.Identity().URI
content, _, err := fh.Read(ctx)
if err != nil {
data.err = err
return data
}
data.mapper = newColumnMapper(uri, c.FileSet(), tok, content)
data.ast, data.mapper, data.parseError, data.err = parseGo(ctx, c, fh, mode)
return data
})
return &parseGoHandle{
@ -73,13 +63,6 @@ func (c *cache) ParseGoHandle(fh source.FileHandle, mode source.ParseMode) sourc
mode: mode,
}
}
func newColumnMapper(uri span.URI, fset *token.FileSet, tok *token.File, content []byte) *protocol.ColumnMapper {
return &protocol.ColumnMapper{
URI: uri,
Converter: span.NewTokenConverter(fset, tok),
Content: content,
}
}
func (h *parseGoHandle) File() source.FileHandle {
return h.file
@ -89,22 +72,22 @@ func (h *parseGoHandle) Mode() source.ParseMode {
return h.mode
}
func (h *parseGoHandle) Parse(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error) {
func (h *parseGoHandle) Parse(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error, error) {
v := h.handle.Get(ctx)
if v == nil {
return nil, nil, ctx.Err()
return nil, nil, nil, ctx.Err()
}
data := v.(*parseGoData)
return data.ast, data.mapper, data.err
return data.ast, data.mapper, data.parseError, data.err
}
func (h *parseGoHandle) Cached(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error) {
func (h *parseGoHandle) Cached(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error, error) {
v := h.handle.Cached()
if v == nil {
return nil, nil, errors.Errorf("no cached value for %s", h.file.Identity().URI)
return nil, nil, nil, errors.Errorf("no cached AST for %s", h.file.Identity().URI)
}
data := v.(*parseGoData)
return data.ast, data.mapper, data.err
return data.ast, data.mapper, data.parseError, data.err
}
func hashParseKey(ph source.ParseGoHandle) string {
@ -122,13 +105,13 @@ func hashParseKeys(phs []source.ParseGoHandle) string {
return hashContents(b.Bytes())
}
func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.ParseMode) (*ast.File, error) {
func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.ParseMode) (file *ast.File, mapper *protocol.ColumnMapper, parseError error, err error) {
ctx, done := trace.StartSpan(ctx, "cache.parseGo", telemetry.File.Of(fh.Identity().URI.Filename()))
defer done()
buf, _, err := fh.Read(ctx)
if err != nil {
return nil, err
return nil, nil, nil, err
}
parseLimit <- struct{}{}
defer func() { <-parseLimit }()
@ -136,21 +119,37 @@ func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.Pa
if mode == source.ParseHeader {
parserMode = parser.ImportsOnly | parser.ParseComments
}
ast, err := parser.ParseFile(c.fset, fh.Identity().URI.Filename(), buf, parserMode)
if ast != nil {
file, parseError = parser.ParseFile(c.fset, fh.Identity().URI.Filename(), buf, parserMode)
if file != nil {
if mode == source.ParseExported {
trimAST(ast)
trimAST(file)
}
// Fix any badly parsed parts of the AST.
tok := c.fset.File(ast.Pos())
if err := fix(ctx, ast, tok, buf); err != nil {
tok := c.fset.File(file.Pos())
if err := fix(ctx, file, tok, buf); err != nil {
log.Error(ctx, "failed to fix AST", err)
}
}
if ast == nil {
return nil, err
// If the file is nil only due to parse errors,
// the parse errors are the actual errors.
if file == nil {
return nil, nil, parseError, parseError
}
return ast, err
tok := c.FileSet().File(file.Pos())
if tok == nil {
return nil, nil, parseError, err
}
uri := fh.Identity().URI
content, _, err := fh.Read(ctx)
if err != nil {
return nil, nil, parseError, err
}
m := &protocol.ColumnMapper{
URI: uri,
Converter: span.NewTokenConverter(c.FileSet(), tok),
Content: content,
}
return file, m, parseError, nil
}
// trimAST clears any part of the AST not relevant to type checking

13
internal/lsp/cache/pkg.go vendored
View File

@ -151,11 +151,20 @@ func (pkg *pkg) Files() []source.ParseGoHandle {
return pkg.files
}
func (pkg *pkg) File(uri span.URI) (source.ParseGoHandle, error) {
for _, ph := range pkg.Files() {
if ph.File().Identity().URI == uri {
return ph, nil
}
}
return nil, errors.Errorf("no ParseGoHandle for %s", uri)
}
func (pkg *pkg) GetSyntax(ctx context.Context) []*ast.File {
var syntax []*ast.File
for _, ph := range pkg.files {
file, _, _ := ph.Cached(ctx)
if file != nil {
file, _, _, err := ph.Cached(ctx)
if err == nil {
syntax = append(syntax, file)
}
}

5
internal/lsp/diagnostics.go
View File

@ -23,6 +23,7 @@ func (s *Server) diagnostics(view source.View, uri span.URI) error {
defer done()
ctx = telemetry.File.With(ctx, uri)
f, err := view.GetFile(ctx, uri)
if err != nil {
return err
@ -45,8 +46,9 @@ func (s *Server) diagnostics(view source.View, uri span.URI) error {
if s.undelivered == nil {
s.undelivered = make(map[span.URI][]source.Diagnostic)
}
log.Error(ctx, "failed to deliver diagnostic (will retry)", err, telemetry.File)
s.undelivered[uri] = diagnostics
log.Error(ctx, "failed to deliver diagnostic (will retry)", err, telemetry.File)
continue
}
// In case we had old, undelivered diagnostics.
@ -58,6 +60,7 @@ func (s *Server) diagnostics(view source.View, uri span.URI) error {
if err := s.publishDiagnostics(ctx, uri, diagnostics); err != nil {
log.Error(ctx, "failed to deliver diagnostic for (will not retry)", err, telemetry.File)
}
// If we fail to deliver the same diagnostics twice, just give up.
delete(s.undelivered, uri)
}

4
internal/lsp/link.go
View File

@ -28,8 +28,8 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink
return nil, err
}
fh := f.Handle(ctx)
file, m, err := view.Session().Cache().ParseGoHandle(fh, source.ParseFull).Parse(ctx)
if file == nil {
file, m, _, err := view.Session().Cache().ParseGoHandle(fh, source.ParseFull).Parse(ctx)
if err != nil {
return nil, err
}
var links []protocol.DocumentLink

12
internal/lsp/source/completion.go
View File

@ -376,14 +376,12 @@ func Completion(ctx context.Context, view View, f GoFile, pos protocol.Position,
if err != nil {
return nil, nil, err
}
var ph ParseGoHandle
for _, h := range pkg.Files() {
if h.File().Identity().URI == f.URI() {
ph = h
}
ph, err := pkg.File(f.URI())
if err != nil {
return nil, nil, err
}
file, m, err := ph.Cached(ctx)
if file == nil {
file, m, _, err := ph.Cached(ctx)
if err != nil {
return nil, nil, err
}
spn, err := m.PointSpan(pos)

5
internal/lsp/source/completion_format.go
View File

@ -123,14 +123,13 @@ func (c *completer) item(cand candidate) (CompletionItem, error) {
if !pos.IsValid() {
return item, nil
}
uri := span.FileURI(pos.Filename)
ph, pkg, err := c.pkg.FindFile(c.ctx, uri)
if err != nil {
return CompletionItem{}, err
}
file, _, err := ph.Cached(c.ctx)
if file == nil {
file, _, _, err := ph.Cached(c.ctx)
if err != nil {
return CompletionItem{}, err
}
if !(file.Pos() <= obj.Pos() && obj.Pos() <= file.End()) {

22
internal/lsp/source/diagnostics.go
View File

@ -8,7 +8,6 @@ import (
"bytes"
"context"
"fmt"
"go/ast"
"strings"
"golang.org/x/tools/go/analysis"
@ -181,22 +180,15 @@ func diagnostics(ctx context.Context, view View, pkg Package, reports map[span.U
// 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, view View, pkg Package, spn span.Span, isTypeError bool) (protocol.Range, error) {
var (
fh FileHandle
file *ast.File
m *protocol.ColumnMapper
err error
)
for _, ph := range pkg.Files() {
if ph.File().Identity().URI == spn.URI() {
fh = ph.File()
file, m, err = ph.Cached(ctx)
}
}
if file == nil {
ph, err := pkg.File(spn.URI())
if err != nil {
return protocol.Range{}, err
}
data, _, err := fh.Read(ctx)
_, m, _, err := ph.Cached(ctx)
if err != nil {
return protocol.Range{}, err
}
data, _, err := ph.File().Read(ctx)
if err != nil {
return protocol.Range{}, err
}

5
internal/lsp/source/folding_range.go
View File

@ -19,9 +19,8 @@ type FoldingRangeInfo struct {
func FoldingRange(ctx context.Context, view View, f GoFile, lineFoldingOnly bool) (ranges []*FoldingRangeInfo, err error) {
// TODO(suzmue): consider limiting the number of folding ranges returned, and
// implement a way to prioritize folding ranges in that case.
fh := f.Handle(ctx)
ph := view.Session().Cache().ParseGoHandle(fh, ParseFull)
file, m, err := ph.Parse(ctx)
ph := view.Session().Cache().ParseGoHandle(f.Handle(ctx), ParseFull)
file, m, _, err := ph.Parse(ctx)
if err != nil {
return nil, err
}

30
internal/lsp/source/format.go
View File

@ -37,17 +37,12 @@ func Format(ctx context.Context, view View, f File) ([]protocol.TextEdit, error)
if err != nil {
return nil, err
}
var ph ParseGoHandle
for _, h := range pkg.Files() {
if h.File().Identity().URI == f.URI() {
ph = h
}
}
if ph == nil {
ph, err := pkg.File(f.URI())
if err != nil {
return nil, err
}
file, m, err := ph.Parse(ctx)
if file == nil {
file, m, _, err := ph.Parse(ctx)
if err != nil {
return nil, err
}
if hasListErrors(pkg.GetErrors()) || hasParseErrors(pkg, f.URI()) {
@ -102,13 +97,8 @@ func Imports(ctx context.Context, view View, f GoFile, rng span.Range) ([]protoc
if hasListErrors(pkg.GetErrors()) {
return nil, errors.Errorf("%s has list errors, not running goimports", f.URI())
}
var ph ParseGoHandle
for _, h := range pkg.Files() {
if h.File().Identity().URI == f.URI() {
ph = h
}
}
if ph == nil {
ph, err := pkg.File(f.URI())
if err != nil {
return nil, err
}
options := &imports.Options{
@ -133,8 +123,8 @@ func Imports(ctx context.Context, view View, f GoFile, rng span.Range) ([]protoc
if err != nil {
return nil, err
}
_, m, err := ph.Parse(ctx)
if m == nil {
_, m, _, err := ph.Parse(ctx)
if err != nil {
return nil, err
}
return computeTextEdits(ctx, ph.File(), m, string(formatted))
@ -201,8 +191,8 @@ func AllImportsFixes(ctx context.Context, view View, f File) (edits []protocol.T
if err != nil {
return err
}
_, m, err := ph.Parse(ctx)
if m == nil {
_, m, _, err := ph.Parse(ctx)
if err != nil {
return err
}
edits, err = computeTextEdits(ctx, ph.File(), m, string(formatted))

7
internal/lsp/source/highlight.go
View File

@ -23,10 +23,9 @@ func Highlight(ctx context.Context, view View, uri span.URI, pos protocol.Positi
if err != nil {
return nil, err
}
fh := f.Handle(ctx)
ph := view.Session().Cache().ParseGoHandle(fh, ParseFull)
file, m, err := ph.Parse(ctx)
if file == nil {
ph := view.Session().Cache().ParseGoHandle(f.Handle(ctx), ParseFull)
file, m, _, err := ph.Parse(ctx)
if err != nil {
return nil, err
}
spn, err := m.PointSpan(pos)

15
internal/lsp/source/identifier.go
View File

@ -57,15 +57,12 @@ func Identifier(ctx context.Context, view View, f GoFile, pos protocol.Position)
if err != nil {
return nil, err
}
var ph ParseGoHandle
for _, h := range cph.Files() {
if h.File().Identity().URI == f.URI() {
ph = h
break
}
ph, err := pkg.File(f.URI())
if err != nil {
return nil, err
}
file, m, err := ph.Cached(ctx)
if file == nil {
file, m, _, err := ph.Cached(ctx)
if err != nil {
return nil, err
}
spn, err := m.PointSpan(pos)
@ -246,7 +243,7 @@ func objToNode(ctx context.Context, view View, pkg Package, obj types.Object) (a
if err != nil {
return nil, err
}
declAST, _, err := ph.Cached(ctx)
declAST, _, _, err := ph.Cached(ctx)
if declAST == nil {
return nil, err
}

15
internal/lsp/source/rename.go
View File

@ -176,16 +176,12 @@ func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string)
// getPkgName gets the pkg name associated with an identifer representing
// the import path in an import spec.
func (i *IdentifierInfo) getPkgName(ctx context.Context) (*IdentifierInfo, error) {
var (
file *ast.File
err error
)
for _, ph := range i.pkg.Files() {
if ph.File().Identity().URI == i.File.File().Identity().URI {
file, _, err = ph.Cached(ctx)
}
ph, err := i.pkg.File(i.URI())
if err != nil {
return nil, err
}
if file == nil {
file, _, _, err := ph.Cached(ctx)
if err != nil {
return nil, err
}
var namePos token.Pos
@ -198,7 +194,6 @@ func (i *IdentifierInfo) getPkgName(ctx context.Context) (*IdentifierInfo, error
if !namePos.IsValid() {
return nil, errors.Errorf("import spec not found for %q", i.Name)
}
// Look for the object defined at NamePos.
for _, obj := range i.pkg.GetTypesInfo().Defs {
pkgName, ok := obj.(*types.PkgName)

13
internal/lsp/source/signature_help.go
View File

@ -40,15 +40,12 @@ func SignatureHelp(ctx context.Context, view View, f GoFile, pos protocol.Positi
if err != nil {
return nil, err
}
var ph ParseGoHandle
for _, h := range pkg.Files() {
if h.File().Identity().URI == f.URI() {
ph = h
break
}
ph, err := pkg.File(f.URI())
if err != nil {
return nil, err
}
file, m, err := ph.Cached(ctx)
if file == nil {
file, m, _, err := ph.Cached(ctx)
if err != nil {
return nil, err
}
spn, err := m.PointSpan(pos)

4
internal/lsp/source/suggested_fix.go
View File

@ -18,8 +18,8 @@ func getCodeActions(ctx context.Context, view View, pkg Package, diag analysis.D
if err != nil {
return nil, err
}
_, m, err := ph.Cached(ctx)
if m == nil {
_, m, _, err := ph.Cached(ctx)
if err != nil {
return nil, err
}
mrng, err := posToRange(ctx, view, m, e.Pos, e.End)

14
internal/lsp/source/symbols.go
View File

@ -27,16 +27,12 @@ func DocumentSymbols(ctx context.Context, view View, f GoFile) ([]protocol.Docum
if err != nil {
return nil, err
}
var (
file *ast.File
m *protocol.ColumnMapper
)
for _, ph := range pkg.Files() {
if ph.File().Identity().URI == f.URI() {
file, m, err = ph.Cached(ctx)
}
ph, err := pkg.File(f.URI())
if err != nil {
return nil, err
}
if file == nil {
file, m, _, err := ph.Cached(ctx)
if err != nil {
return nil, err
}

11
internal/lsp/source/util.go
View File

@ -92,8 +92,8 @@ func IsGenerated(ctx context.Context, view View, uri span.URI) bool {
return false
}
ph := view.Session().Cache().ParseGoHandle(f.Handle(ctx), ParseHeader)
parsed, _, err := ph.Parse(ctx)
if parsed == nil {
parsed, _, _, err := ph.Parse(ctx)
if err != nil {
return false
}
tok := view.Session().Cache().FileSet().File(parsed.Pos())
@ -175,11 +175,8 @@ func posToMapper(ctx context.Context, view View, pkg Package, pos token.Pos) (*p
if err != nil {
return nil, err
}
_, m, err := ph.Cached(ctx)
if m == nil {
return nil, err
}
return m, nil
_, m, _, err := ph.Cached(ctx)
return m, err
}
// Matches cgo generated comment as well as the proposed standard:

5
internal/lsp/source/view.go
View File

@ -80,10 +80,10 @@ type ParseGoHandle interface {
// Parse returns the parsed AST for the file.
// If the file is not available, returns nil and an error.
Parse(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error)
Parse(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error, error)
// Cached returns the AST for this handle, if it has already been stored.
Cached(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error)
Cached(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error, error)
}
// ParseMode controls the content of the AST produced when parsing a source file.
@ -288,6 +288,7 @@ type Package interface {
ID() string
PkgPath() string
Files() []ParseGoHandle
File(uri span.URI) (ParseGoHandle, error)
GetSyntax(context.Context) []*ast.File
GetErrors() []packages.Error
GetTypes() *types.Package