From 0bb5a05de81d0029e2aac6a6c97b991de96bbf77 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Mon, 21 Oct 2019 17:49:45 -0400 Subject: [PATCH] internal/lsp: use the AST to get correct ranges Fixes golang/go#29150 Change-Id: I0cb8be25f7f40f7f8becedf2e0002c58620c72da Reviewed-on: https://go-review.googlesource.com/c/tools/+/202542 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/cache/errors.go | 117 +++++++++++++++++++++++------- internal/lsp/tests/diagnostics.go | 7 +- 2 files changed, 92 insertions(+), 32 deletions(-) diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go index 8757efb25b..c03986f7da 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -3,15 +3,19 @@ package cache import ( "bytes" "context" + "go/ast" "go/scanner" + "go/token" "go/types" "strings" "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" + errors "golang.org/x/xerrors" ) func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, error) { @@ -23,6 +27,7 @@ func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, e fixes []source.SuggestedFix related []source.RelatedInformation ) + fset := pkg.snapshot.view.session.cache.fset switch e := e.(type) { case packages.Error: if e.Pos == "" { @@ -32,21 +37,32 @@ func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, e } msg = e.Msg kind = toSourceErrorKind(e.Kind) + case *scanner.Error: msg = e.Msg kind = source.ParseError - spn = span.Parse(e.Pos.String()) + spn, err = scannerErrorRange(ctx, fset, pkg, e.Pos) + if err != nil { + return nil, err + } + case scanner.ErrorList: // The first parser error is likely the root cause of the problem. - if e.Len() > 0 { - spn = span.Parse(e[0].Pos.String()) - msg = e[0].Msg - kind = source.ParseError + if e.Len() <= 0 { + return nil, errors.Errorf("no errors in %v", e) } + msg = e[0].Msg + kind = source.ParseError + spn, err = scannerErrorRange(ctx, fset, pkg, e[0].Pos) + if err != nil { + return nil, err + } + case types.Error: - spn = span.Parse(pkg.snapshot.view.session.cache.fset.Position(e.Pos).String()) msg = e.Msg kind = source.TypeError + spn, err = typeErrorRange(ctx, fset, pkg, e.Pos) + case *analysis.Diagnostic: spn, err = span.NewRange(pkg.snapshot.view.session.cache.fset, e.Pos, e.End).Span() if err != nil { @@ -64,7 +80,7 @@ func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, e return nil, err } } - rng, err := spanToRange(ctx, pkg, spn, kind == source.TypeError) + rng, err := spanToRange(ctx, pkg, spn) if err != nil { return nil, err } @@ -88,7 +104,7 @@ func suggestedFixes(ctx context.Context, pkg *pkg, diag *analysis.Diagnostic) ([ if err != nil { return nil, err } - rng, err := spanToRange(ctx, pkg, spn, false) + rng, err := spanToRange(ctx, pkg, spn) if err != nil { return nil, err } @@ -112,7 +128,7 @@ func relatedInformation(ctx context.Context, pkg *pkg, diag *analysis.Diagnostic if err != nil { return nil, err } - rng, err := spanToRange(ctx, pkg, spn, false) + rng, err := spanToRange(ctx, pkg, spn) if err != nil { return nil, err } @@ -138,10 +154,74 @@ func toSourceErrorKind(kind packages.ErrorKind) source.ErrorKind { } } +func typeErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, pos token.Pos) (span.Span, error) { + spn, err := span.NewRange(fset, pos, pos).Span() + if err != nil { + return span.Span{}, err + } + posn := fset.Position(pos) + ph, _, err := pkg.FindFile(ctx, span.FileURI(posn.Filename)) + if err != nil { + return span.Span{}, err + } + file, m, _, err := ph.Cached(ctx) + if err != nil { + return span.Span{}, err + } + path, _ := astutil.PathEnclosingInterval(file, pos, pos) + if len(path) > 0 { + if s, err := span.NewRange(fset, path[0].Pos(), path[0].End()).Span(); err == nil { + return s, nil + } + } + s, err := spn.WithOffset(m.Converter) + if err != nil { + return span.Span{}, err + } + data, _, err := ph.File().Read(ctx) + if err != nil { + return span.Span{}, err + } + start := s.Start() + offset := start.Offset() + if offset < len(data) { + if width := bytes.IndexAny(data[offset:], " \n,():;[]"); width > 0 { + return span.New(spn.URI(), start, span.NewPoint(start.Line(), start.Column()+width, offset+width)), nil + } + } + return spn, nil +} + +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)) + if err != nil { + return span.Span{}, err + } + file, _, _, err := ph.Cached(ctx) + if err != nil { + return span.Span{}, err + } + tok := fset.File(file.Pos()) + if tok == nil { + return span.Span{}, errors.Errorf("no token.File for %s", ph.File().Identity().URI) + } + pos := tok.Pos(posn.Offset) + path, _ := astutil.PathEnclosingInterval(file, pos, pos) + if len(path) > 0 { + switch n := path[0].(type) { + case *ast.BadDecl, *ast.BadExpr, *ast.BadStmt: + if s, err := span.NewRange(fset, n.Pos(), n.End()).Span(); err == nil { + return s, nil + } + } + } + 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(ctx context.Context, pkg *pkg, spn span.Span, isTypeError bool) (protocol.Range, error) { - ph, err := pkg.File(spn.URI()) +func spanToRange(ctx context.Context, pkg *pkg, spn span.Span) (protocol.Range, error) { + ph, _, err := pkg.FindFile(ctx, spn.URI()) if err != nil { return protocol.Range{}, err } @@ -149,21 +229,6 @@ func spanToRange(ctx context.Context, pkg *pkg, spn span.Span, isTypeError bool) if err != nil { return protocol.Range{}, err } - if spn.IsPoint() && isTypeError { - data, _, err := ph.File().Read(ctx) - if err != nil { - return protocol.Range{}, err - } - if s, err := spn.WithOffset(m.Converter); err == nil { - start := s.Start() - offset := start.Offset() - if offset < len(data) { - if width := bytes.IndexAny(data[offset:], " \n,():;[]"); width > 0 { - spn = span.New(spn.URI(), start, span.NewPoint(start.Line(), start.Column()+width, offset+width)) - } - } - } - } return m.Range(spn) } diff --git a/internal/lsp/tests/diagnostics.go b/internal/lsp/tests/diagnostics.go index f4611ef0b2..92f948c1f7 100644 --- a/internal/lsp/tests/diagnostics.go +++ b/internal/lsp/tests/diagnostics.go @@ -38,12 +38,7 @@ func DiffDiagnostics(want, got []source.Diagnostic) string { if protocol.ComparePosition(w.Range.Start, g.Range.Start) != 0 { return summarizeDiagnostics(i, want, got, "incorrect Start got %v want %v", g.Range.Start, w.Range.Start) } - // Special case for diagnostics on parse errors. - if strings.Contains(string(g.URI), "noparse") { - if protocol.ComparePosition(g.Range.Start, g.Range.End) != 0 || protocol.ComparePosition(w.Range.Start, g.Range.End) != 0 { - return summarizeDiagnostics(i, want, got, "incorrect End got %v want %v", g.Range.End, w.Range.Start) - } - } else if !protocol.IsPoint(g.Range) { // Accept any 'want' range if the diagnostic returns a zero-length range. + if !protocol.IsPoint(g.Range) { // Accept any 'want' range if the diagnostic returns a zero-length range. if protocol.ComparePosition(w.Range.End, g.Range.End) != 0 { return summarizeDiagnostics(i, want, got, "incorrect End got %v want %v", g.Range.End, w.Range.End) }