close_issue_1
/
tools
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

internal/lsp: fix swallowed package errors 

If a package has an error that makes it completely unparseable, such as containing a .go file with no "package" statement, the error was previously unreported. Such errors would manifest as other errors.

Fixes golang/go#31712

Change-Id: I11b8d0e2e4d64b03fbcb4c35e7f0b02fccc83fad
GitHub-Last-Rev: 1581cbe36c269dd964f0b9226dbd63b1650c2a5b
GitHub-Pull-Request: golang/tools#102
Reviewed-on: https://go-review.googlesource.com/c/tools/+/177605
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This commit is contained in:
Evan Digby 2019-05-17 17:45:50 +00:00 committed by Ian Cottrell
parent f217c98fae
commit d88f79806b
8 changed files with 116 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
internal/lsp/cmd/cmd.go
View File

@ -339,7 +339,7 @@ func (c *cmdClient) getFile(ctx context.Context, uri span.URI) *cmdFile {
}
f := c.fset.AddFile(fname, -1, len(content))
f.SetLinesForContent(content)
file.mapper = protocol.NewColumnMapper(uri, c.fset, f, content)
file.mapper = protocol.NewColumnMapper(uri, fname, c.fset, f, content)
}
return file
}

9
internal/lsp/diagnostics.go
View File

@ -6,6 +6,7 @@ package lsp
import (
"context"
"strings"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
@ -40,8 +41,10 @@ func (s *Server) Diagnostics(ctx context.Context, view source.View, uri span.URI
// Anytime we compute diagnostics, make sure to also send along any
// undelivered ones (only for remaining URIs).
for uri, diagnostics := range s.undelivered {
s.publishDiagnostics(ctx, view, uri, diagnostics)
err := s.publishDiagnostics(ctx, view, uri, diagnostics)
if err != nil {
s.session.Logger().Errorf(ctx, "failed to deliver diagnostic for %s: %v", uri, err)
}
// If we fail to deliver the same diagnostics twice, just give up.
delete(s.undelivered, uri)
}
@ -78,7 +81,7 @@ func toProtocolDiagnostics(ctx context.Context, v source.View, diagnostics []sou
return nil, err
}
reports = append(reports, protocol.Diagnostic{
Message: diag.Message,
Message: strings.TrimSpace(diag.Message), // go list returns errors prefixed by newline
Range: rng,
Severity: severity,
Source: diag.Source,

5
internal/lsp/link.go
View File

@ -6,6 +6,7 @@ package lsp
import (
"context"
"fmt"
"strconv"
"golang.org/x/tools/internal/lsp/protocol"
@ -21,6 +22,10 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink
}
// find the import block
ast := f.GetAST(ctx)
if ast == nil {
return nil, fmt.Errorf("no AST for %v", uri)
}
var result []protocol.DocumentLink
for _, imp := range ast.Imports {
spn, err := span.NewRange(f.GetFileSet(ctx), imp.Pos(), imp.End()).Span()

4
internal/lsp/lsp_test.go
View File

@ -594,7 +594,7 @@ func (r *runner) mapper(uri span.URI) (*protocol.ColumnMapper, error) {
if err != nil {
return nil, err
}
return protocol.NewColumnMapper(uri, fset, f, content), nil
return protocol.NewColumnMapper(uri, filename, fset, f, content), nil
}
func TestBytesOffset(t *testing.T) {
@ -624,7 +624,7 @@ func TestBytesOffset(t *testing.T) {
fset := token.NewFileSet()
f := fset.AddFile(fname, -1, len(test.text))
f.SetLinesForContent([]byte(test.text))
mapper := protocol.NewColumnMapper(span.FileURI(fname), fset, f, []byte(test.text))
mapper := protocol.NewColumnMapper(span.FileURI(fname), fname, fset, f, []byte(test.text))
got, err := mapper.Point(test.pos)
if err != nil && test.want != -1 {
t.Errorf("unexpected error: %v", err)

11
internal/lsp/protocol/span.go
View File

@ -23,10 +23,17 @@ func NewURI(uri span.URI) string {
return string(uri)
}
func NewColumnMapper(uri span.URI, fset *token.FileSet, f *token.File, content []byte) *ColumnMapper {
func NewColumnMapper(uri span.URI, fn string, fset *token.FileSet, f *token.File, content []byte) *ColumnMapper {
var converter *span.TokenConverter
if f == nil {
converter = span.NewContentConverter(fn, content)
} else {
converter = span.NewTokenConverter(fset, f)
}
return &ColumnMapper{
URI: uri,
Converter: span.NewTokenConverter(fset, f),
Converter: converter,
Content: content,
}
}

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

@ -8,6 +8,7 @@ import (
"bytes"
"context"
"fmt"
"strings"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/asmdecl"
@ -72,6 +73,12 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag
}
reports[uri] = []Diagnostic{}
}
// Prepare reports for package errors
for _, pkgErr := range pkg.GetErrors() {
reports[packageErrorSpan(pkgErr).URI()] = []Diagnostic{}
}
// Run diagnostics for the package that this URI belongs to.
if !diagnostics(ctx, v, pkg, reports) {
// If we don't have any list, parse, or type errors, run analyses.
@ -117,7 +124,7 @@ func diagnostics(ctx context.Context, v View, pkg Package, reports map[span.URI]
diags = listErrors
}
for _, diag := range diags {
spn := span.Parse(diag.Pos)
spn := packageErrorSpan(diag)
if spn.IsPoint() && diag.Kind == packages.TypeError {
spn = pointToSpan(ctx, v, spn)
}
@ -161,6 +168,29 @@ func analyses(ctx context.Context, v View, pkg Package, reports map[span.URI][]D
return nil
}
// parseDiagnosticMessage attempts to parse a standard error message by stripping off the trailing error message.
// Works only on errors where the message is prefixed by ": ".
// e.g.:
// attributes.go:13:1: expected 'package', found 'type'
func parseDiagnosticMessage(input string) span.Span {
input = strings.TrimSpace(input)
msgIndex := strings.Index(input, ": ")
if msgIndex < 0 {
return span.Parse(input)
}
return span.Parse(input[:msgIndex])
}
func packageErrorSpan(pkgErr packages.Error) span.Span {
if pkgErr.Pos == "" {
return parseDiagnosticMessage(pkgErr.Msg)
}
return span.Parse(pkgErr.Pos)
}
func pointToSpan(ctx context.Context, v View, spn span.Span) span.Span {
// Don't set a range if it's anything other than a type error.
f, err := v.GetFile(ctx, spn.URI())

55
internal/lsp/source/diagnostics_test.go Normal file
View File

@ -0,0 +1,55 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package source
import (
"strings"
"testing"
)
func TestParseErrorMessage(t *testing.T) {
tests := []struct {
name string
in string
expectedFileName string
expectedLine int
expectedColumn int
}{
{
name: "from go list output",
in: "\nattributes.go:13:1: expected 'package', found 'type'",
expectedFileName: "attributes.go",
expectedLine: 13,
expectedColumn: 1,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
spn := parseDiagnosticMessage(tt.in)
fn, err := spn.URI().Filename()
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !strings.HasSuffix(fn, tt.expectedFileName) {
t.Errorf("expected filename with suffix %v but got %v", tt.expectedFileName, fn)
}
if !spn.HasPosition() {
t.Fatalf("expected span to have position")
}
pos := spn.Start()
if pos.Line() != tt.expectedLine {
t.Errorf("expected line %v but got %v", tt.expectedLine, pos.Line())
}
if pos.Column() != tt.expectedColumn {
t.Errorf("expected line %v but got %v", tt.expectedLine, pos.Line())
}
})
}
}

11
internal/lsp/util.go
View File

@ -51,11 +51,14 @@ func getSourceFile(ctx context.Context, v source.View, uri span.URI) (source.Fil
if err != nil {
return nil, nil, err
}
tok := f.GetToken(ctx)
if tok == nil {
return nil, nil, fmt.Errorf("no file information for %v", f.URI())
fname, err := f.URI().Filename()
if err != nil {
return nil, nil, err
}
m := protocol.NewColumnMapper(f.URI(), f.GetFileSet(ctx), tok, f.GetContent(ctx))
m := protocol.NewColumnMapper(f.URI(), fname, f.GetFileSet(ctx), f.GetToken(ctx), f.GetContent(ctx))
return f, m, nil
}