internal/lsp: fix swallowed package errors

If a package has an error that makes it completely unparsable,
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

Author: evandigby

internal/lsp/diagnostics.go

@@ -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.log.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,

internal/lsp/link.go

@@ -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()

internal/lsp/protocol/span.go

@@ -31,6 +31,14 @@ func NewColumnMapper(uri span.URI, fset *token.FileSet, f *token.File, content [
 	}
 }
 
+func NewContentColumnMapper(uri span.URI, fn string, content []byte) *ColumnMapper {
+	return &ColumnMapper{
+		URI:       uri,
+		Converter: span.NewContentConverter(fn, content),
+		Content:   content,
+	}
+}
+
 func (m *ColumnMapper) Location(s span.Span) (Location, error) {
 	rng, err := m.Range(s)
 	if err != nil {

internal/lsp/source/diagnostics.go

@@ -68,6 +68,12 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag
 	for _, filename := range pkg.GetFilenames() {
 		reports[span.FileURI(filename)] = []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.
@@ -109,7 +115,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)
 		}
@@ -153,6 +159,14 @@ func analyses(ctx context.Context, v View, pkg Package, reports map[span.URI][]D
 	return nil
 }
 
+func packageErrorSpan(pkgErr packages.Error) span.Span {
+	if pkgErr.Pos == "" {
+		return span.ParseErrorMessage(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())

internal/lsp/util.go

@@ -51,11 +51,18 @@ 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())
+
+	var m *protocol.ColumnMapper
+	if tok := f.GetToken(ctx); tok == nil {
+		fn, err := f.URI().Filename()
+		if err != nil {
+			return nil, nil, err
+		}
+		m = protocol.NewContentColumnMapper(uri, fn, f.GetContent(ctx))
+	} else {
+		m = protocol.NewColumnMapper(f.URI(), f.GetFileSet(ctx), tok, f.GetContent(ctx))
 	}
-	m := protocol.NewColumnMapper(f.URI(), f.GetFileSet(ctx), tok, f.GetContent(ctx))
+
 	return f, m, nil
 }
 

internal/span/parse.go

@@ -10,6 +10,21 @@ import (
 	"unicode/utf8"
 )
 
+// ParseErrorMessage 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 ParseErrorMessage(input string) Span {
+	input = strings.TrimSpace(input)
+
+	msgIndex := strings.Index(input, ": ")
+	if msgIndex < 0 {
+		return Parse(input)
+	}
+
+	return Parse(input[:msgIndex])
+}
+
 // Parse returns the location represented by the input.
 // All inputs are valid locations, as they can always be a pure filename.
 // The returned span will be normalized, and thus if printed may produce a

internal/span/parse_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 span
+
+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 := ParseErrorMessage(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())
+			}
+		})
+	}
+}