internal/lsp: add warning diagnostics for unused dependencies in go.mod files

This is the first step to surfacing potential fixes and suggestions to a user's go.mod file. Specifically, it will show a warning if you have a dependency that is not used, or if a dependency is declared as indirect when it should be direct and vice versa.

This CL adds functionality for version of Go that are >= 1.14.

Updates golang/go#31999

Change-Id: Id60fa0ee201dcd843f62e2659dda8e795bd671db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211937
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

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())
+}

internal/lsp/cache/parse_mod.go

@@ -28,11 +28,7 @@ type parseModData struct {
 }
 
 func (c *cache) ParseModHandle(fh source.FileHandle) source.ParseModHandle {
-	key := parseKey{
+	h := c.store.Bind(fh.Identity(), func(ctx context.Context) interface{} {
-		file: fh.Identity(),
-		mode: source.ParseFull,
-	}
-	h := c.store.Bind(key, func(ctx context.Context) interface{} {
 		data := &parseModData{}
 		data.modfile, data.err = parseMod(ctx, fh)
 		return data

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 {

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
 		}
 	}

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 {

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),
+	}
+}

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),
+	}
+}

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

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=

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
+}

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

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=

internal/lsp/mod/testdata/unused/main.go

@@ -0,0 +1,5 @@
+// Package unused does something
+package unused
+
+func Yo() {
+}

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

internal/lsp/source/tidy.go

@@ -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
-}

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)
 

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
 }