Skip to content

Commit

Permalink
internal/lsp: show errors when the user is in the wrong directory
Browse files Browse the repository at this point in the history
If we encounter `go list` errors when loading a user's package, we
should try to see if they've encountered any of our common error cases.
They are: 1) a user has GO111MODULE=off, but is outside of their GOPATH,
and 2) a user is in module mode but doesn't have a go.mod file.

Fortunately, go/packages does a great job handling edge cases so gopls
will work well for most of them. The main issue will be unresolved
imports. These show up in DepErrors in `go list`, so go/packages doesn't
propagate them through to the list of errors. This will require changes
to go/packages.

Updates golang/go#31668

Change-Id: Ibd5253b33b38caffeaad54a403c74c0b861fcc14
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194018
Run-TryBot: Rebecca Stambler <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Cottrell <[email protected]>
  • Loading branch information
stamblerre authored and clintjedwards committed Sep 19, 2019
1 parent 7956c30 commit 6aeba54
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 19 deletions.
7 changes: 2 additions & 5 deletions internal/lsp/cmd/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ func (d *definition) Run(ctx context.Context, args ...string) error {
Position: loc.Range.Start,
}
p := protocol.DefinitionParams{
tdpp,
protocol.WorkDoneProgressParams{},
protocol.PartialResultParams{},
TextDocumentPositionParams: tdpp,
}
locs, err := conn.Definition(ctx, &p)
if err != nil {
Expand All @@ -92,8 +90,7 @@ func (d *definition) Run(ctx context.Context, args ...string) error {
return errors.Errorf("%v: not an identifier", from)
}
q := protocol.HoverParams{
tdpp,
protocol.WorkDoneProgressParams{},
TextDocumentPositionParams: tdpp,
}
hover, err := conn.Hover(ctx, &q)
if err != nil {
Expand Down
8 changes: 7 additions & 1 deletion internal/lsp/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,16 @@ func (s *Server) diagnostics(view source.View, uri span.URI) error {
if !ok {
return errors.Errorf("%s is not a Go file", f.URI())
}
reports, err := source.Diagnostics(ctx, view, gof, view.Options().DisabledAnalyses)
reports, warningMsg, err := source.Diagnostics(ctx, view, gof, view.Options().DisabledAnalyses)
if err != nil {
return err
}
if warningMsg != "" {
s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
Type: protocol.Info,
Message: warningMsg,
})
}

s.undeliveredMu.Lock()
defer s.undeliveredMu.Unlock()
Expand Down
7 changes: 3 additions & 4 deletions internal/lsp/general.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,9 @@ func (s *Server) initialize(ctx context.Context, params *protocol.ParamInitia) (
Name: path.Base(params.RootURI),
}}
} else {
// no folders and no root, single file mode
//TODO(iancottrell): not sure how to do single file mode yet
//issue: golang.org/issue/31168
return nil, errors.Errorf("single file mode not supported yet")
// No folders and no root--we are in single file mode.
// TODO: https://golang.org/issue/34160.
return nil, errors.Errorf("gopls does not yet support editing a single file. Please open a directory.")
}
}

Expand Down
4 changes: 2 additions & 2 deletions internal/lsp/lsp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) {
if !ok {
t.Fatalf("%s is not a Go file: %v", uri, err)
}
results, err := source.Diagnostics(r.ctx, v, gof, nil)
results, _, err := source.Diagnostics(r.ctx, v, gof, nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -558,7 +558,7 @@ func (r *runner) SuggestedFix(t *testing.T, data tests.SuggestedFixes) {
if err != nil {
t.Fatal(err)
}
results, err := source.Diagnostics(r.ctx, v, f, nil)
results, _, err := source.Diagnostics(r.ctx, v, f, nil)
if err != nil {
t.Fatal(err)
}
Expand Down
22 changes: 16 additions & 6 deletions internal/lsp/source/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,26 +64,36 @@ const (
SeverityError
)

func Diagnostics(ctx context.Context, view View, f GoFile, disabledAnalyses map[string]struct{}) (map[span.URI][]Diagnostic, error) {
func Diagnostics(ctx context.Context, view View, f GoFile, disabledAnalyses map[string]struct{}) (map[span.URI][]Diagnostic, string, error) {
ctx, done := trace.StartSpan(ctx, "source.Diagnostics", telemetry.File.Of(f.URI()))
defer done()

cphs, err := f.CheckPackageHandles(ctx)
if err != nil {
return nil, err
return nil, "", err
}
cph := NarrowestCheckPackageHandle(cphs)
pkg, err := cph.Check(ctx)
if err != nil {
log.Error(ctx, "no package for file", err)
return singleDiagnostic(f.URI(), "%s is not part of a package", f.URI()), nil
return singleDiagnostic(f.URI(), "%s is not part of a package", f.URI()), "", nil
}

// Prepare the reports we will send for the files in this package.
reports := make(map[span.URI][]Diagnostic)
for _, fh := range pkg.Files() {
clearReports(view, reports, fh.File().Identity().URI)
}

// If we have `go list` errors, we may want to offer a warning message to the user.
var warningMsg string
if hasListErrors(pkg.GetErrors()) {
warningMsg, err = checkCommonErrors(ctx, view, f.URI())
if err != nil {
log.Error(ctx, "error checking common errors", err, telemetry.File.Of(f.URI))
}
}

// Prepare any additional reports for the errors in this package.
for _, err := range pkg.GetErrors() {
if err.Kind != packages.ListError {
Expand All @@ -104,19 +114,19 @@ func Diagnostics(ctx context.Context, view View, f GoFile, disabledAnalyses map[
for _, f := range revDeps {
cphs, err := f.CheckPackageHandles(ctx)
if err != nil {
return nil, err
return nil, "", err
}
cph := WidestCheckPackageHandle(cphs)
pkg, err := cph.Check(ctx)
if err != nil {
return nil, err
return nil, warningMsg, err
}
for _, fh := range pkg.Files() {
clearReports(view, reports, fh.File().Identity().URI)
}
diagnostics(ctx, view, pkg, reports)
}
return reports, nil
return reports, warningMsg, nil
}

type diagnosticSet struct {
Expand Down
86 changes: 86 additions & 0 deletions internal/lsp/source/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package source

import (
"bytes"
"context"
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"

"golang.org/x/tools/internal/span"
)

func checkCommonErrors(ctx context.Context, view View, uri span.URI) (string, error) {
// Some cases we should be able to detect:
//
// 1. The user is in GOPATH mode and is working outside their GOPATH
// 2. The user is in module mode and has opened a subdirectory of their module
//
gopath := os.Getenv("GOPATH")
cfg := view.Config(ctx)

// Invoke `go env GOMOD` inside of the directory of the file.
fdir := filepath.Dir(uri.Filename())
b, err := invokeGo(ctx, fdir, cfg.Env, "env", "GOMOD")
if err != nil {
return "", err
}
modFile := strings.TrimSpace(b.String())
if modFile == filepath.FromSlash("/dev/null") {
modFile = ""
}

// Not inside of a module.
inAModule := modFile != ""
inGopath := strings.HasPrefix(uri.Filename(), filepath.Join(gopath, "src"))
moduleMode := os.Getenv("GO111MODULE")

var msg string
// The user is in a module.
if inAModule {
// The workspace root is open to a directory different from the module root.
if modRoot := filepath.Dir(modFile); cfg.Dir != filepath.Dir(modFile) {
msg = fmt.Sprintf("Your workspace root is %s, but your module root is %s. Please add %s as a workspace folder.", cfg.Dir, modRoot, modRoot)
}
} else if inGopath {
if moduleMode == "on" {
msg = "You are in module mode, but you are not inside of a module. Please create a module."
}
} else {
msg = "You are neither in a module nor in your GOPATH. Please see X for information on how to set up your Go project."
}
return msg, nil
}

// invokeGo returns the stdout of a go command invocation.
// Borrowed from golang.org/x/tools/go/packages/golist.go.
func invokeGo(ctx context.Context, dir string, env []string, args ...string) (*bytes.Buffer, error) {
stdout := new(bytes.Buffer)
stderr := new(bytes.Buffer)
cmd := exec.CommandContext(ctx, "go", args...)
// On darwin the cwd gets resolved to the real path, which breaks anything that
// expects the working directory to keep the original path, including the
// go command when dealing with modules.
// The Go stdlib has a special feature where if the cwd and the PWD are the
// same node then it trusts the PWD, so by setting it in the env for the child
// process we fix up all the paths returned by the go command.
cmd.Env = append(append([]string{}, env...), "PWD="+dir)
cmd.Dir = dir
cmd.Stdout = stdout
cmd.Stderr = stderr

if err := cmd.Run(); err != nil {
// Check for 'go' executable not being found.
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)
}
}
return stdout, nil
}
2 changes: 2 additions & 0 deletions internal/lsp/source/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"bytes"
"context"
"go/format"
"log"

"golang.org/x/tools/go/packages"
"golang.org/x/tools/internal/imports"
Expand Down Expand Up @@ -268,6 +269,7 @@ func hasParseErrors(pkg Package, uri span.URI) bool {
func hasListErrors(errors []packages.Error) bool {
for _, err := range errors {
if err.Kind == packages.ListError {
log.Printf("LIST ERROR: %v", err)
return true
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/source/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) {
if err != nil {
t.Fatal(err)
}
results, err := source.Diagnostics(r.ctx, r.view, f.(source.GoFile), nil)
results, _, err := source.Diagnostics(r.ctx, r.view, f.(source.GoFile), nil)
if err != nil {
t.Fatal(err)
}
Expand Down

0 comments on commit 6aeba54

Please sign in to comment.