diff --git a/internal/lsp/cache/modfiles.go b/internal/lsp/cache/modfiles.go index 6d87f43b78..63403d01e1 100644 --- a/internal/lsp/cache/modfiles.go +++ b/internal/lsp/cache/modfiles.go @@ -20,17 +20,10 @@ import ( // This function will return the main go.mod file for this folder if it exists and whether the -modfile // flag exists for this version of go. func modfileFlagExists(ctx context.Context, folder string, env []string) (string, bool, error) { - var tempEnv []string - for i := range env { - tempEnv = append(tempEnv, env[i]) - if strings.Contains(env[i], "GO111MODULE") { - tempEnv[i] = "GO111MODULE=off" - } - } // Borrowed from (internal/imports/mod.go:620) const format = `{{range context.ReleaseTags}}{{if eq . "go1.14"}}{{.}}{{end}}{{end}}` // Check the go version by running "go list" with modules off. - stdout, err := source.InvokeGo(ctx, folder, tempEnv, "list", "-f", format) + stdout, err := source.InvokeGo(ctx, folder, append(env, "GO111MODULE=off"), "list", "-f", format) if err != nil { return "", false, err } @@ -66,12 +59,12 @@ func getModfiles(ctx context.Context, folder string, options source.Options) (*m if modfile == "" || modfile == os.DevNull { return nil, errors.Errorf("go env GOMOD cannot detect a go.mod file in this folder") } + // Copy the current go.mod file into the temporary go.mod file. tempFile, err := ioutil.TempFile("", "go.*.mod") if err != nil { return nil, err } defer tempFile.Close() - // Copy the current go.mod file into the temporary go.mod file. origFile, err := os.Open(modfile) if err != nil { return nil, err @@ -80,5 +73,20 @@ func getModfiles(ctx context.Context, folder string, options source.Options) (*m if _, err := io.Copy(tempFile, origFile); err != nil { return nil, err } + copySumFile(modfile, tempFile.Name()) return &modfiles{real: modfile, temp: tempFile.Name()}, nil } + +func copySumFile(realFile, tempFile string) { + realSum := realFile[0:len(realFile)-3] + "sum" + tempSum := tempFile[0:len(tempFile)-3] + "sum" + stat, err := os.Stat(realSum) + if err != nil || !stat.Mode().IsRegular() { + return + } + contents, err := ioutil.ReadFile(realSum) + if err != nil { + return + } + ioutil.WriteFile(tempSum, contents, stat.Mode()) +} diff --git a/internal/lsp/cache/parse_mod.go b/internal/lsp/cache/parse_mod.go index b08f752abf..7c358d3db1 100644 --- a/internal/lsp/cache/parse_mod.go +++ b/internal/lsp/cache/parse_mod.go @@ -28,11 +28,7 @@ type parseModData struct { } func (c *cache) ParseModHandle(fh source.FileHandle) source.ParseModHandle { - key := parseKey{ - file: fh.Identity(), - mode: source.ParseFull, - } - h := c.store.Bind(key, func(ctx context.Context) interface{} { + h := c.store.Bind(fh.Identity(), func(ctx context.Context) interface{} { data := &parseModData{} data.modfile, data.err = parseMod(ctx, fh) return data diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index e2eb5571f3..d9c41420b6 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -6,6 +6,7 @@ package cache import ( "context" + "io" "os" "sync" @@ -65,6 +66,39 @@ func (s *snapshot) View() source.View { return s.view } +func (s *snapshot) ModFiles(ctx context.Context) (source.FileHandle, source.FileHandle, error) { + if s.view.modfiles == nil { + return nil, nil, nil + } + realfh, err := s.GetFile(ctx, span.FileURI(s.view.modfiles.real)) + if err != nil { + return nil, nil, err + } + if realfh == nil { + return nil, nil, errors.Errorf("go.mod filehandle is nil") + } + // Copy the real go.mod file content into the temp go.mod file. + origFile, err := os.Open(s.view.modfiles.real) + if err != nil { + return nil, nil, err + } + defer origFile.Close() + tempFile, err := os.OpenFile(s.view.modfiles.temp, os.O_WRONLY, os.ModePerm) + if err != nil { + return nil, nil, err + } + defer tempFile.Close() + if _, err := io.Copy(tempFile, origFile); err != nil { + return nil, nil, err + } + // Go directly to disk to get the correct FileHandle, since we just copied the file without invalidating the snapshot. + tempfh := s.view.Session().Cache().GetFile(span.FileURI(s.view.modfiles.temp), source.Mod) + if tempfh == nil { + return nil, nil, errors.Errorf("temporary go.mod filehandle is nil") + } + return realfh, tempfh, nil +} + func (s *snapshot) PackageHandles(ctx context.Context, fh source.FileHandle) ([]source.PackageHandle, error) { // If the file is a go.mod file, go.Packages.Load will always return 0 packages. if fh.Identity().Kind == source.Mod { diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 6742ab5e9b..235d25683e 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -29,7 +29,7 @@ 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. - if err := source.ModTidy(ctx, view); err != nil { + 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 9ab6f0e550..99f6dd1614 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -8,6 +8,7 @@ import ( "context" "strings" + "golang.org/x/tools/internal/lsp/mod" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/telemetry" @@ -61,6 +62,8 @@ func (s *Server) diagnoseSnapshot(ctx context.Context, snapshot source.Snapshot) s.publishReports(ctx, reports, false) }(snapshot, fh) } + // Run diagnostics on the go.mod file. + s.diagnoseModfile(snapshot) } func (s *Server) diagnoseFile(snapshot source.Snapshot, fh source.FileHandle) { @@ -88,6 +91,23 @@ func (s *Server) diagnoseFile(snapshot source.Snapshot, fh source.FileHandle) { s.publishReports(ctx, reports, true) } +func (s *Server) diagnoseModfile(snapshot source.Snapshot) { + ctx := snapshot.View().BackgroundContext() + ctx, done := trace.StartSpan(ctx, "lsp:background-worker") + defer done() + + f, diags, err := mod.Diagnostics(ctx, snapshot) + if err != nil { + if err != context.Canceled { + log.Error(ctx, "diagnoseModfile: could not generate diagnostics", err) + } + return + } + reports := map[source.FileIdentity][]source.Diagnostic{f: diags} + // Publish empty diagnostics for files. + s.publishReports(ctx, reports, true) +} + func (s *Server) publishReports(ctx context.Context, reports map[source.FileIdentity][]source.Diagnostic, publishEmpty bool) { // Check for context cancellation before publishing diagnostics. if ctx.Err() != nil { diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go new file mode 100644 index 0000000000..eb5ff40a55 --- /dev/null +++ b/internal/lsp/mod/diagnostics.go @@ -0,0 +1,93 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package mod provides core features related to go.mod file +// handling for use by Go editors and tools. +package mod + +import ( + "context" + "fmt" + + "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" + "golang.org/x/tools/internal/telemetry/trace" +) + +func Diagnostics(ctx context.Context, snapshot source.Snapshot) (source.FileIdentity, []source.Diagnostic, error) { + // TODO: We will want to support diagnostics for go.mod files even when the -modfile flag is turned off. + realfh, tempfh, err := snapshot.ModFiles(ctx) + if err != nil { + return source.FileIdentity{}, nil, err + } + // Check the case when the tempModfile flag is turned off. + if realfh == nil || tempfh == nil { + return source.FileIdentity{}, nil, nil + } + + ctx, done := trace.StartSpan(ctx, "modfiles.Diagnostics", telemetry.File.Of(realfh.Identity().URI)) + defer done() + + // If the view has a temporary go.mod file, we want to run "go mod tidy" to be able to + // diff between the real and the temp files. + cfg := snapshot.View().Config(ctx) + args := append([]string{"mod", "tidy"}, cfg.BuildFlags...) + if _, err := source.InvokeGo(ctx, snapshot.View().Folder().Filename(), cfg.Env, args...); err != nil { + return source.FileIdentity{}, nil, err + } + + realMod, err := snapshot.View().Session().Cache().ParseModHandle(realfh).Parse(ctx) + if err != nil { + return source.FileIdentity{}, nil, err + } + tempMod, err := snapshot.View().Session().Cache().ParseModHandle(tempfh).Parse(ctx) + if err != nil { + return source.FileIdentity{}, nil, err + } + + reports := []source.Diagnostic{} + // Check indirect vs direct, and removal of dependencies. + realReqs := make(map[string]*modfile.Require, len(realMod.Require)) + tempReqs := make(map[string]*modfile.Require, len(tempMod.Require)) + for _, req := range realMod.Require { + realReqs[req.Mod.Path] = req + } + for _, req := range tempMod.Require { + realReq := realReqs[req.Mod.Path] + if realReq != nil && realReq.Indirect == req.Indirect { + delete(realReqs, req.Mod.Path) + } + tempReqs[req.Mod.Path] = req + } + for _, req := range realReqs { + if req.Syntax == nil { + continue + } + dep := req.Mod.Path + diag := &source.Diagnostic{ + Message: fmt.Sprintf("%s is not used in this module.", dep), + Source: "go mod tidy", + Range: protocol.Range{Start: getPos(req.Syntax.Start), End: getPos(req.Syntax.End)}, + Severity: protocol.SeverityWarning, + } + if tempReqs[dep] != nil && req.Indirect != tempReqs[dep].Indirect { + diag.Message = fmt.Sprintf("%s should be an indirect dependency.", dep) + if req.Indirect { + diag.Message = fmt.Sprintf("%s should not be an indirect dependency.", dep) + } + } + reports = append(reports, *diag) + } + return realfh.Identity(), reports, nil +} + +// TODO: Check to see if we need to go through internal/span. +func getPos(pos modfile.Position) protocol.Position { + return protocol.Position{ + Line: float64(pos.Line - 1), + Character: float64(pos.LineRune - 1), + } +} diff --git a/internal/lsp/mod/mod_test.go b/internal/lsp/mod/mod_test.go new file mode 100644 index 0000000000..4c5a48929c --- /dev/null +++ b/internal/lsp/mod/mod_test.go @@ -0,0 +1,135 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package mod_test + +import ( + "context" + "fmt" + "io/ioutil" + "os" + "path" + "path/filepath" + "testing" + + "golang.org/x/tools/internal/lsp/cache" + "golang.org/x/tools/internal/lsp/mod" + "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/lsp/tests" + "golang.org/x/tools/internal/span" + "golang.org/x/tools/internal/testenv" +) + +func TestMain(m *testing.M) { + testenv.ExitIfSmallMachine() + os.Exit(m.Run()) +} + +func TestDiagnostics(t *testing.T) { + ctx := tests.Context(t) + cache := cache.New(nil) + session := cache.NewSession(ctx) + options := tests.DefaultOptions() + options.TempModfile = true + options.Env = append(os.Environ(), "GOPACKAGESDRIVER=off", "GOROOT=") + + for _, tt := range []struct { + testdir string + want []source.Diagnostic + }{ + { + testdir: "indirect", + want: []source.Diagnostic{ + { + Message: "golang.org/x/tools should not be an indirect dependency.", + Source: "go mod tidy", + Range: protocol.Range{Start: getPos(4, 0), End: getPos(4, 61)}, + Severity: protocol.SeverityWarning, + }, + }, + }, + { + testdir: "unused", + want: []source.Diagnostic{ + { + Message: "golang.org/x/tools is not used in this module.", + Source: "go mod tidy", + Range: protocol.Range{Start: getPos(4, 0), End: getPos(4, 61)}, + Severity: protocol.SeverityWarning, + }, + }, + }, + } { + t.Run(tt.testdir, func(t *testing.T) { + // TODO: Once we refactor this to work with go/packages/packagestest. We do not + // need to copy to a temporary directory. + // Make sure to copy the test directory to a temporary directory so we do not + // modify the test code or add go.sum files when we run the tests. + folder, err := copyToTempDir(filepath.Join("testdata", tt.testdir)) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(folder) + _, snapshot, err := session.NewView(ctx, "diagnostics_test", span.FileURI(folder), options) + if err != nil { + t.Fatal(err) + } + // TODO: Add testing for when the -modfile flag is turned off and we still get diagnostics. + if !hasTempModfile(ctx, snapshot) { + return + } + fileID, got, err := mod.Diagnostics(ctx, snapshot) + if err != nil { + t.Fatal(err) + } + if diff := tests.DiffDiagnostics(fileID.URI, tt.want, got); diff != "" { + t.Error(diff) + } + }) + } +} + +func hasTempModfile(ctx context.Context, snapshot source.Snapshot) bool { + _, t, _ := snapshot.ModFiles(ctx) + return t != nil +} + +func copyToTempDir(folder string) (string, error) { + if _, err := os.Stat(folder); err != nil { + return "", err + } + dst, err := ioutil.TempDir("", "modfile_test") + if err != nil { + return "", err + } + fds, err := ioutil.ReadDir(folder) + if err != nil { + return "", err + } + for _, fd := range fds { + srcfp := path.Join(folder, fd.Name()) + dstfp := path.Join(dst, fd.Name()) + stat, err := os.Stat(srcfp) + if err != nil { + return "", err + } + if !stat.Mode().IsRegular() { + return "", fmt.Errorf("cannot copy non regular file %s", srcfp) + } + contents, err := ioutil.ReadFile(srcfp) + if err != nil { + return "", err + } + ioutil.WriteFile(dstfp, contents, stat.Mode()) + } + return dst, nil +} + +func getPos(line, character int) protocol.Position { + return protocol.Position{ + Line: float64(line), + Character: float64(character), + } +} diff --git a/internal/lsp/mod/testdata/indirect/go.mod b/internal/lsp/mod/testdata/indirect/go.mod new file mode 100644 index 0000000000..2e5dc1384a --- /dev/null +++ b/internal/lsp/mod/testdata/indirect/go.mod @@ -0,0 +1,5 @@ +module indirect + +go 1.12 + +require golang.org/x/tools v0.0.0-20191219192050-56b0b28a00f7 // indirect diff --git a/internal/lsp/mod/testdata/indirect/go.sum b/internal/lsp/mod/testdata/indirect/go.sum new file mode 100644 index 0000000000..8fec86ccd2 --- /dev/null +++ b/internal/lsp/mod/testdata/indirect/go.sum @@ -0,0 +1,12 @@ +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/tools v0.0.0-20191219192050-56b0b28a00f7 h1:u+nComwpgIe2VK1OTg8C74VQWda+MuB+wkIEsqFeoxY= +golang.org/x/tools v0.0.0-20191219192050-56b0b28a00f7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/lsp/mod/testdata/indirect/main.go b/internal/lsp/mod/testdata/indirect/main.go new file mode 100644 index 0000000000..a596abfd0c --- /dev/null +++ b/internal/lsp/mod/testdata/indirect/main.go @@ -0,0 +1,10 @@ +// Package indirect does something +package indirect + +import ( + "golang.org/x/tools/go/packages" +) + +func Yo() { + var _ packages.Config +} diff --git a/internal/lsp/mod/testdata/unused/go.mod b/internal/lsp/mod/testdata/unused/go.mod new file mode 100644 index 0000000000..2c2f19caf0 --- /dev/null +++ b/internal/lsp/mod/testdata/unused/go.mod @@ -0,0 +1,5 @@ +module unused + +go 1.12 + +require golang.org/x/tools v0.0.0-20191219192050-56b0b28a00f7 diff --git a/internal/lsp/mod/testdata/unused/go.sum b/internal/lsp/mod/testdata/unused/go.sum new file mode 100644 index 0000000000..e8a4a480de --- /dev/null +++ b/internal/lsp/mod/testdata/unused/go.sum @@ -0,0 +1,11 @@ +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/tools v0.0.0-20191219192050-56b0b28a00f7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/lsp/mod/testdata/unused/main.go b/internal/lsp/mod/testdata/unused/main.go new file mode 100644 index 0000000000..72bb9a8032 --- /dev/null +++ b/internal/lsp/mod/testdata/unused/main.go @@ -0,0 +1,5 @@ +// Package unused does something +package unused + +func Yo() { +} diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 43881a0b93..fd1259c9b9 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -43,6 +43,9 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, fh FileHandle, withAnal ctx, done := trace.StartSpan(ctx, "source.Diagnostics", telemetry.File.Of(fh.Identity().URI)) defer done() + if fh.Identity().Kind != Go { + return nil, "", errors.Errorf("unexpected file type: %q", fh.Identity().URI.Filename) + } phs, err := snapshot.PackageHandles(ctx, fh) if err != nil { return nil, "", err diff --git a/internal/lsp/source/tidy.go b/internal/lsp/source/tidy.go deleted file mode 100644 index 0370c7f2ca..0000000000 --- a/internal/lsp/source/tidy.go +++ /dev/null @@ -1,17 +0,0 @@ -package source - -import ( - "context" -) - -func ModTidy(ctx context.Context, view View) error { - cfg := view.Config(ctx) - - // Running `go mod tidy` modifies the file on disk directly. - // Ideally, we should return modules that could possibly be removed - // and apply each action as an edit. - // - // TODO(rstambler): This will be possible when golang/go#27005 is resolved. - _, err := InvokeGo(ctx, view.Folder().Filename(), cfg.Env, "mod", "tidy") - return err -} diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 3d9bb6bf53..541d9e203a 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -39,6 +39,9 @@ type Snapshot interface { // This is used to get the SuggestedFixes associated with that error. FindAnalysisError(ctx context.Context, pkgID, analyzerName, msg string, rng protocol.Range) (*Error, error) + // ModFiles returns the FileHandles of the go.mod files attached to the view associated with this snapshot. + ModFiles(ctx context.Context) (FileHandle, FileHandle, error) + // PackageHandle returns the CheckPackageHandle for the given package ID. PackageHandle(ctx context.Context, id string) (PackageHandle, error) diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 17fa2a3740..3795e70abf 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -85,7 +85,15 @@ func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocume if params.Text != nil { c.Text = []byte(*params.Text) } - _, err := s.session.DidModifyFile(ctx, c) + snapshots, err := s.session.DidModifyFile(ctx, c) + if err != nil { + return err + } + snapshot, _, err := snapshotOf(s.session, c.URI, snapshots) + if err != nil { + return err + } + go s.diagnoseModfile(snapshot) return err }