internal/lsp: use URIs instead of FileIdentity in errors
This change modifies the source.Error type to have a URI instead of a
FileIdentity associated with an error.
Change-Id: I8056bdc881dd3ec43af02cca1366815c0bc6dfcd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214586
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go
index 9be9809..56462e1 100644
--- a/internal/lsp/cache/errors.go
+++ b/internal/lsp/cache/errors.go
@@ -7,7 +7,6 @@
import (
"bytes"
"context"
- "fmt"
"go/scanner"
"go/token"
"go/types"
@@ -105,12 +104,8 @@
if err != nil {
return nil, err
}
- ph, err := pkg.File(spn.URI())
- if err != nil {
- return nil, fmt.Errorf("finding file for error %q: %v", msg, err)
- }
return &source.Error{
- File: ph.File().Identity(),
+ URI: spn.URI(),
Range: rng,
Message: msg,
Kind: kind,
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 6c41056..07f05ee 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -89,20 +89,11 @@
s.deliveredMu.Lock()
defer s.deliveredMu.Unlock()
- for identity, diagnostics := range reports {
+ for fileID, diagnostics := range reports {
// Don't deliver diagnostics if the context has already been canceled.
if ctx.Err() != nil {
break
}
- // Rather than using the identity provided in the report,
- // get the FileHandle directly through the snapshot.
- // This prevents us from using cached file versions.
- fh, err := snapshot.GetFile(identity.URI)
- if err != nil {
- log.Error(ctx, "publishReports: failed to get FileHandle", err, telemetry.File.Of(identity.URI))
- continue
- }
- fileID := fh.Identity()
// Pre-sort diagnostics to avoid extra work when we compare them.
source.SortDiagnostics(diagnostics)
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 6929f9b..0f27b43 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -94,19 +94,27 @@
continue
}
// If no file is associated with the error, pick an open file from the package.
- if e.File.URI.Filename() == "" {
+ if e.URI.Filename() == "" {
for _, ph := range pkg.CompiledGoFiles() {
if snapshot.View().Session().IsOpen(ph.File().Identity().URI) {
- e.File = ph.File().Identity()
+ e.URI = ph.File().Identity().URI
}
}
}
- clearReports(snapshot, reports, e.File)
+ fh, err := snapshot.GetFile(e.URI)
+ if err != nil {
+ return nil, warningMsg, err
+ }
+ clearReports(snapshot, reports, fh.Identity())
}
// Run diagnostics for the package that this URI belongs to.
- if !diagnostics(ctx, snapshot, pkg, reports) && withAnalysis {
+ hadDiagnostics, err := diagnostics(ctx, snapshot, reports, pkg)
+ if err != nil {
+ return nil, warningMsg, err
+ }
+ if !hadDiagnostics && withAnalysis {
// If we don't have any list, parse, or type errors, run analyses.
- if err := analyses(ctx, snapshot, ph, disabledAnalyses, reports); err != nil {
+ if err := analyses(ctx, snapshot, reports, ph, disabledAnalyses); err != nil {
// Exit early if the context has been canceled.
if err == context.Canceled {
return nil, warningMsg, err
@@ -131,7 +139,7 @@
for _, fh := range pkg.CompiledGoFiles() {
clearReports(snapshot, reports, fh.File().Identity())
}
- diagnostics(ctx, snapshot, pkg, reports)
+ diagnostics(ctx, snapshot, reports, pkg)
}
return reports, warningMsg, nil
}
@@ -140,7 +148,7 @@
listErrors, parseErrors, typeErrors []*Diagnostic
}
-func diagnostics(ctx context.Context, snapshot Snapshot, pkg Package, reports map[FileIdentity][]Diagnostic) bool {
+func diagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, pkg Package) (bool, error) {
ctx, done := trace.StartSpan(ctx, "source.diagnostics", telemetry.Package.Of(pkg.ID()))
_ = ctx // circumvent SA4006
defer done()
@@ -152,10 +160,14 @@
Range: e.Range,
Severity: protocol.SeverityError,
}
- set, ok := diagSets[e.File]
+ fh, err := snapshot.GetFile(e.URI)
+ if err != nil {
+ return false, err
+ }
+ set, ok := diagSets[fh.Identity()]
if !ok {
set = &diagnosticSet{}
- diagSets[e.File] = set
+ diagSets[fh.Identity()] = set
}
switch e.Kind {
case ParseError:
@@ -181,12 +193,12 @@
if len(diags) > 0 {
nonEmptyDiagnostics = true
}
- addReports(ctx, reports, snapshot, fileID, diags...)
+ addReports(ctx, snapshot, reports, fileID, diags...)
}
- return nonEmptyDiagnostics
+ return nonEmptyDiagnostics, nil
}
-func analyses(ctx context.Context, snapshot Snapshot, ph PackageHandle, disabledAnalyses map[string]struct{}, reports map[FileIdentity][]Diagnostic) error {
+func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, ph PackageHandle, disabledAnalyses map[string]struct{}) error {
var analyzers []*analysis.Analyzer
for _, a := range snapshot.View().Options().Analyzers {
if _, ok := disabledAnalyses[a.Name]; ok {
@@ -209,7 +221,11 @@
if onlyDeletions(e.SuggestedFixes) {
tags = append(tags, protocol.Unnecessary)
}
- addReports(ctx, reports, snapshot, e.File, &Diagnostic{
+ fh, err := snapshot.GetFile(e.URI)
+ if err != nil {
+ return err
+ }
+ addReports(ctx, snapshot, reports, fh.Identity(), &Diagnostic{
Range: e.Range,
Message: e.Message,
Source: e.Category,
@@ -229,7 +245,7 @@
reports[fileID] = []Diagnostic{}
}
-func addReports(ctx context.Context, reports map[FileIdentity][]Diagnostic, snapshot Snapshot, fileID FileIdentity, diagnostics ...*Diagnostic) error {
+func addReports(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, fileID FileIdentity, diagnostics ...*Diagnostic) error {
if snapshot.View().Ignore(fileID.URI) {
return nil
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 14b7d13..83e659f 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -334,7 +334,7 @@
}
type Error struct {
- File FileIdentity
+ URI span.URI
Range protocol.Range
Kind ErrorKind
Message string
@@ -354,7 +354,7 @@
)
func (e *Error) Error() string {
- return fmt.Sprintf("%s:%s: %s", e.File, e.Range, e.Message)
+ return fmt.Sprintf("%s:%s: %s", e.URI, e.Range, e.Message)
}
type BuiltinPackage interface {
