Skip to content

Commit b42efcd

Browse files
committed
internal/lsp: try to parse diagnostics out of go list errors
This change attempts to parse diagnostics out of `go list` error messages so that we can present them in a better way to the user. This approach is definitely tailored to the unknown revision error described in golang/go#38232, but we can modify it to handle other cases as well. Fixes golang/go#38232 Change-Id: I0b0a8c39a189a127dc36894a25614535c804a3f0 Reviewed-on: https://go-review.googlesource.com/c/tools/+/242477 Run-TryBot: Rebecca Stambler <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]>
1 parent 0a5cd10 commit b42efcd

File tree

5 files changed

+162
-26
lines changed

5 files changed

+162
-26
lines changed

internal/lsp/cache/load.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
124124
event.Log(ctx, "go/packages.Load", tag.Snapshot.Of(s.ID()), tag.Directory.Of(cfg.Dir), tag.Query.Of(query), tag.PackageCount.Of(len(pkgs)))
125125
}
126126
if len(pkgs) == 0 {
127-
return err
127+
return errors.Errorf("%v: %w", err, source.PackagesLoadError)
128128
}
129129

130130
for _, pkg := range pkgs {

internal/lsp/diagnostics.go

+57-19
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"golang.org/x/tools/internal/lsp/protocol"
1717
"golang.org/x/tools/internal/lsp/source"
1818
"golang.org/x/tools/internal/xcontext"
19+
"golang.org/x/xerrors"
1920
)
2021

2122
type diagnosticKey struct {
@@ -61,13 +62,13 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
6162
var wg sync.WaitGroup
6263

6364
// Diagnose the go.mod file.
64-
reports, err := mod.Diagnostics(ctx, snapshot)
65-
if err != nil {
66-
event.Error(ctx, "warning: diagnose go.mod", err, tag.Directory.Of(snapshot.View().Folder().Filename()))
67-
}
65+
reports, modErr := mod.Diagnostics(ctx, snapshot)
6866
if ctx.Err() != nil {
6967
return nil, nil
7068
}
69+
if modErr != nil {
70+
event.Error(ctx, "warning: diagnose go.mod", modErr, tag.Directory.Of(snapshot.View().Folder().Filename()))
71+
}
7172
for id, diags := range reports {
7273
if id.URI == "" {
7374
event.Error(ctx, "missing URI for module diagnostics", fmt.Errorf("empty URI"), tag.Directory.Of(snapshot.View().Folder().Filename()))
@@ -83,7 +84,19 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
8384
// Diagnose all of the packages in the workspace.
8485
wsPackages, err := snapshot.WorkspacePackages(ctx)
8586
if err != nil {
86-
s.handleFatalErrors(ctx, snapshot, err)
87+
// Try constructing a more helpful error message out of this error.
88+
if s.handleFatalErrors(ctx, snapshot, modErr, err) {
89+
return nil, nil
90+
}
91+
msg := `The code in the workspace failed to compile (see the error message below).
92+
If you believe this is a mistake, please file an issue: https://github.com/golang/go/issues/new.`
93+
event.Error(ctx, msg, err, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder()))
94+
if err := s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
95+
Type: protocol.Error,
96+
Message: fmt.Sprintf("%s\n%v", msg, err),
97+
}); err != nil {
98+
event.Error(ctx, "ShowMessage failed", err, tag.Directory.Of(snapshot.View().Folder()))
99+
}
87100
return nil, nil
88101
}
89102
var shows *protocol.ShowMessageParams
@@ -229,8 +242,15 @@ func toProtocolDiagnostics(diagnostics []*source.Diagnostic) []protocol.Diagnost
229242
return reports
230243
}
231244

232-
func (s *Server) handleFatalErrors(ctx context.Context, snapshot source.Snapshot, err error) {
233-
switch err {
245+
func (s *Server) handleFatalErrors(ctx context.Context, snapshot source.Snapshot, modErr, loadErr error) bool {
246+
modURI := snapshot.View().ModFile()
247+
248+
// We currently only have workarounds for errors associated with modules.
249+
if modURI == "" {
250+
return false
251+
}
252+
253+
switch loadErr {
234254
case source.InconsistentVendoring:
235255
item, err := s.client.ShowMessageRequest(ctx, &protocol.ShowMessageRequestParams{
236256
Type: protocol.Error,
@@ -240,27 +260,45 @@ See https://github.com/golang/go/issues/39164 for more detail on this issue.`,
240260
{Title: "go mod vendor"},
241261
},
242262
})
243-
if item == nil || err != nil {
263+
// If the user closes the pop-up, don't show them further errors.
264+
if item == nil {
265+
return true
266+
}
267+
if err != nil {
244268
event.Error(ctx, "go mod vendor ShowMessageRequest failed", err, tag.Directory.Of(snapshot.View().Folder()))
245-
return
269+
return true
246270
}
247-
modURI := snapshot.View().ModFile()
248271
if err := s.directGoModCommand(ctx, protocol.URIFromSpanURI(modURI), "mod", []string{"vendor"}...); err != nil {
249272
if err := s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
250273
Type: protocol.Error,
251274
Message: fmt.Sprintf(`"go mod vendor" failed with %v`, err),
252275
}); err != nil {
253-
event.Error(ctx, "ShowMessage failed", err)
276+
if err != nil {
277+
event.Error(ctx, "go mod vendor ShowMessage failed", err, tag.Directory.Of(snapshot.View().Folder()))
278+
}
254279
}
255280
}
256-
default:
257-
msg := "failed to load workspace packages, skipping diagnostics"
258-
event.Error(ctx, msg, err, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder()))
259-
if err := s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
260-
Type: protocol.Error,
261-
Message: fmt.Sprintf("%s: %v", msg, err),
262-
}); err != nil {
263-
event.Error(ctx, "ShowMessage failed", err, tag.Directory.Of(snapshot.View().Folder()))
281+
return true
282+
}
283+
// If there is a go.mod-related error, as well as a workspace load error,
284+
// there is likely an issue with the go.mod file. Try to parse the error
285+
// message and create a diagnostic.
286+
if modErr == nil {
287+
return false
288+
}
289+
if xerrors.Is(loadErr, source.PackagesLoadError) {
290+
fh, err := snapshot.GetFile(ctx, modURI)
291+
if err != nil {
292+
return false
264293
}
294+
diag, err := mod.ExtractGoCommandError(ctx, snapshot, fh, loadErr)
295+
if err != nil {
296+
return false
297+
}
298+
s.publishReports(ctx, snapshot, map[diagnosticKey][]*source.Diagnostic{
299+
{id: fh.Identity()}: {diag},
300+
})
301+
return true
265302
}
303+
return false
266304
}

internal/lsp/mod/diagnostics.go

+96-3
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,17 @@ package mod
88

99
import (
1010
"context"
11+
"fmt"
12+
"regexp"
13+
"strings"
1114

15+
"golang.org/x/mod/modfile"
16+
"golang.org/x/mod/module"
1217
"golang.org/x/tools/internal/event"
1318
"golang.org/x/tools/internal/lsp/debug/tag"
1419
"golang.org/x/tools/internal/lsp/protocol"
1520
"golang.org/x/tools/internal/lsp/source"
21+
"golang.org/x/tools/internal/span"
1622
)
1723

1824
func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.FileIdentity][]*source.Diagnostic, error) {
@@ -32,16 +38,16 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.File
3238
if err == source.ErrTmpModfileUnsupported {
3339
return nil, nil
3440
}
41+
reports := map[source.FileIdentity][]*source.Diagnostic{
42+
fh.Identity(): {},
43+
}
3544
if err != nil {
3645
return nil, err
3746
}
3847
diagnostics, err := mth.Tidy(ctx)
3948
if err != nil {
4049
return nil, err
4150
}
42-
reports := map[source.FileIdentity][]*source.Diagnostic{
43-
fh.Identity(): {},
44-
}
4551
for _, e := range diagnostics {
4652
diag := &source.Diagnostic{
4753
Message: e.Message,
@@ -119,3 +125,90 @@ func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, diags []proto
119125
func sameDiagnostic(d protocol.Diagnostic, e source.Error) bool {
120126
return d.Message == e.Message && protocol.CompareRange(d.Range, e.Range) == 0 && d.Source == e.Category
121127
}
128+
129+
var moduleAtVersionRe = regexp.MustCompile(`(?P<module>.*)@(?P<version>.*)`)
130+
131+
// ExtractGoCommandError tries to parse errors that come from the go command
132+
// and shape them into go.mod diagnostics.
133+
func ExtractGoCommandError(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, loadErr error) (*source.Diagnostic, error) {
134+
// We try to match module versions in error messages. Some examples:
135+
//
136+
// err: exit status 1: stderr: go: [email protected]: reading example.com/@v/v1.2.2.mod: no such file or directory
137+
// exit status 1: go: github.com/cockroachdb/apd/[email protected]: reading github.com/cockroachdb/apd/go.mod at revision v2.0.72: unknown revision v2.0.72
138+
//
139+
// We split on colons and attempt to match on something that matches
140+
// module@version. If we're able to find a match, we try to find anything
141+
// that matches it in the go.mod file.
142+
var v module.Version
143+
for _, s := range strings.Split(loadErr.Error(), ":") {
144+
s = strings.TrimSpace(s)
145+
match := moduleAtVersionRe.FindStringSubmatch(s)
146+
if match == nil || len(match) < 3 {
147+
continue
148+
}
149+
v.Path = match[1]
150+
v.Version = match[2]
151+
if err := module.Check(v.Path, v.Version); err == nil {
152+
break
153+
}
154+
}
155+
pmh, err := snapshot.ParseModHandle(ctx, fh)
156+
if err != nil {
157+
return nil, err
158+
}
159+
parsed, m, _, err := pmh.Parse(ctx)
160+
if err != nil {
161+
return nil, err
162+
}
163+
toDiagnostic := func(line *modfile.Line) (*source.Diagnostic, error) {
164+
rng, err := rangeFromPositions(fh.URI(), m, line.Start, line.End)
165+
if err != nil {
166+
return nil, err
167+
}
168+
return &source.Diagnostic{
169+
Message: loadErr.Error(),
170+
Range: rng,
171+
Severity: protocol.SeverityError,
172+
}, nil
173+
}
174+
// Check if there are any require, exclude, or replace statements that
175+
// match this module version.
176+
for _, req := range parsed.Require {
177+
if req.Mod != v {
178+
continue
179+
}
180+
return toDiagnostic(req.Syntax)
181+
}
182+
for _, ex := range parsed.Exclude {
183+
if ex.Mod != v {
184+
continue
185+
}
186+
return toDiagnostic(ex.Syntax)
187+
}
188+
for _, rep := range parsed.Replace {
189+
if rep.New != v && rep.Old != v {
190+
continue
191+
}
192+
return toDiagnostic(rep.Syntax)
193+
}
194+
return nil, fmt.Errorf("no diagnostics for %v", loadErr)
195+
}
196+
197+
func rangeFromPositions(uri span.URI, m *protocol.ColumnMapper, s, e modfile.Position) (protocol.Range, error) {
198+
toPoint := func(offset int) (span.Point, error) {
199+
l, c, err := m.Converter.ToPosition(offset)
200+
if err != nil {
201+
return span.Point{}, err
202+
}
203+
return span.NewPoint(l, c, offset), nil
204+
}
205+
start, err := toPoint(s.Byte)
206+
if err != nil {
207+
return protocol.Range{}, err
208+
}
209+
end, err := toPoint(e.Byte)
210+
if err != nil {
211+
return protocol.Range{}, err
212+
}
213+
return m.Range(span.New(uri, start, end))
214+
}

internal/lsp/regtest/modfile_test.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,8 @@ require (
325325

326326
// Reproduces golang/go#38232.
327327
func TestUnknownRevision(t *testing.T) {
328+
testenv.NeedsGo1Point(t, 14)
329+
328330
const unknown = `
329331
-- go.mod --
330332
module mod.com
@@ -350,7 +352,7 @@ func main() {
350352
)
351353
env.OpenFile("go.mod")
352354
env.Await(
353-
SomeShowMessage("failed to load workspace packages, skipping diagnostics"),
355+
env.DiagnosticAtRegexp("go.mod", "example.com v1.2.2"),
354356
)
355357
env.RegexpReplace("go.mod", "v1.2.2", "v1.2.3")
356358
env.Editor.SaveBufferWithoutActions(env.Ctx, "go.mod") // go.mod changes must be on disk
@@ -387,7 +389,7 @@ func main() {
387389
env.RegexpReplace("go.mod", "v1.2.3", "v1.2.2")
388390
env.Editor.SaveBufferWithoutActions(env.Ctx, "go.mod") // go.mod changes must be on disk
389391
env.Await(
390-
SomeShowMessage("failed to load workspace packages, skipping diagnostics"),
392+
env.DiagnosticAtRegexp("go.mod", "example.com v1.2.2"),
391393
)
392394
env.RegexpReplace("go.mod", "v1.2.2", "v1.2.3")
393395
env.Editor.SaveBufferWithoutActions(env.Ctx, "go.mod") // go.mod changes must be on disk

internal/lsp/source/view.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -516,4 +516,7 @@ func (e *Error) Error() string {
516516
return fmt.Sprintf("%s:%s: %s", e.URI, e.Range, e.Message)
517517
}
518518

519-
var InconsistentVendoring = errors.New("inconsistent vendoring")
519+
var (
520+
InconsistentVendoring = errors.New("inconsistent vendoring")
521+
PackagesLoadError = errors.New("packages.Load error")
522+
)

0 commit comments

Comments
 (0)