internal/lsp: fix flaking internal/lsp/cmd tests
Reloading metadata on demand fails for some our test packages,
because I don't understand how to construct arguments to commands.
Change-Id: Ib18454a09772e854a612528af898d06ce14133c2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214717
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index b759545..00042dd2 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -298,7 +298,7 @@
if pkg.pkgPath == "unsafe" {
pkg.types = types.Unsafe
} else if len(files) == 0 { // not the unsafe package, no parsed files
- return nil, errors.Errorf("no parsed files for package %s, expected: %s, errors: %v", pkg.pkgPath, pkg.compiledGoFiles, actualErrors)
+ return nil, errors.Errorf("no parsed files for package %s, expected: %s, errors: %v, list errors: %v", pkg.pkgPath, pkg.compiledGoFiles, actualErrors, rawErrors)
} else {
pkg.types = types.NewPackage(string(m.pkgPath), m.name)
}
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index e824b06..ee0681b 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -36,35 +36,33 @@
}
func (s *snapshot) load(ctx context.Context, scope interface{}) ([]*metadata, error) {
- var query string
+ var query []string
switch scope := scope.(type) {
case []packagePath:
- for i, p := range scope {
- if i != 0 {
- query += " "
- }
- query += string(p)
+ for _, p := range scope {
+ query = append(query, string(p))
}
case packagePath:
- query = string(scope)
+ query = append(query, string(scope))
case fileURI:
- query = fmt.Sprintf("file=%s", span.URI(scope).Filename())
+ query = append(query, fmt.Sprintf("file=%s", span.URI(scope).Filename()))
case directoryURI:
filename := span.URI(scope).Filename()
- query = fmt.Sprintf("%s/...", filename)
+ q := fmt.Sprintf("%s/...", filename)
// Simplify the query if it will be run in the requested directory.
// This ensures compatibility with Go 1.12 that doesn't allow
// <directory>/... in GOPATH mode.
if s.view.folder.Filename() == filename {
- query = "./..."
+ q = "./..."
}
+ query = append(query, q)
}
ctx, done := trace.StartSpan(ctx, "cache.view.load", telemetry.Query.Of(query))
defer done()
cfg := s.view.Config(ctx)
- pkgs, err := packages.Load(cfg, query)
+ pkgs, err := packages.Load(cfg, query...)
// If the context was canceled, return early. Otherwise, we might be
// type-checking an incomplete result. Check the context directly,
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 7ed158c..0bd0fee 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -16,7 +16,6 @@
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/lsp/telemetry"
"golang.org/x/tools/internal/span"
- "golang.org/x/tools/internal/telemetry/log"
errors "golang.org/x/xerrors"
)
@@ -382,8 +381,7 @@
// so just build the package handle directly (without a reload).
ph, err := s.buildPackageHandle(ctx, pkgID, source.ParseExported)
if err != nil {
- log.Error(ctx, "KnownPackages: failed to create PackageHandle", err, telemetry.Package.Of(pkgID))
- continue
+ return nil, err
}
results = append(results, ph)
}
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 0f27b43..89bf8d8 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -56,8 +56,9 @@
// not correctly configured. Report errors, if possible.
var warningMsg string
if len(ph.MissingDependencies()) > 0 {
- if warningMsg, err = checkCommonErrors(ctx, snapshot.View()); err != nil {
- log.Error(ctx, "error checking common errors", err, telemetry.File.Of(fh.Identity().URI))
+ warningMsg, err = checkCommonErrors(ctx, snapshot.View())
+ if err != nil {
+ return nil, "", err
}
}
reports, msg, err := PackageDiagnostics(ctx, snapshot, ph, withAnalysis, disabledAnalyses)
@@ -77,9 +78,9 @@
// we may have an ad-hoc package with multiple files. Show a warning message.
// TODO(golang/go#36416): Remove this when golang.org/cl/202277 is merged.
if len(pkg.CompiledGoFiles()) == 1 && hasUndeclaredErrors(pkg) {
- fh := pkg.CompiledGoFiles()[0].File()
- if warningMsg, err = checkCommonErrors(ctx, snapshot.View()); err != nil {
- log.Error(ctx, "error checking common errors", err, telemetry.File.Of(fh.Identity().URI))
+ warningMsg, err = checkCommonErrors(ctx, snapshot.View())
+ if err != nil {
+ return nil, "", err
}
}
// Prepare the reports we will send for the files in this package.
diff --git a/internal/lsp/source/errors.go b/internal/lsp/source/errors.go
index 9bb9436..5d2d534 100644
--- a/internal/lsp/source/errors.go
+++ b/internal/lsp/source/errors.go
@@ -8,6 +8,8 @@
"os/exec"
"path/filepath"
"strings"
+
+ errors "golang.org/x/xerrors"
)
const (
@@ -77,8 +79,8 @@
return msg, nil
}
-// invokeGo returns the stdout of a go command invocation.
-// Borrowed from golang.org/x/tools/go/packages/golist.go.
+// InvokeGo returns the output of a go command invocation.
+// It does not try to recover from errors.
func InvokeGo(ctx context.Context, dir string, env []string, args ...string) (*bytes.Buffer, error) {
stdout := new(bytes.Buffer)
stderr := new(bytes.Buffer)
@@ -99,12 +101,10 @@
if ee, ok := err.(*exec.Error); ok && ee.Err == exec.ErrNotFound {
return nil, fmt.Errorf("'gopls requires 'go', but %s", exec.ErrNotFound)
}
- if _, ok := err.(*exec.ExitError); !ok {
- // Catastrophic error:
- // - context cancellation
- return nil, fmt.Errorf("couldn't exec 'go %v': %s %T", args, err, err)
+ if ctx.Err() != nil {
+ return nil, ctx.Err()
}
- return stdout, fmt.Errorf("%s", stderr)
+ return stdout, errors.Errorf("err: %v: stderr: %s", err, stderr)
}
return stdout, nil
}
diff --git a/internal/lsp/source/implementation.go b/internal/lsp/source/implementation.go
index b1260b1..fb896f8 100644
--- a/internal/lsp/source/implementation.go
+++ b/internal/lsp/source/implementation.go
@@ -30,17 +30,14 @@
if impl.pkg == nil || len(impl.pkg.CompiledGoFiles()) == 0 {
continue
}
-
rng, err := objToMappedRange(s.View(), impl.pkg, impl.obj)
if err != nil {
return nil, err
}
-
pr, err := rng.Range()
if err != nil {
return nil, err
}
-
locations = append(locations, protocol.Location{
URI: protocol.NewURI(rng.URI()),
Range: pr,
