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>
diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go
index 00ccd02..9b676c9 100644
--- a/internal/lsp/cmd/cmd.go
+++ b/internal/lsp/cmd/cmd.go
@@ -339,7 +339,7 @@
}
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
}
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 851d9bd..4e4f7fc 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -6,6 +6,7 @@
import (
"context"
+ "strings"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
@@ -40,8 +41,10 @@
// 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 @@
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,
diff --git a/internal/lsp/link.go b/internal/lsp/link.go
index d437fb1..fd8eb49 100644
--- a/internal/lsp/link.go
+++ b/internal/lsp/link.go
@@ -6,6 +6,7 @@
import (
"context"
+ "fmt"
"strconv"
"golang.org/x/tools/internal/lsp/protocol"
@@ -21,6 +22,10 @@
}
// 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()
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 172f77c..e6cffa0 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -594,7 +594,7 @@
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 @@
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)
diff --git a/internal/lsp/protocol/span.go b/internal/lsp/protocol/span.go
index 9045b2c..f41fba8 100644
--- a/internal/lsp/protocol/span.go
+++ b/internal/lsp/protocol/span.go
@@ -23,10 +23,17 @@
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,
}
}
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index f6ae675..2977928 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -8,6 +8,7 @@
"bytes"
"context"
"fmt"
+ "strings"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/asmdecl"
@@ -72,6 +73,12 @@
}
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 @@
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 @@
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())
diff --git a/internal/lsp/source/diagnostics_test.go b/internal/lsp/source/diagnostics_test.go
new file mode 100644
index 0000000..8d37a52
--- /dev/null
+++ b/internal/lsp/source/diagnostics_test.go
@@ -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())
+ }
+ })
+ }
+}
diff --git a/internal/lsp/util.go b/internal/lsp/util.go
index 0d61de5..aa8be47 100644
--- a/internal/lsp/util.go
+++ b/internal/lsp/util.go
@@ -51,11 +51,14 @@
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
}
