internal/lsp/cache: split up sourceDiagnostics
In practice, the only code shared among the various switch cases was the
call to spanToRange, which definitely doesn't justify the giant
function. Split it out into per-type functions.
I removed the unused case for types.Error, and a bit of error checking I
believe to be redundant. I don't intend any functional changes.
At this point it might be worth considering moving the functions into
other files, but I don't think it matters that much.
Change-Id: I05b4d86dd37a9ff1887a116183c915c225faf3a7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/297129
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go
index 2b537e0..a06e2db 100644
--- a/internal/lsp/cache/analysis.go
+++ b/internal/lsp/cache/analysis.go
@@ -348,7 +348,7 @@
}
for _, diag := range diagnostics {
- srcDiags, err := sourceDiagnostics(ctx, snapshot, pkg, protocol.SeverityWarning, diag)
+ srcDiags, err := analysisDiagnosticDiagnostics(ctx, snapshot, pkg, protocol.SeverityWarning, diag)
if err != nil {
event.Error(ctx, "unable to compute analysis error position", err, tag.Category.Of(diag.Category), tag.Package.Of(pkg.ID()))
continue
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index e64e7c3..0607a16 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -383,7 +383,7 @@
// Try to attach error messages to the file as much as possible.
var found bool
for _, e := range m.errors {
- srcDiags, err := sourceDiagnostics(ctx, snapshot, pkg, protocol.SeverityError, e)
+ srcDiags, err := goPackagesErrorDiagnostics(ctx, snapshot, pkg, e)
if err != nil {
continue
}
@@ -445,7 +445,7 @@
if len(m.errors) != 0 {
pkg.hasListOrParseErrors = true
for _, e := range m.errors {
- diags, err := sourceDiagnostics(ctx, snapshot, pkg, protocol.SeverityError, e)
+ diags, err := goPackagesErrorDiagnostics(ctx, snapshot, pkg, e)
if err != nil {
event.Error(ctx, "unable to compute positions for list errors", err, tag.Package.Of(pkg.ID()))
continue
@@ -457,7 +457,7 @@
if len(parseErrors) != 0 {
pkg.hasListOrParseErrors = true
for _, e := range parseErrors {
- diags, err := sourceDiagnostics(ctx, snapshot, pkg, protocol.SeverityError, e)
+ diags, err := parseErrorDiagnostics(ctx, snapshot, pkg, e)
if err != nil {
event.Error(ctx, "unable to compute positions for parse errors", err, tag.Package.Of(pkg.ID()))
continue
@@ -472,7 +472,7 @@
for _, e := range expandErrors(typeErrors) {
pkg.hasTypeErrors = true
- diags, err := sourceDiagnostics(ctx, snapshot, pkg, protocol.SeverityError, e)
+ diags, err := typeErrorDiagnostics(ctx, snapshot, pkg, e)
if err != nil {
event.Error(ctx, "unable to compute positions for type errors", err, tag.Package.Of(pkg.ID()))
continue
diff --git a/internal/lsp/cache/error_test.go b/internal/lsp/cache/error_test.go
index d361e03..43cc03f 100644
--- a/internal/lsp/cache/error_test.go
+++ b/internal/lsp/cache/error_test.go
@@ -28,7 +28,7 @@
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- spn := parseGoListError(tt.in)
+ spn := parseGoListError(tt.in, ".")
fn := spn.URI().Filename()
if !strings.HasSuffix(fn, tt.expectedFileName) {
diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go
index b6e7d23..20f35a6 100644
--- a/internal/lsp/cache/errors.go
+++ b/internal/lsp/cache/errors.go
@@ -14,156 +14,157 @@
"strconv"
"strings"
- errors "golang.org/x/xerrors"
-
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"
"golang.org/x/tools/internal/analysisinternal"
- "golang.org/x/tools/internal/event"
- "golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
"golang.org/x/tools/internal/typesinternal"
+ errors "golang.org/x/xerrors"
)
-func sourceDiagnostics(ctx context.Context, snapshot *snapshot, pkg *pkg, severity protocol.DiagnosticSeverity, e interface{}) ([]*source.Diagnostic, error) {
- fset := snapshot.view.session.cache.fset
- var (
- spn span.Span
- err error
- msg string
- code typesinternal.ErrorCode
- diagSrc source.DiagnosticSource
- fixes []source.SuggestedFix
- related []source.RelatedInformation
- )
- switch e := e.(type) {
- case packages.Error:
- diagSrc = toDiagnosticSource(e.Kind)
- var ok bool
- if msg, spn, ok = parseGoListImportCycleError(ctx, snapshot, e, pkg); ok {
- diagSrc = source.TypeError
- break
+func goPackagesErrorDiagnostics(ctx context.Context, snapshot *snapshot, pkg *pkg, e packages.Error) ([]*source.Diagnostic, error) {
+ if msg, spn, ok := parseGoListImportCycleError(ctx, snapshot, e, pkg); ok {
+ rng, err := spanToRange(snapshot, pkg, spn)
+ if err != nil {
+ return nil, err
}
- if e.Pos == "" {
- spn = parseGoListError(e.Msg)
+ return []*source.Diagnostic{{
+ URI: spn.URI(),
+ Range: rng,
+ Severity: protocol.SeverityError,
+ Source: source.TypeError,
+ Message: msg,
+ }}, nil
+ }
- // We may not have been able to parse a valid span.
- if _, err := spanToRange(snapshot, pkg, spn); err != nil {
- var diags []*source.Diagnostic
- for _, cgf := range pkg.compiledGoFiles {
- diags = append(diags, &source.Diagnostic{
- URI: cgf.URI,
- Severity: severity,
- Source: diagSrc,
- Message: msg,
- })
- }
- return diags, nil
+ var spn span.Span
+ if e.Pos == "" {
+ spn = parseGoListError(e.Msg, pkg.m.config.Dir)
+ // We may not have been able to parse a valid span. Apply the errors to all files.
+ if _, err := spanToRange(snapshot, pkg, spn); err != nil {
+ var diags []*source.Diagnostic
+ for _, cgf := range pkg.compiledGoFiles {
+ diags = append(diags, &source.Diagnostic{
+ URI: cgf.URI,
+ Severity: protocol.SeverityError,
+ Source: source.ListError,
+ Message: e.Msg,
+ })
}
- } else {
- spn = span.ParseInDir(e.Pos, pkg.m.config.Dir)
+ return diags, nil
}
- case *scanner.Error:
- msg = e.Msg
- diagSrc = source.ParseError
- spn, err = scannerErrorRange(snapshot, pkg, e.Pos)
- if err != nil {
- if ctx.Err() != nil {
- return nil, ctx.Err()
- }
- event.Error(ctx, "no span for scanner.Error pos", err, tag.Package.Of(pkg.ID()))
- spn = span.Parse(e.Pos.String())
- }
+ } else {
+ spn = span.ParseInDir(e.Pos, pkg.m.config.Dir)
+ }
- case scanner.ErrorList:
- // The first parser error is likely the root cause of the problem.
- if e.Len() <= 0 {
- return nil, errors.Errorf("no errors in %v", e)
- }
- msg = e[0].Msg
- diagSrc = source.ParseError
- spn, err = scannerErrorRange(snapshot, pkg, e[0].Pos)
- if err != nil {
- if ctx.Err() != nil {
- return nil, ctx.Err()
- }
- event.Error(ctx, "no span for scanner.Error pos", err, tag.Package.Of(pkg.ID()))
- spn = span.Parse(e[0].Pos.String())
- }
- case types.Error:
- msg = e.Msg
- diagSrc = source.TypeError
- if !e.Pos.IsValid() {
- return nil, fmt.Errorf("invalid position for type error %v", e)
- }
- code, spn, err = typeErrorData(fset, pkg, e)
- if err != nil {
- return nil, err
- }
- case extendedError:
- perr := e.primary
- msg = perr.Msg
- diagSrc = source.TypeError
- if !perr.Pos.IsValid() {
- return nil, fmt.Errorf("invalid position for type error %v", e)
- }
- code, spn, err = typeErrorData(fset, pkg, e.primary)
- if err != nil {
- return nil, err
- }
- for _, s := range e.secondaries {
- var x source.RelatedInformation
- x.Message = s.Msg
- _, xspn, err := typeErrorData(fset, pkg, s)
- if err != nil {
- return nil, fmt.Errorf("invalid position for type error %v", s)
- }
- x.URI = xspn.URI()
- rng, err := spanToRange(snapshot, pkg, xspn)
- if err != nil {
- return nil, err
- }
- x.Range = rng
- related = append(related, x)
- }
- case *analysis.Diagnostic:
- spn, err = span.NewRange(fset, e.Pos, e.End).Span()
- if err != nil {
- return nil, err
- }
- msg = e.Message
- diagSrc = source.AnalyzerErrorKind(e.Category)
- fixes, err = suggestedAnalysisFixes(snapshot, pkg, e)
- if err != nil {
- return nil, err
- }
- related, err = relatedInformation(snapshot, pkg, e)
- if err != nil {
- return nil, err
- }
- default:
- panic(fmt.Sprintf("%T unexpected", e))
+ rng, err := spanToRange(snapshot, pkg, spn)
+ if err != nil {
+ return nil, err
+ }
+ return []*source.Diagnostic{{
+ URI: spn.URI(),
+ Range: rng,
+ Severity: protocol.SeverityError,
+ Source: source.ListError,
+ Message: e.Msg,
+ }}, nil
+}
+
+func parseErrorDiagnostics(ctx context.Context, snapshot *snapshot, pkg *pkg, errList scanner.ErrorList) ([]*source.Diagnostic, error) {
+ // The first parser error is likely the root cause of the problem.
+ if errList.Len() <= 0 {
+ return nil, errors.Errorf("no errors in %v", errList)
+ }
+ e := errList[0]
+ pgf, err := pkg.File(span.URIFromPath(e.Pos.Filename))
+ if err != nil {
+ return nil, err
+ }
+ pos := pgf.Tok.Pos(e.Pos.Offset)
+ spn, err := span.NewRange(snapshot.FileSet(), pos, pos).Span()
+ if err != nil {
+ return nil, err
}
rng, err := spanToRange(snapshot, pkg, spn)
if err != nil {
return nil, err
}
- sd := &source.Diagnostic{
- URI: spn.URI(),
- Range: rng,
- Severity: severity,
- Source: diagSrc,
- Message: msg,
- Related: related,
- SuggestedFixes: fixes,
+ return []*source.Diagnostic{{
+ URI: spn.URI(),
+ Range: rng,
+ Severity: protocol.SeverityError,
+ Source: source.ParseError,
+ Message: e.Msg,
+ }}, nil
+}
+
+func typeErrorDiagnostics(ctx context.Context, snapshot *snapshot, pkg *pkg, e extendedError) ([]*source.Diagnostic, error) {
+ code, spn, err := typeErrorData(snapshot.FileSet(), pkg, e.primary)
+ if err != nil {
+ return nil, err
+ }
+ rng, err := spanToRange(snapshot, pkg, spn)
+ if err != nil {
+ return nil, err
+ }
+ diag := &source.Diagnostic{
+ URI: spn.URI(),
+ Range: rng,
+ Severity: protocol.SeverityError,
+ Source: source.TypeError,
+ Message: e.primary.Msg,
}
if code != 0 {
- sd.Code = code.String()
- sd.CodeHref = typesCodeHref(snapshot, code)
+ diag.Code = code.String()
+ diag.CodeHref = typesCodeHref(snapshot, code)
}
- return []*source.Diagnostic{sd}, nil
+
+ for _, secondary := range e.secondaries {
+ _, secondarySpan, err := typeErrorData(snapshot.FileSet(), pkg, secondary)
+ if err != nil {
+ return nil, err
+ }
+ rng, err := spanToRange(snapshot, pkg, secondarySpan)
+ if err != nil {
+ return nil, err
+ }
+ diag.Related = append(diag.Related, source.RelatedInformation{
+ URI: secondarySpan.URI(),
+ Range: rng,
+ Message: secondary.Msg,
+ })
+ }
+ return []*source.Diagnostic{diag}, nil
+}
+
+func analysisDiagnosticDiagnostics(ctx context.Context, snapshot *snapshot, pkg *pkg, severity protocol.DiagnosticSeverity, e *analysis.Diagnostic) ([]*source.Diagnostic, error) {
+ spn, err := span.NewRange(snapshot.FileSet(), e.Pos, e.End).Span()
+ if err != nil {
+ return nil, err
+ }
+ rng, err := spanToRange(snapshot, pkg, spn)
+ if err != nil {
+ return nil, err
+ }
+ fixes, err := suggestedAnalysisFixes(snapshot, pkg, e)
+ if err != nil {
+ return nil, err
+ }
+ related, err := relatedInformation(snapshot, pkg, e)
+ if err != nil {
+ return nil, err
+ }
+ return []*source.Diagnostic{{
+ URI: spn.URI(),
+ Range: rng,
+ Severity: protocol.SeverityWarning,
+ Source: source.AnalyzerErrorKind(e.Category),
+ Message: e.Message,
+ Related: related,
+ SuggestedFixes: fixes,
+ }}, nil
}
func typesCodeHref(snapshot *snapshot, code typesinternal.ErrorCode) string {
@@ -217,19 +218,6 @@
return out, nil
}
-func toDiagnosticSource(kind packages.ErrorKind) source.DiagnosticSource {
- switch kind {
- case packages.ListError:
- return source.ListError
- case packages.ParseError:
- return source.ParseError
- case packages.TypeError:
- return source.TypeError
- default:
- return source.UnknownError
- }
-}
-
func typeErrorData(fset *token.FileSet, pkg *pkg, terr types.Error) (typesinternal.ErrorCode, span.Span, error) {
ecode, start, end, ok := typesinternal.ReadGo116ErrorData(terr)
if !ok {
@@ -255,16 +243,6 @@
return span.FileSpan(pgf.Tok, pgf.Mapper.Converter, start, end)
}
-func scannerErrorRange(snapshot *snapshot, pkg *pkg, posn token.Position) (span.Span, error) {
- fset := snapshot.view.session.cache.fset
- pgf, err := pkg.File(span.URIFromPath(posn.Filename))
- if err != nil {
- return span.Span{}, err
- }
- pos := pgf.Tok.Pos(posn.Offset)
- return span.NewRange(fset, pos, pos).Span()
-}
-
// spanToRange converts a span.Span to a protocol.Range,
// assuming that the span belongs to the package whose diagnostics are being computed.
func spanToRange(snapshot *snapshot, pkg *pkg, spn span.Span) (protocol.Range, error) {
@@ -283,13 +261,13 @@
//
// attributes.go:13:1: expected 'package', found 'type'
//
-func parseGoListError(input string) span.Span {
+func parseGoListError(input, wd string) span.Span {
input = strings.TrimSpace(input)
msgIndex := strings.Index(input, ": ")
if msgIndex < 0 {
return span.Parse(input)
}
- return span.Parse(input[:msgIndex])
+ return span.ParseInDir(input[:msgIndex], wd)
}
func parseGoListImportCycleError(ctx context.Context, snapshot *snapshot, e packages.Error, pkg *pkg) (string, span.Span, bool) {
