internal/lsp: refactor go.mod diagnostics to simplify the API
Currently, diagnostics for modules that are missing from the go.mod
appear in the Go files in which those modules are imported. As a result,
the diagnostics were previously calculated as part of the Go file
diagnostic calculations. This is convoluted and required passing around
an extra map.
This CL puts that logic in the ModTidyHandle where it belongs.
The diagnostics for the Go files are combined from the multiple sources.
Also, added a skipped test for golang/go#39784, since this CL was
originally intended to be a fix for that issue...
Change-Id: Ic0f9aa235dcd56ea131a2339de9801346f715415
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 2c2e708..5f8d57e 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -200,6 +200,10 @@
return data.pkg, data.err
}
+func (ph *packageHandle) ID() string {
+ return string(ph.m.id)
+}
+
func (ph *packageHandle) CompiledGoFiles() []source.ParseGoHandle {
var files []source.ParseGoHandle
for _, f := range ph.compiledGoFiles {
@@ -208,10 +212,6 @@
return files
}
-func (ph *packageHandle) ID() string {
- return string(ph.m.id)
-}
-
func (ph *packageHandle) MissingDependencies() []string {
var md []string
for i := range ph.m.missingDeps {
@@ -220,28 +220,6 @@
return md
}
-func hashImports(ctx context.Context, wsPackages []source.PackageHandle) (string, error) {
- results := make(map[string]bool)
- var imports []string
- for _, ph := range wsPackages {
- // Check package since we do not always invalidate the metadata.
- pkg, err := ph.Check(ctx)
- if err != nil {
- return "", err
- }
- for _, path := range pkg.Imports() {
- imp := path.PkgPath()
- if _, ok := results[imp]; !ok {
- results[imp] = true
- imports = append(imports, imp)
- }
- }
- }
- sort.Strings(imports)
- hashed := strings.Join(imports, ",")
- return hashContents([]byte(hashed)), nil
-}
-
func (ph *packageHandle) Cached() (source.Package, error) {
return ph.cached()
}
diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go
index 4227982..4e125c4 100644
--- a/internal/lsp/cache/mod_tidy.go
+++ b/internal/lsp/cache/mod_tidy.go
@@ -7,7 +7,12 @@
import (
"context"
"fmt"
+ "go/ast"
+ "go/token"
"io/ioutil"
+ "sort"
+ "strconv"
+ "strings"
"golang.org/x/mod/modfile"
"golang.org/x/tools/internal/event"
@@ -37,10 +42,6 @@
type modTidyData struct {
memoize.NoCopy
- // missingDeps contains dependencies that should be added to the view's
- // go.mod file.
- missingDeps map[string]*modfile.Require
-
// diagnostics are any errors and associated suggested fixes for
// the go.mod file.
diagnostics []source.Error
@@ -48,13 +49,13 @@
err error
}
-func (mth *modTidyHandle) Tidy(ctx context.Context) (map[string]*modfile.Require, []source.Error, error) {
+func (mth *modTidyHandle) Tidy(ctx context.Context) ([]source.Error, error) {
v, err := mth.handle.Get(ctx)
if err != nil {
- return nil, nil, err
+ return nil, err
}
data := v.(*modTidyData)
- return data.missingDeps, data.diagnostics, data.err
+ return data.diagnostics, data.err
}
func (s *snapshot) ModTidyHandle(ctx context.Context) (source.ModTidyHandle, error) {
@@ -72,14 +73,22 @@
if err != nil {
return nil, err
}
- wsPackages, err := s.WorkspacePackages(ctx)
+ wsPhs, err := s.WorkspacePackages(ctx)
if ctx.Err() != nil {
return nil, ctx.Err()
}
if err != nil {
return nil, err
}
- imports, err := hashImports(ctx, wsPackages)
+ var workspacePkgs []source.Package
+ for _, ph := range wsPhs {
+ pkg, err := ph.Check(ctx)
+ if err != nil {
+ return nil, err
+ }
+ workspacePkgs = append(workspacePkgs, pkg)
+ }
+ importHash, err := hashImports(ctx, workspacePkgs)
if err != nil {
return nil, err
}
@@ -93,11 +102,12 @@
modURI = s.view.modURI
cfg = s.config(ctx)
options = s.view.Options()
+ fset = s.view.session.cache.fset
)
key := modTidyKey{
sessionID: s.view.session.id,
view: folder.Filename(),
- imports: imports,
+ imports: importHash,
unsavedOverlays: overlayHash,
gomod: pmh.Mod().Identity().String(),
cfg: hashConfig(cfg),
@@ -150,7 +160,10 @@
missingDeps[req.Mod.Path] = req
}
}
- diagnostics, err := modRequireErrors(pmh.Mod().URI(), m, missingDeps, unusedDeps, options)
+ // First, compute any errors specific to the go.mod file. These include
+ // unused dependencies and modules with incorrect // indirect comments.
+ /// Both the diagnostic and the fix will appear on the go.mod file.
+ modRequireErrs, err := modRequireErrors(pmh.Mod().URI(), m, missingDeps, unusedDeps, options)
if err != nil {
return &modTidyData{err: err}
}
@@ -159,9 +172,15 @@
delete(missingDeps, req.Mod.Path)
}
}
+ // Next, compute any diagnostics for modules that are missing from the
+ // go.mod file. The fixes will be for the go.mod file, but the
+ // diagnostics should appear on the import statements in the Go files.
+ missingModuleErrs, err := missingModuleErrors(ctx, fset, m, workspacePkgs, missingDeps, options)
+ if err != nil {
+ return &modTidyData{err: err}
+ }
return &modTidyData{
- missingDeps: missingDeps,
- diagnostics: diagnostics,
+ diagnostics: append(modRequireErrs, missingModuleErrs...),
}
})
s.mu.Lock()
@@ -173,6 +192,23 @@
return s.modTidyHandle, nil
}
+func hashImports(ctx context.Context, wsPackages []source.Package) (string, error) {
+ results := make(map[string]bool)
+ var imports []string
+ for _, pkg := range wsPackages {
+ for _, path := range pkg.Imports() {
+ imp := path.PkgPath()
+ if _, ok := results[imp]; !ok {
+ results[imp] = true
+ imports = append(imports, imp)
+ }
+ }
+ }
+ sort.Strings(imports)
+ hashed := strings.Join(imports, ",")
+ return hashContents([]byte(hashed)), nil
+}
+
// modRequireErrors extracts the errors that occur on the require directives.
// It checks for directness issues and unused dependencies.
func modRequireErrors(uri span.URI, m *protocol.ColumnMapper, missingDeps, unusedDeps map[string]*modfile.Require, options source.Options) ([]source.Error, error) {
@@ -241,7 +277,7 @@
}
return source.Error{
Category: ModTidyError,
- Message: fmt.Sprintf("%s should be a direct dependency.", req.Mod.Path),
+ Message: fmt.Sprintf("%s should be a direct dependency", req.Mod.Path),
Range: rng,
URI: uri,
SuggestedFixes: []source.SuggestedFix{{
@@ -259,7 +295,7 @@
}
return source.Error{
Category: ModTidyError,
- Message: fmt.Sprintf("%s should be an indirect dependency.", req.Mod.Path),
+ Message: fmt.Sprintf("%s should be an indirect dependency", req.Mod.Path),
Range: rng,
URI: uri,
SuggestedFixes: []source.SuggestedFix{{
@@ -374,3 +410,126 @@
}
return m.Range(span.New(uri, start, end))
}
+
+// missingModuleErrors returns diagnostics for each file in each workspace
+// package that has dependencies that are not reflected in the go.mod file.
+func missingModuleErrors(ctx context.Context, fset *token.FileSet, modMapper *protocol.ColumnMapper, pkgs []source.Package, missingMods map[string]*modfile.Require, options source.Options) ([]source.Error, error) {
+ for _, pkg := range pkgs {
+ missingPkgs := map[string]*modfile.Require{}
+ for _, imp := range pkg.Imports() {
+ if req, ok := missingMods[imp.PkgPath()]; ok {
+ missingPkgs[imp.PkgPath()] = req
+ break
+ }
+ for dep, req := range missingMods {
+ // 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) {
+ missingPkgs[imp.PkgPath()] = req
+ break
+ }
+ }
+ }
+ if len(missingPkgs) > 0 {
+ errors, err := missingModules(ctx, fset, modMapper, pkg, missingPkgs, options)
+ if err != nil {
+ return nil, err
+ }
+ return errors, nil
+ }
+ }
+ return nil, nil
+}
+
+func missingModules(ctx context.Context, fset *token.FileSet, modMapper *protocol.ColumnMapper, pkg source.Package, missing map[string]*modfile.Require, options source.Options) ([]source.Error, error) {
+ var errors []source.Error
+ for _, pgh := range pkg.CompiledGoFiles() {
+ file, _, m, _, err := pgh.Parse(ctx)
+ if err != nil {
+ return nil, err
+ }
+ 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 len(imports) == 0 {
+ continue
+ }
+ for mod, req := range missing {
+ if req.Syntax == nil {
+ continue
+ }
+ imp, ok := imports[mod]
+ if !ok {
+ continue
+ }
+ spn, err := span.NewRange(fset, imp.Path.Pos(), imp.Path.End()).Span()
+ if err != nil {
+ return nil, err
+ }
+ rng, err := m.Range(spn)
+ if err != nil {
+ return nil, err
+ }
+ edits, err := addRequireFix(modMapper, req, options)
+ if err != nil {
+ return nil, err
+ }
+ errors = append(errors, source.Error{
+ URI: pgh.File().URI(),
+ Range: rng,
+ Message: fmt.Sprintf("%s is not in your go.mod file", req.Mod.Path),
+ Category: "go mod tidy",
+ Kind: source.ModTidyError,
+ SuggestedFixes: []source.SuggestedFix{
+ {
+ Title: "Add %s to your go.mod file",
+ Edits: edits,
+ },
+ },
+ })
+ }
+ }
+ return errors, nil
+}
+
+// addRequireFix creates edits for adding a given require to a go.mod file.
+func addRequireFix(m *protocol.ColumnMapper, req *modfile.Require, options source.Options) (map[span.URI][]protocol.TextEdit, error) {
+ // We need a private copy of the parsed go.mod file, since we're going to
+ // modify it.
+ copied, err := modfile.Parse("", m.Content, nil)
+ if err != nil {
+ return nil, err
+ }
+ // Calculate the quick fix edits that need to be made to the go.mod file.
+ if err := copied.AddRequire(req.Mod.Path, req.Mod.Version); err != nil {
+ return nil, err
+ }
+ copied.SortBlocks()
+ newContents, err := copied.Format()
+ if err != nil {
+ return nil, err
+ }
+ // Calculate the edits to be made due to the change.
+ diff := options.ComputeEdits(m.URI, string(m.Content), string(newContents))
+ edits, err := source.ToProtocolEdits(m, diff)
+ if err != nil {
+ return nil, err
+ }
+ return map[span.URI][]protocol.TextEdit{
+ m.URI: edits,
+ }, nil
+}
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index 75aeef7..e433630 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -7,12 +7,13 @@
import (
"context"
"fmt"
- "regexp"
"sort"
"strings"
"golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/imports"
+ "golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/mod"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
@@ -51,7 +52,7 @@
switch fh.Kind() {
case source.Mod:
if diagnostics := params.Context.Diagnostics; len(diagnostics) > 0 {
- modFixes, err := mod.SuggestedFixes(ctx, snapshot, fh, diagnostics)
+ modFixes, err := mod.SuggestedFixes(ctx, snapshot, diagnostics)
if err != nil {
return nil, err
}
@@ -135,9 +136,10 @@
// If there are any diagnostics relating to the go.mod file,
// add their corresponding quick fixes.
- moduleQuickFixes, err := getModuleQuickFixes(ctx, snapshot, diagnostics)
+ moduleQuickFixes, err := mod.SuggestedFixes(ctx, snapshot, diagnostics)
if err != nil {
- return nil, err
+ // Not a fatal error.
+ event.Error(ctx, "module suggested fixes failed", err, tag.Directory.Of(snapshot.View().Folder()))
}
codeActions = append(codeActions, moduleQuickFixes...)
}
@@ -176,47 +178,6 @@
return codeActions, nil
}
-var missingRequirementRe = regexp.MustCompile(`(.+) is not in your go.mod file`)
-
-func getModuleQuickFixes(ctx context.Context, snapshot source.Snapshot, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) {
- // Don't bother getting quick fixes if we have no relevant diagnostics.
- var missingDeps map[string]protocol.Diagnostic
- for _, diagnostic := range diagnostics {
- matches := missingRequirementRe.FindStringSubmatch(diagnostic.Message)
- if len(matches) != 2 {
- continue
- }
- if missingDeps == nil {
- missingDeps = make(map[string]protocol.Diagnostic)
- }
- missingDeps[matches[1]] = diagnostic
- }
- if len(missingDeps) == 0 {
- return nil, nil
- }
- // Get suggested fixes for each missing dependency.
- edits, err := mod.SuggestedGoFixes(ctx, snapshot)
- if err != nil {
- return nil, err
- }
- var codeActions []protocol.CodeAction
- for dep, diagnostic := range missingDeps {
- edit, ok := edits[dep]
- if !ok {
- continue
- }
- codeActions = append(codeActions, protocol.CodeAction{
- Title: fmt.Sprintf("Add %s to go.mod", dep),
- Diagnostics: []protocol.Diagnostic{diagnostic},
- Edit: protocol.WorkspaceEdit{
- DocumentChanges: []protocol.TextDocumentEdit{edit},
- },
- Kind: protocol.QuickFix,
- })
- }
- return codeActions, nil
-}
-
func (s *Server) getSupportedCodeActions() []protocol.CodeActionKind {
allCodeActionKinds := make(map[protocol.CodeActionKind]struct{})
for _, kinds := range s.session.Options().SupportedCodeActions {
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index a2e653f..b1f9085 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -60,29 +60,22 @@
var wg sync.WaitGroup
// Diagnose the go.mod file.
- reports, missingModules, err := mod.Diagnostics(ctx, snapshot)
+ reports, err := mod.Diagnostics(ctx, snapshot)
if err != nil {
event.Error(ctx, "warning: diagnose go.mod", err, tag.Directory.Of(snapshot.View().Folder().Filename()))
}
if ctx.Err() != nil {
return nil, nil
}
- // Ensure that the reports returned from mod.Diagnostics are only related
- // to the go.mod file for the module.
- if len(reports) > 1 {
- panic(fmt.Sprintf("expected 1 report from mod.Diagnostics, got %v: %v", len(reports), reports))
- }
modURI := snapshot.View().ModFile()
for id, diags := range reports {
if id.URI == "" {
event.Error(ctx, "missing URI for module diagnostics", fmt.Errorf("empty URI"), tag.Directory.Of(snapshot.View().Folder().Filename()))
continue
}
- if id.URI != modURI {
- panic(fmt.Sprintf("expected module diagnostics report for %q, got %q", modURI, id.URI))
- }
key := diagnosticKey{
- id: id,
+ id: id,
+ withAnalysis: true, // treat go.mod diagnostics like analyses
}
allReports[key] = diags
}
@@ -124,7 +117,7 @@
withAnalyses = true
}
}
- reports, warn, err := source.Diagnostics(ctx, snapshot, ph, missingModules, withAnalyses)
+ reports, warn, err := source.Diagnostics(ctx, snapshot, ph, withAnalyses)
// Check if might want to warn the user about their build configuration.
// Our caller decides whether to send the message.
if warn && !snapshot.View().ValidBuildConfiguration() {
@@ -143,7 +136,7 @@
id: id,
withAnalysis: withAnalyses,
}
- allReports[key] = diags
+ allReports[key] = append(allReports[key], diags...)
}
reportsMu.Unlock()
}(ph)
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 02252bf..620c936 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -412,7 +412,7 @@
},
})
if err != nil {
- t.Fatal(err)
+ t.Fatalf("CodeAction %s failed: %v", spn, err)
}
// Hack: We assume that we only get one code action per range.
// TODO(rstambler): Support multiple code actions per test.
@@ -646,7 +646,6 @@
}
}
})
-
}
}
diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go
index 184b216..cd36883 100644
--- a/internal/lsp/mod/diagnostics.go
+++ b/internal/lsp/mod/diagnostics.go
@@ -9,17 +9,16 @@
import (
"context"
- "golang.org/x/mod/modfile"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
)
-func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.FileIdentity][]*source.Diagnostic, map[string]*modfile.Require, error) {
+func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.FileIdentity][]*source.Diagnostic, error) {
uri := snapshot.View().ModFile()
if uri == "" {
- return nil, nil, nil
+ return nil, nil
}
ctx, done := event.Start(ctx, "mod.Diagnostics", tag.URI.Of(uri))
@@ -27,18 +26,18 @@
fh, err := snapshot.GetFile(ctx, uri)
if err != nil {
- return nil, nil, err
+ return nil, err
}
mth, err := snapshot.ModTidyHandle(ctx)
if err == source.ErrTmpModfileUnsupported {
- return nil, nil, nil
+ return nil, nil
}
if err != nil {
- return nil, nil, err
+ return nil, err
}
- missingDeps, diagnostics, err := mth.Tidy(ctx)
+ diagnostics, err := mth.Tidy(ctx)
if err != nil {
- return nil, nil, err
+ return nil, err
}
reports := map[source.FileIdentity][]*source.Diagnostic{
fh.Identity(): {},
@@ -54,12 +53,16 @@
} else {
diag.Severity = protocol.SeverityWarning
}
+ fh, err := snapshot.GetFile(ctx, e.URI)
+ if err != nil {
+ return nil, err
+ }
reports[fh.Identity()] = append(reports[fh.Identity()], diag)
}
- return reports, missingDeps, nil
+ return reports, nil
}
-func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, diags []protocol.Diagnostic) ([]protocol.CodeAction, error) {
+func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, diags []protocol.Diagnostic) ([]protocol.CodeAction, error) {
mth, err := snapshot.ModTidyHandle(ctx)
if err == source.ErrTmpModfileUnsupported {
return nil, nil
@@ -67,7 +70,7 @@
if err != nil {
return nil, err
}
- _, diagnostics, err := mth.Tidy(ctx)
+ diagnostics, err := mth.Tidy(ctx)
if err != nil {
return nil, err
}
@@ -113,81 +116,6 @@
return actions, nil
}
-func SuggestedGoFixes(ctx context.Context, snapshot source.Snapshot) (map[string]protocol.TextDocumentEdit, error) {
- uri := snapshot.View().ModFile()
- if uri == "" {
- return nil, nil
- }
- ctx, done := event.Start(ctx, "mod.SuggestedGoFixes", tag.URI.Of(uri))
- defer done()
-
- fh, err := snapshot.GetFile(ctx, uri)
- if err != nil {
- return nil, err
- }
- mth, err := snapshot.ModTidyHandle(ctx)
- if err == source.ErrTmpModfileUnsupported {
- return nil, nil
- }
- if err != nil {
- return nil, err
- }
- missingDeps, _, err := mth.Tidy(ctx)
- if err != nil {
- return nil, err
- }
- if len(missingDeps) == 0 {
- return nil, nil
- }
- pmh, err := snapshot.ParseModHandle(ctx, fh)
- if err != nil {
- return nil, err
- }
- _, m, _, err := pmh.Parse(ctx)
- if err != nil {
- return nil, err
- }
- // Get the contents of the go.mod file before we make any changes.
- oldContents, err := fh.Read()
- if err != nil {
- return nil, err
- }
- textDocumentEdits := make(map[string]protocol.TextDocumentEdit)
- for dep, req := range missingDeps {
- // We need a private copy of the parsed go.mod file, since we're going to
- // modify it.
- copied, err := modfile.Parse("", oldContents, nil)
- if err != nil {
- return nil, err
- }
- // Calculate the quick fix edits that need to be made to the go.mod file.
- if err := copied.AddRequire(req.Mod.Path, req.Mod.Version); err != nil {
- return nil, err
- }
- copied.SortBlocks()
- newContents, err := copied.Format()
- if err != nil {
- return nil, err
- }
- // Calculate the edits to be made due to the change.
- diff := snapshot.View().Options().ComputeEdits(fh.URI(), string(oldContents), string(newContents))
- edits, err := source.ToProtocolEdits(m, diff)
- if err != nil {
- return nil, err
- }
- textDocumentEdits[dep] = protocol.TextDocumentEdit{
- TextDocument: protocol.VersionedTextDocumentIdentifier{
- Version: fh.Version(),
- TextDocumentIdentifier: protocol.TextDocumentIdentifier{
- URI: protocol.URIFromSpanURI(fh.URI()),
- },
- },
- Edits: edits,
- }
- }
- return textDocumentEdits, nil
-}
-
func sameDiagnostic(d protocol.Diagnostic, e source.Error) bool {
return d.Message == e.Message && protocol.CompareRange(d.Range, e.Range) == 0 && d.Source == e.Category
}
diff --git a/internal/lsp/regtest/modfile_test.go b/internal/lsp/regtest/modfile_test.go
index 32a71fd..3b5c2f8 100644
--- a/internal/lsp/regtest/modfile_test.go
+++ b/internal/lsp/regtest/modfile_test.go
@@ -206,3 +206,89 @@
)
}, WithProxy(proxy))
}
+
+func TestBadlyVersionedModule(t *testing.T) {
+ t.Skipf("not fixed yet, see golang/go#39784")
+
+ const badModule = `
+-- example.com/blah/@v/list --
+v1.0.0
+-- example.com/blah/@v/v1.0.0.mod --
+module example.com
+
+go 1.12
+-- example.com/blah@v1.0.0/blah.go --
+package blah
+
+const Name = "Blah"
+-- example.com/blah@v1.0.0/blah_test.go --
+package blah_test
+
+import (
+ "testing"
+)
+
+func TestBlah(t *testing.T) {}
+
+-- example.com/blah/v2/@v/list --
+v2.0.0
+-- example.com/blah/v2/@v/v2.0.0.mod --
+module example.com
+
+go 1.12
+-- example.com/blah/v2@v2.0.0/blah.go --
+package blah
+
+const Name = "Blah"
+-- example.com/blah/v2@v2.0.0/blah_test.go --
+package blah_test
+
+import (
+ "testing"
+
+ "example.com/blah"
+)
+
+func TestBlah(t *testing.T) {}
+`
+ const pkg = `
+-- go.mod --
+module mod.com
+
+require (
+ example.com/blah/v2 v2.0.0
+)
+-- main.go --
+package main
+
+import "example.com/blah/v2"
+
+func main() {
+ fmt.Println(blah.Name)
+}
+`
+ runner.Run(t, pkg, func(t *testing.T, env *Env) {
+ env.OpenFile("main.go")
+ env.OpenFile("go.mod")
+ metBy := env.Await(
+ env.DiagnosticAtRegexp("go.mod", "example.com/blah/v2"),
+ NoDiagnostics("main.go"),
+ )
+ d, ok := metBy[0].(*protocol.PublishDiagnosticsParams)
+ if !ok {
+ t.Fatalf("unexpected type for metBy (%T)", metBy)
+ }
+ env.ApplyQuickFixes("main.go", d.Diagnostics)
+ const want = `
+module mod.com
+
+require (
+ example.com/blah v1.0.0
+ example.com/blah/v2 v2.0.0
+)
+`
+ if got := env.Editor.BufferText("go.mod"); got != want {
+ t.Fatalf("suggested fixes failed:\n%s", tests.Diff(want, got))
+ }
+ }, WithProxy(badModule))
+}
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index b244959..634bd85 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -7,11 +7,8 @@
import (
"context"
"fmt"
- "go/ast"
- "strconv"
"strings"
- "golang.org/x/mod/modfile"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/lsp/debug/tag"
@@ -41,7 +38,7 @@
Message string
}
-func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missingModules map[string]*modfile.Require, withAnalysis bool) (map[FileIdentity][]*Diagnostic, bool, error) {
+func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, withAnalysis bool) (map[FileIdentity][]*Diagnostic, bool, error) {
onlyIgnoredFiles := true
for _, pgh := range ph.CompiledGoFiles() {
onlyIgnoredFiles = onlyIgnoredFiles && snapshot.View().IgnoredFile(pgh.File().URI())
@@ -66,38 +63,10 @@
if len(pkg.CompiledGoFiles()) == 1 && hasUndeclaredErrors(pkg) {
warn = true
}
-
- missing := make(map[string]*modfile.Require)
- for _, imp := range pkg.Imports() {
- if req, ok := missingModules[imp.PkgPath()]; ok {
- missing[imp.PkgPath()] = req
- 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) {
- missing[imp.PkgPath()] = req
- break
- }
- }
- }
-
// Prepare the reports we will send for the files in this package.
reports := make(map[FileIdentity][]*Diagnostic)
for _, pgh := range pkg.CompiledGoFiles() {
clearReports(ctx, snapshot, reports, pgh.File().URI())
- if len(missing) > 0 {
- if err := missingModulesDiagnostics(ctx, snapshot, reports, missing, pgh.File().URI()); err != nil {
- return nil, warn, err
- }
- }
}
// Prepare any additional reports for the errors in this package.
for _, e := range pkg.GetErrors() {
@@ -172,7 +141,7 @@
if err != nil {
return FileIdentity{}, nil, err
}
- reports, _, err := Diagnostics(ctx, snapshot, ph, nil, true)
+ reports, _, err := Diagnostics(ctx, snapshot, ph, true)
if err != nil {
return FileIdentity{}, nil, err
}
@@ -237,62 +206,6 @@
return nonEmptyDiagnostics, hasTypeErrors, nil
}
-func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, missingModules map[string]*modfile.Require, uri span.URI) error {
- if len(missingModules) == 0 {
- return nil
- }
- fh, err := snapshot.GetFile(ctx, uri)
- if err != nil {
- return err
- }
- pgh := snapshot.View().Session().Cache().ParseGoHandle(ctx, fh, ParseHeader)
- file, _, m, _, err := pgh.Parse(ctx)
- if err != nil {
- 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, analyses map[string]Analyzer) error {
var analyzers []*analysis.Analyzer
for _, a := range analyses {
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 0057fda..195cb90 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -345,7 +345,7 @@
type ModTidyHandle interface {
// Tidy returns the results of `go mod tidy` for the module.
- Tidy(ctx context.Context) (map[string]*modfile.Require, []Error, error)
+ Tidy(ctx context.Context) ([]Error, error)
}
var ErrTmpModfileUnsupported = errors.New("-modfile is unsupported for this Go version")
@@ -487,6 +487,7 @@
ListError
ParseError
TypeError
+ ModTidyError
Analysis
)
diff --git a/internal/lsp/testdata/missingdep/primarymod/go.mod.golden b/internal/lsp/testdata/missingdep/primarymod/go.mod.golden
index 862c051..72492ed 100644
--- a/internal/lsp/testdata/missingdep/primarymod/go.mod.golden
+++ b/internal/lsp/testdata/missingdep/primarymod/go.mod.golden
@@ -1,4 +1,4 @@
--- suggestedfix_main_5_2 --
+-- suggestedfix_main_7_2 --
module missingdep
go 1.12
diff --git a/internal/lsp/testdata/missingdep/primarymod/main.go b/internal/lsp/testdata/missingdep/primarymod/main.go
index 18be555..0dab163 100644
--- a/internal/lsp/testdata/missingdep/primarymod/main.go
+++ b/internal/lsp/testdata/missingdep/primarymod/main.go
@@ -2,9 +2,12 @@
package missingdep
import (
- "example.com/extramodule/pkg" //@diag("\"example.com/extramodule/pkg\"", "go mod tidy", "example.com/extramodule is not in your go.mod file.", "warning"),suggestedfix("\"example.com/extramodule/pkg\"", "quickfix")
+ "fmt"
+
+ "example.com/extramodule/pkg" //@diag("\"example.com/extramodule/pkg\"", "go mod tidy", "example.com/extramodule is not in your go.mod file", "warning"),suggestedfix("\"example.com/extramodule/pkg\"", "quickfix")
)
func Yo() {
_ = pkg.Test
+ fmt.Printf("%s") //@diag("fmt.Printf(\"%s\")", "printf", "Printf format %s reads arg #1, but call has 0 args", "warning")
}
diff --git a/internal/lsp/testdata/missingdep/summary.txt.golden b/internal/lsp/testdata/missingdep/summary.txt.golden
index 5c4f74a..0c7b9bf 100644
--- a/internal/lsp/testdata/missingdep/summary.txt.golden
+++ b/internal/lsp/testdata/missingdep/summary.txt.golden
@@ -7,7 +7,7 @@
FuzzyCompletionsCount = 0
RankedCompletionsCount = 0
CaseSensitiveCompletionsCount = 0
-DiagnosticsCount = 1
+DiagnosticsCount = 2
FoldingRangesCount = 0
FormatCount = 0
ImportCount = 0
diff --git a/internal/lsp/testdata/missingtwodep/primarymod/main.go b/internal/lsp/testdata/missingtwodep/primarymod/main.go
index 081305d..a2b2454 100644
--- a/internal/lsp/testdata/missingtwodep/primarymod/main.go
+++ b/internal/lsp/testdata/missingtwodep/primarymod/main.go
@@ -2,9 +2,9 @@
package missingtwodep
import (
- "example.com/anothermodule/hey" //@diag("\"example.com/anothermodule/hey\"", "go mod tidy", "example.com/anothermodule is not in your go.mod file.", "warning"),suggestedfix("\"example.com/anothermodule/hey\"", "quickfix")
- "example.com/extramodule/pkg" //@diag("\"example.com/extramodule/pkg\"", "go mod tidy", "example.com/extramodule is not in your go.mod file.", "warning"),suggestedfix("\"example.com/extramodule/pkg\"", "quickfix")
- "example.com/extramodule/yo" //@diag("\"example.com/extramodule/yo\"", "go mod tidy", "example.com/extramodule is not in your go.mod file.", "warning"),suggestedfix("\"example.com/extramodule/yo\"", "quickfix")
+ "example.com/anothermodule/hey" //@diag("\"example.com/anothermodule/hey\"", "go mod tidy", "example.com/anothermodule is not in your go.mod file", "warning"),suggestedfix("\"example.com/anothermodule/hey\"", "quickfix")
+ "example.com/extramodule/pkg" //@diag("\"example.com/extramodule/pkg\"", "go mod tidy", "example.com/extramodule is not in your go.mod file", "warning"),suggestedfix("\"example.com/extramodule/pkg\"", "quickfix")
+ "example.com/extramodule/yo" //@diag("\"example.com/extramodule/yo\"", "go mod tidy", "example.com/extramodule is not in your go.mod file", "warning"),suggestedfix("\"example.com/extramodule/yo\"", "quickfix")
)
func Yo() {
