diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 8bd21b8d5a..5d637e2d92 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -331,18 +331,15 @@ func typeCheck(ctx context.Context, fset *token.FileSet, m *metadata, mode sourc return nil, ctx.Err() } - // We don't care about a package's errors unless we have parsed it in full. - if mode == source.ParseFull { - for _, e := range rawErrors { - srcErr, err := sourceError(ctx, fset, pkg, e) - if err != nil { - log.Error(ctx, "unable to compute error positions", err, telemetry.Package.Of(pkg.ID())) - continue - } - pkg.errors = append(pkg.errors, srcErr) + // TODO(golang/go#35964): Propagate `go list` errors here when go/packages supports it. + for _, e := range rawErrors { + srcErr, err := sourceError(ctx, fset, pkg, e) + if err != nil { + log.Error(ctx, "unable to compute error positions", err, telemetry.Package.Of(pkg.ID())) + continue } + pkg.errors = append(pkg.errors, srcErr) } - return pkg, nil } diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go index 9afdf6de35..2504c33162 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -38,6 +38,14 @@ func sourceError(ctx context.Context, fset *token.FileSet, pkg *pkg, e interface msg = e.Msg kind = toSourceErrorKind(e.Kind) + // If the range can't be derived from the parseGoListError function, then we do not have a valid position. + if _, err := spanToRange(ctx, pkg, spn); err != nil && e.Pos == "" { + return &source.Error{ + Message: msg, + Kind: kind, + }, nil + } + case *scanner.Error: msg = e.Msg kind = source.ParseError diff --git a/internal/lsp/cmd/test/check.go b/internal/lsp/cmd/test/check.go index 024d987818..65e96889b9 100644 --- a/internal/lsp/cmd/test/check.go +++ b/internal/lsp/cmd/test/check.go @@ -50,8 +50,8 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti expect = fmt.Sprintf("%v:%v: %v", uri.Filename(), diag.Range.Start.Line+1, diag.Message) } expect = r.NormalizePrefix(expect) - // Skip the badimport test for now, until we do a better job with diagnostic ranges. - if strings.Contains(uri.Filename(), "badimport") { + // Skip the badimport and import cycle not allowed test for now, until we do a better job with diagnostic ranges. + if strings.Contains(uri.Filename(), "badimport") || strings.Contains(expect, "import cycle") { continue } _, found := got[expect] @@ -62,8 +62,8 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti } } for extra := range got { - // Skip the badimport test for now, until we do a better job with diagnostic ranges. - if strings.Contains(extra, "badimport") { + // Skip the badimport and import cycle not allowed test for now, until we do a better job with diagnostic ranges. + if strings.Contains(extra, "badimport") || strings.Contains(extra, "import cycle") { continue } t.Errorf("extra diagnostic %q", extra) diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index d53249b694..dbc94502b0 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -71,9 +71,14 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, f File, withAnalysis bo } // Prepare any additional reports for the errors in this package. for _, e := range pkg.GetErrors() { - if e.Kind != ListError { + // We only need to handle lower-level errors. + if !(e.Kind == UnknownError || e.Kind == ListError) { continue } + // If no file is associated with the error, default to the current file. + if e.File.URI.Filename() == "" { + e.File = fh.Identity() + } clearReports(snapshot, reports, e.File) } // Run diagnostics for the package that this URI belongs to. @@ -133,7 +138,7 @@ func diagnostics(ctx context.Context, snapshot Snapshot, pkg Package, reports ma case TypeError: set.typeErrors = append(set.typeErrors, diag) diag.Source = "compiler" - case ListError: + case ListError, UnknownError: set.listErrors = append(set.listErrors, diag) diag.Source = "go list" } diff --git a/internal/lsp/testdata/circular/b/b.go b/internal/lsp/testdata/circular/b/b.go new file mode 100644 index 0000000000..950abba561 --- /dev/null +++ b/internal/lsp/testdata/circular/b/b.go @@ -0,0 +1,9 @@ +package b //@diag("", "go list", "import cycle not allowed") + +import ( + "golang.org/x/tools/internal/lsp/circular/one" +) + +func Test1() { + one.Test() +} diff --git a/internal/lsp/testdata/circular/one/one.go b/internal/lsp/testdata/circular/one/one.go new file mode 100644 index 0000000000..8e74e8b0ed --- /dev/null +++ b/internal/lsp/testdata/circular/one/one.go @@ -0,0 +1,9 @@ +package one + +import ( + "golang.org/x/tools/internal/lsp/circular/b" +) + +func Test() { + b.Test1() +} diff --git a/internal/lsp/testdata/circular/self.go b/internal/lsp/testdata/circular/self.go new file mode 100644 index 0000000000..452ab50772 --- /dev/null +++ b/internal/lsp/testdata/circular/self.go @@ -0,0 +1,12 @@ +package circular //@diag("", "go list", "import cycle not allowed") + +import ( + "golang.org/x/tools/internal/lsp/circular" +) + +func print() { + Test() +} + +func Test() { +} diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index 60d49aabaf..3c952c0c1d 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -6,7 +6,7 @@ DeepCompletionsCount = 5 FuzzyCompletionsCount = 7 RankedCompletionsCount = 26 CaseSensitiveCompletionsCount = 4 -DiagnosticsCount = 22 +DiagnosticsCount = 24 FoldingRangesCount = 2 FormatCount = 6 ImportCount = 7