internal/lsp/source: extract helper, improve error messages
Lack of context in error messages is making my life difficult. Add
context to a few, refactoring out some duplicate code along the way.
Change-Id: I3a940b12ec7c82b1ae1fc477694a2b8b45f6ff71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209860
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go
index 6fdee56..9afdf6d 100644
--- a/internal/lsp/cache/errors.go
+++ b/internal/lsp/cache/errors.go
@@ -3,6 +3,7 @@
import (
"bytes"
"context"
+ "fmt"
"go/scanner"
"go/token"
"go/types"
@@ -89,7 +90,7 @@
}
ph, err := pkg.File(spn.URI())
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("finding file for error %q: %v", msg, err)
}
return &source.Error{
File: ph.File().Identity(),
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index b06c634..52d0bbe 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -6,6 +6,7 @@
import (
"context"
+ "fmt"
"go/ast"
"go/constant"
"go/token"
@@ -394,22 +395,9 @@
startTime := time.Now()
- fh := snapshot.Handle(ctx, f)
- phs, err := snapshot.PackageHandles(ctx, fh)
+ pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle)
if err != nil {
- return nil, nil, err
- }
- ph, err := NarrowestCheckPackageHandle(phs)
- if err != nil {
- return nil, nil, err
- }
- pkg, err := ph.Check(ctx)
- if err != nil {
- return nil, nil, err
- }
- pgh, err := pkg.File(f.URI())
- if err != nil {
- return nil, nil, err
+ return nil, nil, fmt.Errorf("getting file for Completion: %v", err)
}
file, m, _, err := pgh.Cached()
if err != nil {
diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go
index b637e41..7f965be 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -194,7 +194,7 @@
}
}
if ph == nil {
- return nil, errors.Errorf("no ParseGoHandle for %s", c.filename)
+ return nil, errors.Errorf("building import completion for %v: no ParseGoHandle for %s", imp.importPath, c.filename)
}
return computeOneImportFixEdits(c.ctx, c.snapshot.View(), ph, &imports.ImportFix{
diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go
index 797b093..0e49f9a 100644
--- a/internal/lsp/source/format.go
+++ b/internal/lsp/source/format.go
@@ -8,6 +8,7 @@
import (
"bytes"
"context"
+ "fmt"
"go/ast"
"go/format"
"go/parser"
@@ -27,23 +28,11 @@
ctx, done := trace.StartSpan(ctx, "source.Format")
defer done()
- fh := snapshot.Handle(ctx, f)
- phs, err := snapshot.PackageHandles(ctx, fh)
+ pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle)
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("getting file for Format: %v", err)
}
- ph, err := NarrowestCheckPackageHandle(phs)
- if err != nil {
- return nil, err
- }
- pkg, err := ph.Check(ctx)
- if err != nil {
- return nil, err
- }
- pgh, err := pkg.File(f.URI())
- if err != nil {
- return nil, err
- }
+
// Be extra careful that the file's ParseMode is correct,
// otherwise we might replace the user's code with a trimmed AST.
if pgh.Mode() != ParseFull {
@@ -102,26 +91,13 @@
ctx, done := trace.StartSpan(ctx, "source.AllImportsFixes")
defer done()
- fh := snapshot.Handle(ctx, f)
- phs, err := snapshot.PackageHandles(ctx, fh)
+ pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle)
if err != nil {
- return nil, nil, err
- }
- ph, err := NarrowestCheckPackageHandle(phs)
- if err != nil {
- return nil, nil, err
- }
- pkg, err := ph.Check(ctx)
- if err != nil {
- return nil, nil, err
+ return nil, nil, fmt.Errorf("getting file for AllImportsFixes: %v", err)
}
if hasListErrors(pkg) {
return nil, nil, errors.Errorf("%s has list errors, not running goimports", f.URI())
}
- pgh, err := pkg.File(f.URI())
- if err != nil {
- return nil, nil, err
- }
options := &imports.Options{
// Defaults.
AllErrors: true,
diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go
index 849bc80..d8a72bd 100644
--- a/internal/lsp/source/highlight.go
+++ b/internal/lsp/source/highlight.go
@@ -6,6 +6,7 @@
import (
"context"
+ "fmt"
"go/ast"
"go/token"
@@ -20,22 +21,9 @@
ctx, done := trace.StartSpan(ctx, "source.Highlight")
defer done()
- fh := snapshot.Handle(ctx, f)
- phs, err := snapshot.PackageHandles(ctx, fh)
+ pkg, pgh, err := getParsedFile(ctx, snapshot, f, WidestCheckPackageHandle)
if err != nil {
- return nil, err
- }
- ph, err := WidestCheckPackageHandle(phs)
- if err != nil {
- return nil, err
- }
- pkg, err := ph.Check(ctx)
- if err != nil {
- return nil, err
- }
- pgh, err := pkg.File(f.URI())
- if err != nil {
- return nil, err
+ return nil, fmt.Errorf("getting file for Highlight: %v", err)
}
file, m, _, err := pgh.Parse(ctx)
if err != nil {
diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go
index dc91d05..a0720c0 100644
--- a/internal/lsp/source/identifier.go
+++ b/internal/lsp/source/identifier.go
@@ -6,6 +6,7 @@
import (
"context"
+ "fmt"
"go/ast"
"go/token"
"go/types"
@@ -50,22 +51,9 @@
ctx, done := trace.StartSpan(ctx, "source.Identifier")
defer done()
- fh := snapshot.Handle(ctx, f)
- phs, err := snapshot.PackageHandles(ctx, fh)
+ pkg, pgh, err := getParsedFile(ctx, snapshot, f, WidestCheckPackageHandle)
if err != nil {
- return nil, err
- }
- ph, err := WidestCheckPackageHandle(phs)
- if err != nil {
- return nil, err
- }
- pkg, err := ph.Check(ctx)
- if err != nil {
- return nil, err
- }
- pgh, err := pkg.File(f.URI())
- if err != nil {
- return nil, err
+ return nil, fmt.Errorf("getting file for Identifier: %v", err)
}
file, m, _, err := pgh.Cached()
if err != nil {
diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go
index 1df62d0..29a7d8c 100644
--- a/internal/lsp/source/rename.go
+++ b/internal/lsp/source/rename.go
@@ -7,6 +7,7 @@
import (
"bytes"
"context"
+ "fmt"
"go/ast"
"go/format"
"go/token"
@@ -182,7 +183,7 @@
func (i *IdentifierInfo) getPkgName(ctx context.Context) (*IdentifierInfo, error) {
ph, err := i.pkg.File(i.URI())
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("finding file for identifier %v: %v", i.Name, err)
}
file, _, _, err := ph.Cached()
if err != nil {
diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go
index 471d1e5..c819956 100644
--- a/internal/lsp/source/signature_help.go
+++ b/internal/lsp/source/signature_help.go
@@ -6,6 +6,7 @@
import (
"context"
+ "fmt"
"go/ast"
"go/doc"
"go/token"
@@ -31,22 +32,9 @@
ctx, done := trace.StartSpan(ctx, "source.SignatureHelp")
defer done()
- fh := snapshot.Handle(ctx, f)
- phs, err := snapshot.PackageHandles(ctx, fh)
+ pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle)
if err != nil {
- return nil, err
- }
- ph, err := NarrowestCheckPackageHandle(phs)
- if err != nil {
- return nil, err
- }
- pkg, err := ph.Check(ctx)
- if err != nil {
- return nil, err
- }
- pgh, err := pkg.File(f.URI())
- if err != nil {
- return nil, err
+ return nil, fmt.Errorf("getting file for SignatureHelp: %v", err)
}
file, m, _, err := pgh.Cached()
if err != nil {
diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go
index 0124b6f..8acbdd4 100644
--- a/internal/lsp/source/symbols.go
+++ b/internal/lsp/source/symbols.go
@@ -18,22 +18,9 @@
ctx, done := trace.StartSpan(ctx, "source.DocumentSymbols")
defer done()
- fh := snapshot.Handle(ctx, f)
- phs, err := snapshot.PackageHandles(ctx, fh)
+ pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle)
if err != nil {
- return nil, err
- }
- ph, err := NarrowestCheckPackageHandle(phs)
- if err != nil {
- return nil, err
- }
- pkg, err := ph.Check(ctx)
- if err != nil {
- return nil, err
- }
- pgh, err := pkg.File(f.URI())
- if err != nil {
- return nil, err
+ return nil, fmt.Errorf("getting file for DocumentSymbols: %v", err)
}
file, m, _, err := pgh.Cached()
if err != nil {
diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go
index 16c1960..191e529 100644
--- a/internal/lsp/source/util.go
+++ b/internal/lsp/source/util.go
@@ -65,6 +65,26 @@
return s.m.URI
}
+// getParsedFile is a convenience function that extracts the Package and ParseGoHandle for a File in a Snapshot.
+// selectPackage is typically Narrowest/WidestCheckPackageHandle below.
+func getParsedFile(ctx context.Context, snapshot Snapshot, f File, selectPackage func([]PackageHandle) (PackageHandle, error)) (Package, ParseGoHandle, error) {
+ fh := snapshot.Handle(ctx, f)
+ phs, err := snapshot.PackageHandles(ctx, fh)
+ if err != nil {
+ return nil, nil, err
+ }
+ ph, err := selectPackage(phs)
+ if err != nil {
+ return nil, nil, err
+ }
+ pkg, err := ph.Check(ctx)
+ if err != nil {
+ return nil, nil, err
+ }
+ pgh, err := pkg.File(f.URI())
+ return pkg, pgh, err
+}
+
// NarrowestCheckPackageHandle picks the "narrowest" package for a given file.
//
// By "narrowest" package, we mean the package with the fewest number of files
