From c4cbd3b08ff4b6a8938280e0c6ce8ce05cef1d7f Mon Sep 17 00:00:00 2001 From: Rohan Challa Date: Fri, 24 Jan 2020 10:14:25 -0500 Subject: [PATCH] internal/lsp: add diagnostics in .go files for missing deps in go.mod This change adds diagnostics to imports in .go files that are missing the dependency in the go.mod file. The quick fix that adds the appropriate require statement is coming in a follow up CL. Updates golang/go#31999 Change-Id: I314cdbe8e3dd27da744ca0391e7a6e5dc1ebaa77 Reviewed-on: https://go-review.googlesource.com/c/tools/+/216277 Run-TryBot: Rohan Challa TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/diagnostics.go | 37 +++++++----- internal/lsp/lsp_test.go | 2 +- internal/lsp/mod/diagnostics.go | 13 +++-- internal/lsp/mod/mod_test.go | 2 +- internal/lsp/source/diagnostics.go | 93 +++++++++++++++++++++++++++++- 5 files changed, 123 insertions(+), 24 deletions(-) diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 2cfa8ea527..e3323f7b8f 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -36,6 +36,28 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot) { ctx, done := trace.StartSpan(ctx, "lsp:background-worker") defer done() + // Diagnose the go.mod file. + reports, missingModules, err := mod.Diagnostics(ctx, snapshot) + if ctx.Err() != nil { + return + } + if err != nil { + log.Error(ctx, "diagnose: could not generate diagnostics for go.mod file", err) + return + } + // Ensure that the reports returned from mod.Diagnostics are only related to the + // go.mod file for the module. + if len(reports) > 1 { + panic("unexpected reports from mod.Diagnostics") + } + modURI, _ := snapshot.View().ModFiles() + for fi := range reports { + if fi.URI != modURI { + panic("unexpected reports from mod.Diagnostics") + } + } + s.publishReports(ctx, snapshot, reports, false) + // Diagnose all of the packages in the workspace. go func() { wsPackages, err := snapshot.WorkspacePackages(ctx) @@ -55,7 +77,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot) { withAnalyses = true } } - reports, warn, err := source.Diagnostics(ctx, snapshot, ph, withAnalyses) + reports, warn, err := source.Diagnostics(ctx, snapshot, ph, missingModules, withAnalyses) // Check if might want to warn the user about their build configuration. if warn && !snapshot.View().ValidBuildConfiguration() { s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ @@ -75,19 +97,6 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot) { }(ph) } }() - - // Diagnose the go.mod file. - go func() { - reports, err := mod.Diagnostics(ctx, snapshot) - if ctx.Err() != nil { - return - } - if err != nil { - log.Error(ctx, "diagnose: could not generate diagnostics for go.mod file", err) - return - } - s.publishReports(ctx, snapshot, reports, false) - }() } func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, reports map[source.FileIdentity][]source.Diagnostic, withAnalysis bool) { diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 2d5ea2ef78..91e6086ab2 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -972,7 +972,7 @@ func TestModfileSuggestedFixes(t *testing.T) { t.Fatal(err) } - reports, err := mod.Diagnostics(ctx, snapshot) + reports, _, err := mod.Diagnostics(ctx, snapshot) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index 960f493c93..5c39efacc1 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/internal/lsp/mod/diagnostics.go @@ -9,6 +9,7 @@ package mod import ( "context" + "golang.org/x/mod/modfile" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/telemetry" @@ -16,13 +17,13 @@ import ( "golang.org/x/tools/internal/telemetry/trace" ) -func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.FileIdentity][]source.Diagnostic, error) { +func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.FileIdentity][]source.Diagnostic, map[string]*modfile.Require, error) { // TODO: We will want to support diagnostics for go.mod files even when the -modfile flag is turned off. realURI, tempURI := snapshot.View().ModFiles() // Check the case when the tempModfile flag is turned off. if realURI == "" || tempURI == "" { - return nil, nil + return nil, nil, nil } ctx, done := trace.StartSpan(ctx, "mod.Diagnostics", telemetry.File.Of(realURI)) @@ -30,11 +31,11 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.File realfh, err := snapshot.GetFile(realURI) if err != nil { - return nil, err + return nil, nil, err } - _, _, _, parseErrors, err := snapshot.ModTidyHandle(ctx, realfh).Tidy(ctx) + _, _, missingDeps, parseErrors, err := snapshot.ModTidyHandle(ctx, realfh).Tidy(ctx) if err != nil { - return nil, err + return nil, nil, err } reports := map[source.FileIdentity][]source.Diagnostic{ @@ -54,7 +55,7 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.File } reports[realfh.Identity()] = append(reports[realfh.Identity()], diag) } - return reports, nil + return reports, missingDeps, nil } func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, realfh source.FileHandle, diags []protocol.Diagnostic) []protocol.CodeAction { diff --git a/internal/lsp/mod/mod_test.go b/internal/lsp/mod/mod_test.go index 7a149c19f3..c9c7d24e52 100644 --- a/internal/lsp/mod/mod_test.go +++ b/internal/lsp/mod/mod_test.go @@ -150,7 +150,7 @@ func TestDiagnostics(t *testing.T) { if !hasTempModfile(ctx, snapshot) { return } - reports, err := Diagnostics(ctx, snapshot) + reports, _, err := Diagnostics(ctx, snapshot) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 6fcf8640dc..fefbb9aa8a 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -6,8 +6,12 @@ package source import ( "context" + "fmt" + "go/ast" + "strconv" "strings" + "golang.org/x/mod/modfile" "golang.org/x/tools/go/analysis" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/telemetry" @@ -39,7 +43,7 @@ type RelatedInformation struct { Message string } -func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, withAnalysis bool) (map[FileIdentity][]Diagnostic, bool, error) { +func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missingModules map[string]*modfile.Require, 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 @@ -56,12 +60,41 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, withA if len(pkg.CompiledGoFiles()) == 1 && hasUndeclaredErrors(pkg) { warn = true } + + isMissingModule := false + for _, imp := range pkg.Imports() { + if _, ok := missingModules[imp.PkgPath()]; ok { + isMissingModule = true + continue + } + for dep, req := range missingModules { + // If the import is a package of the dependency, then add the package to the map, this will + // eliminate the need to do this prefix package search on each import for each file. + // Example: + // import ( + // "golang.org/x/tools/go/expect" + // "golang.org/x/tools/go/packages" + // ) + // They both are related to the same module: "golang.org/x/tools" + if req != nil && strings.HasPrefix(imp.PkgPath(), dep) { + missingModules[imp.PkgPath()] = req + isMissingModule = true + break + } + } + } + // 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, warn, err } + if isMissingModule { + if err := missingModulesDiagnostics(ctx, snapshot, reports, missingModules, fh.File().Identity().URI); err != nil { + return nil, warn, err + } + } } // Prepare any additional reports for the errors in this package. for _, e := range pkg.GetErrors() { @@ -116,7 +149,7 @@ func FileDiagnostics(ctx context.Context, snapshot Snapshot, uri span.URI) (File if err != nil { return FileIdentity{}, nil, err } - reports, _, err := Diagnostics(ctx, snapshot, ph, true) + reports, _, err := Diagnostics(ctx, snapshot, ph, nil, true) if err != nil { return FileIdentity{}, nil, err } @@ -182,6 +215,62 @@ func diagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentit return nonEmptyDiagnostics, nil } +func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, missingModules map[string]*modfile.Require, uri span.URI) error { + if snapshot.View().Ignore(uri) || len(missingModules) == 0 { + return nil + } + fh, err := snapshot.GetFile(uri) + if err != nil { + return err + } + file, m, _, err := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseHeader).Parse(ctx) + if err != nil { + log.Error(ctx, "could not parse go file when checking for missing modules", err) + return err + } + // Make a dependency->import map to improve performance when finding missing dependencies. + imports := make(map[string]*ast.ImportSpec) + for _, imp := range file.Imports { + if imp.Path == nil { + continue + } + if target, err := strconv.Unquote(imp.Path.Value); err == nil { + imports[target] = imp + } + } + // If the go file has 0 imports, then we do not need to check for missing dependencies. + if len(imports) == 0 { + return nil + } + if reports[fh.Identity()] == nil { + reports[fh.Identity()] = []Diagnostic{} + } + for mod, req := range missingModules { + if req.Syntax == nil { + continue + } + imp, ok := imports[mod] + if !ok { + continue + } + spn, err := span.NewRange(snapshot.View().Session().Cache().FileSet(), imp.Path.Pos(), imp.Path.End()).Span() + if err != nil { + return err + } + rng, err := m.Range(spn) + if err != nil { + return err + } + reports[fh.Identity()] = append(reports[fh.Identity()], Diagnostic{ + Message: fmt.Sprintf("%s is not in your go.mod file.", req.Mod.Path), + Range: rng, + Source: "go mod tidy", + Severity: protocol.SeverityWarning, + }) + } + return nil +} + func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, ph PackageHandle, disabledAnalyses map[string]struct{}) error { var analyzers []*analysis.Analyzer for _, a := range snapshot.View().Options().Analyzers {