From bdfa187a520d32efe652bdeac9376d416892b912 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 24 Jan 2020 01:36:46 -0500 Subject: [PATCH] internal/lsp: remove the checkErrors command in internal/lsp/source Now that the view can report its own validity, we no longer need to have an extra function to handle that. Change-Id: Icd22f9b08601bd0fe18be064c43d21be2d6782e7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/216144 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/command.go | 2 + internal/lsp/diagnostics.go | 11 +++--- internal/lsp/source/diagnostics.go | 25 ++++--------- internal/lsp/source/errors.go | 59 ------------------------------ 4 files changed, 16 insertions(+), 81 deletions(-) diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 85e237274c..6e94a58a74 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -29,6 +29,8 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom return nil, errors.Errorf("%s is not a mod file", uri) } // Run go.mod tidy on the view. + // TODO: This should go through the ModTidyHandle on the view. + // That will also allow us to move source.InvokeGo into internal/lsp/cache. if _, err := source.InvokeGo(ctx, view.Folder().Filename(), view.Config(ctx).Env, "mod", "tidy"); err != nil { return nil, err } diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index d6d204c57d..6f62167d0b 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -55,12 +55,13 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot) { withAnalyses = true } } - reports, msg, err := source.Diagnostics(ctx, snapshot, ph, withAnalyses) - // Check the warning message before the errors. - if msg != "" { + reports, warn, err := source.Diagnostics(ctx, snapshot, ph, withAnalyses) + // Check if might want to warn the user about their build configuration. + if warn && !snapshot.View().ValidBuildConfiguration() { s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ - Type: protocol.Warning, - Message: msg, + Type: protocol.Warning, + // TODO(rstambler): We should really be able to point to a link on the website. + Message: `You are neither in a module nor in your GOPATH. Please see https://github.com/golang/go/wiki/Modules for information on how to set up your Go project.`, }) } if ctx.Err() != nil { diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index b85bedd71d..bafa6e38cc 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -39,19 +39,16 @@ type RelatedInformation struct { Message string } -func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, withAnalysis bool) (map[FileIdentity][]Diagnostic, string, error) { +func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, withAnalysis bool) (map[FileIdentity][]Diagnostic, bool, error) { // If we are missing dependencies, it may because the user's workspace is // not correctly configured. Report errors, if possible. - var ( - warn bool - warningMsg string - ) + var warn bool if len(ph.MissingDependencies()) > 0 { warn = true } pkg, err := ph.Check(ctx) if err != nil { - return nil, "", err + return nil, false, err } // If we have a package with a single file and errors about "undeclared" symbols, // we may have an ad-hoc package with multiple files. Show a warning message. @@ -59,17 +56,11 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, withA if len(pkg.CompiledGoFiles()) == 1 && hasUndeclaredErrors(pkg) { warn = true } - if warn { - warningMsg, err = checkCommonErrors(ctx, snapshot.View()) - if err != nil { - return nil, "", err - } - } // Prepare the reports we will send for the files in this package. reports := make(map[FileIdentity][]Diagnostic) for _, fh := range pkg.CompiledGoFiles() { if err := clearReports(snapshot, reports, fh.File().Identity().URI); err != nil { - return nil, warningMsg, err + return nil, warn, err } } // Prepare any additional reports for the errors in this package. @@ -87,25 +78,25 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, withA } } if err := clearReports(snapshot, reports, e.URI); err != nil { - return nil, warningMsg, err + return nil, warn, err } } // Run diagnostics for the package that this URI belongs to. hadDiagnostics, err := diagnostics(ctx, snapshot, reports, pkg) if err != nil { - return nil, warningMsg, err + return nil, warn, err } if !hadDiagnostics && withAnalysis { // If we don't have any list, parse, or type errors, run analyses. if err := analyses(ctx, snapshot, reports, ph, snapshot.View().Options().DisabledAnalyses); err != nil { // Exit early if the context has been canceled. if ctx.Err() != nil { - return nil, warningMsg, ctx.Err() + return nil, warn, ctx.Err() } log.Error(ctx, "failed to run analyses", err, telemetry.Package.Of(ph.ID())) } } - return reports, warningMsg, nil + return reports, warn, nil } func FileDiagnostics(ctx context.Context, snapshot Snapshot, uri span.URI) (FileIdentity, []Diagnostic, error) { diff --git a/internal/lsp/source/errors.go b/internal/lsp/source/errors.go index af6c64ad93..5858603578 100644 --- a/internal/lsp/source/errors.go +++ b/internal/lsp/source/errors.go @@ -4,70 +4,11 @@ import ( "bytes" "context" "fmt" - "os" "os/exec" - "path/filepath" - "strings" errors "golang.org/x/xerrors" ) -const ( - // TODO(rstambler): We should really be able to point to a link on the website. - modulesWiki = "https://github.com/golang/go/wiki/Modules" -) - -func checkCommonErrors(ctx context.Context, v View) (string, error) { - // Unfortunately, we probably can't have go/packages expose a function like this. - // Since we only really understand the `go` command, check the user's GOPACKAGESDRIVER - // and, if they are using `go list`, consider the possible error cases. - gopackagesdriver := os.Getenv("GOPACKAGESDRIVER") - if gopackagesdriver != "" && gopackagesdriver != "off" { - return "", nil - } - - // 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") - folder := v.Folder().Filename() - - modfile, _ := v.ModFiles() - modRoot := filepath.Dir(modfile.Filename()) - - // Not inside of a module. - inAModule := modfile != "" - - // The user may have a multiple directories in their GOPATH. - var inGopath bool - for _, gp := range filepath.SplitList(gopath) { - if strings.HasPrefix(folder, filepath.Join(gp, "src")) { - inGopath = true - break - } - } - - moduleMode := os.Getenv("GO111MODULE") - - var msg string - // The user is in a module. - if inAModule { - rel, err := filepath.Rel(modRoot, folder) - if err != nil || strings.HasPrefix(rel, "..") { - msg = fmt.Sprintf("Your workspace root is %s, but your module root is %s. Please add %s or a subdirectory as a workspace folder.", folder, 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 = fmt.Sprintf("You are neither in a module nor in your GOPATH. Please see %s for information on how to set up your Go project.", modulesWiki) - } - return msg, nil -} - // 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) {