From b5fc9d354d99c98f615cc62f71ea3e0d336c51fc Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Mon, 20 Jul 2020 02:32:34 -0400 Subject: [PATCH] internal/lsp: add parse errors as diagnostics even when parsing fails When a package consists of files that only fail to parse, we fail to associate parse errors with the package, and therefore return no diagnostics. To address this, we need to associate the errors with the package. This involves adding a *token.File to the parseGoData so that error messages and positions can be properly extracted, even when there is no associated AST. Fixes golang/go#39763 Change-Id: I5620821b9bcbeb499c498f9061dcf487d159bed5 Reviewed-on: https://go-review.googlesource.com/c/tools/+/243579 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/cache/check.go | 13 +++++++++++ internal/lsp/cache/errors.go | 21 +++++++++--------- internal/lsp/cache/parse.go | 28 +++++++++++++++++++++--- internal/lsp/regtest/diagnostics_test.go | 20 +++++++++++++++++ 4 files changed, 68 insertions(+), 14 deletions(-) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 5f8d57e0d3..d14e2346f6 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -331,6 +331,19 @@ func typeCheck(ctx context.Context, fset *token.FileSet, m *metadata, mode sourc // race to Unsafe.completed. return pkg, nil } else if len(files) == 0 { // not the unsafe package, no parsed files + // Try to attach errors messages to the file as much as possible. + var found bool + for _, e := range rawErrors { + srcErr, err := sourceError(ctx, fset, pkg, e) + if err != nil { + continue + } + found = true + pkg.errors = append(pkg.errors, srcErr) + } + if found { + return pkg, nil + } return nil, errors.Errorf("no parsed files for package %s, expected: %s, errors: %v, list errors: %v", pkg.pkgPath, pkg.compiledGoFiles, actualErrors, rawErrors) } else { pkg.types = types.NewPackage(string(m.pkgPath), string(m.name)) diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go index eff1c80728..a2d034fd0c 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -208,30 +208,29 @@ func typeErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, pos toke } func scannerErrorRange(fset *token.FileSet, pkg *pkg, posn token.Position) (span.Span, error) { - ph, _, err := source.FindFileInPackage(pkg, span.URIFromPath(posn.Filename)) + pgh, _, err := source.FindFileInPackage(pkg, span.URIFromPath(posn.Filename)) if err != nil { return span.Span{}, err } - file, _, _, _, err := ph.Cached() - if err != nil { - return span.Span{}, err + data, err := pgh.(*parseGoHandle).cached() + if data.tok == nil { + if err != nil { + return span.Span{}, err + } + return span.Span{}, errors.Errorf("no token.File for %s: %v", pgh.File().URI(), data.err) } - tok := fset.File(file.Pos()) - if tok == nil { - return span.Span{}, errors.Errorf("no token.File for %s", ph.File().URI()) - } - pos := tok.Pos(posn.Offset) + pos := data.tok.Pos(posn.Offset) return span.NewRange(fset, pos, pos).Span() } // spanToRange converts a span.Span to a protocol.Range, // assuming that the span belongs to the package whose diagnostics are being computed. func spanToRange(pkg *pkg, spn span.Span) (protocol.Range, error) { - ph, _, err := source.FindFileInPackage(pkg, spn.URI()) + pgh, _, err := source.FindFileInPackage(pkg, spn.URI()) if err != nil { return protocol.Range{}, err } - _, _, m, _, err := ph.Cached() + _, _, m, _, err := pgh.Cached() if err != nil { return protocol.Range{}, err } diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index 75d8888cfe..45ca298493 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -43,6 +43,7 @@ type parseGoData struct { memoize.NoCopy ast *ast.File + tok *token.File mapper *protocol.ColumnMapper // Source code used to build the AST. It may be different from the @@ -112,12 +113,20 @@ func (pgh *parseGoHandle) parse(ctx context.Context) (*parseGoData, error) { } func (pgh *parseGoHandle) Cached() (*ast.File, []byte, *protocol.ColumnMapper, error, error) { + data, err := pgh.cached() + if err != nil { + return nil, nil, nil, nil, err + } + return data.ast, data.src, data.mapper, data.parseError, data.err +} + +func (pgh *parseGoHandle) cached() (*parseGoData, error) { v := pgh.handle.Cached() if v == nil { - return nil, nil, nil, nil, errors.Errorf("no cached AST for %s", pgh.file.URI()) + return nil, errors.Errorf("no cached AST for %s", pgh.file.URI()) } data := v.(*parseGoData) - return data.ast, data.src, data.mapper, data.parseError, data.err + return data, nil } func (pgh *parseGoHandle) PosToDecl(ctx context.Context) (map[token.Pos]ast.Decl, error) { @@ -278,7 +287,19 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod if file != nil { tok = fset.File(file.Pos()) if tok == nil { - return &parseGoData{err: errors.Errorf("successfully parsed but no token.File for %s (%v)", fh.URI(), parseError)} + tok = fset.AddFile(fh.URI().Filename(), -1, len(buf)) + tok.SetLinesForContent(buf) + return &parseGoData{ + ast: file, + src: buf, + tok: tok, + mapper: &protocol.ColumnMapper{ + URI: fh.URI(), + Content: buf, + Converter: span.NewTokenConverter(fset, tok), + }, + parseError: parseError, + } } // Fix any badly parsed parts of the AST. @@ -319,6 +340,7 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod return &parseGoData{ src: buf, ast: file, + tok: tok, mapper: m, parseError: parseError, fixed: fixed, diff --git a/internal/lsp/regtest/diagnostics_test.go b/internal/lsp/regtest/diagnostics_test.go index e7e66b3328..22d2a1c521 100644 --- a/internal/lsp/regtest/diagnostics_test.go +++ b/internal/lsp/regtest/diagnostics_test.go @@ -1098,3 +1098,23 @@ func main() { ) }) } + +// Reproduces golang/go#39763. +func TestInvalidPackageName(t *testing.T) { + testenv.NeedsGo1Point(t, 15) + + const pkgDefault = ` +-- go.mod -- +module mod.com +-- main.go -- +package default + +func main() {} +` + runner.Run(t, pkgDefault, func(t *testing.T, env *Env) { + env.OpenFile("main.go") + env.Await( + env.DiagnosticAtRegexp("main.go", "default"), + ) + }) +}