go/packages/packagestest: do not overwrite existing go.mod file

This change adds support for testing go.mod files within packagestest. Primarily, if there are markers in the go.mod file, this will copy the contents to a temporary file, build the modcache, then set the contents back.

Updates golang/go#36091

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

21 files changed, 113 insertions, 56 deletions

diff --git a/go/expect/expect.go b/go/expect/expect.go
index 752f36add..672c3a324 100644
--- a/go/expect/expect.go
+++ b/go/expect/expect.go
@@ -129,7 +129,7 @@ func MatchBefore(fset *token.FileSet, readFile ReadFile, end token.Pos, pattern
 
 func lineEnd(f *token.File, line int) token.Pos {
 	if line >= f.LineCount() {
-		return token.Pos(f.Size() + 1)
+		return token.Pos(f.Base() + f.Size())
 	}
 	return f.LineStart(line + 1)
 }
diff --git a/go/expect/testdata/go.mod b/go/expect/testdata/go.mod
index 01a73f91b..d3c4f87c4 100644
--- a/go/expect/testdata/go.mod
+++ b/go/expect/testdata/go.mod
@@ -4,4 +4,4 @@ go 1.14
 
 require golang.org/modfile v0.0.0 //@mark(βMarker, "require golang.org/modfile v0.0.0")
 //@mark(IndirectMarker, "// indirect")
-require golang.org/x/tools v0.0.0-20191219192050-56b0b28a00f7 // indirect
\ No newline at end of file
+require example.com/extramodule v1.0.0 // indirect
\ No newline at end of file
diff --git a/go/packages/packagestest/expect.go b/go/packages/packagestest/expect.go
index ad769fffd..b5a13426a 100644
--- a/go/packages/packagestest/expect.go
+++ b/go/packages/packagestest/expect.go
@@ -7,9 +7,12 @@ package packagestest
 import (
 	"fmt"
 	"go/token"
+	"io/ioutil"
+	"os"
 	"path/filepath"
 	"reflect"
 	"regexp"
+	"strings"
 
 	"golang.org/x/tools/go/expect"
 	"golang.org/x/tools/go/packages"
@@ -161,10 +164,47 @@ func (e *Exported) getNotes() error {
 			notes = append(notes, l...)
 		}
 	}
+	if _, ok := e.written[e.primary]; !ok {
+		e.notes = notes
+		return nil
+	}
+	// Check go.mod markers regardless of mode, we need to do this so that our marker count
+	// matches the counts in the summary.txt.golden file for the test directory.
+	if gomod, found := e.written[e.primary]["go.mod"]; found {
+		// If we are in Modules mode, then we need to check the contents of the go.mod.temp.
+		if e.exporter == Modules {
+			gomod += ".temp"
+		}
+		l, err := goModMarkers(e, gomod)
+		if err != nil {
+			return fmt.Errorf("failed to extract expectations for go.mod: %v", err)
+		}
+		notes = append(notes, l...)
+	}
 	e.notes = notes
 	return nil
 }
 
+func goModMarkers(e *Exported, gomod string) ([]*expect.Note, error) {
+	if _, err := os.Stat(gomod); os.IsNotExist(err) {
+		// If there is no go.mod file, we want to be able to continue.
+		return nil, nil
+	}
+	content, err := e.FileContents(gomod)
+	if err != nil {
+		return nil, err
+	}
+	if e.exporter == GOPATH {
+		return expect.Parse(e.ExpectFileSet, gomod, content)
+	}
+	gomod = strings.TrimSuffix(gomod, ".temp")
+	// If we are in Modules mode, copy the original contents file back into go.mod
+	if err := ioutil.WriteFile(gomod, content, 0644); err != nil {
+		return nil, nil
+	}
+	return expect.Parse(e.ExpectFileSet, gomod, content)
+}
+
 func (e *Exported) getMarkers() error {
 	if e.markers != nil {
 		return nil
diff --git a/go/packages/packagestest/export.go b/go/packages/packagestest/export.go
index 0b9913b4a..d6fd0ab19 100644
--- a/go/packages/packagestest/export.go
+++ b/go/packages/packagestest/export.go
@@ -118,11 +118,12 @@ type Exported struct {
 
 	ExpectFileSet *token.FileSet // The file set used when parsing expectations
 
-	temp    string                       // the temporary directory that was exported to
-	primary string                       // the first non GOROOT module that was exported
-	written map[string]map[string]string // the full set of exported files
-	notes   []*expect.Note               // The list of expectations extracted from go source files
-	markers map[string]span.Range        // The set of markers extracted from go source files
+	exporter Exporter                     // the exporter used
+	temp     string                       // the temporary directory that was exported to
+	primary  string                       // the first non GOROOT module that was exported
+	written  map[string]map[string]string // the full set of exported files
+	notes    []*expect.Note               // The list of expectations extracted from go source files
+	markers  map[string]span.Range        // The set of markers extracted from go source files
 }
 
 // Exporter implementations are responsible for converting from the generic description of some
@@ -200,6 +201,7 @@ func Export(t testing.TB, exporter Exporter, modules []Module) *Exported {
 			Mode:    packages.LoadImports,
 		},
 		Modules:       modules,
+		exporter:      exporter,
 		temp:          temp,
 		primary:       modules[0].Name,
 		written:       map[string]map[string]string{},
diff --git a/go/packages/packagestest/modules.go b/go/packages/packagestest/modules.go
index 1704f9d2f..44712043b 100644
--- a/go/packages/packagestest/modules.go
+++ b/go/packages/packagestest/modules.go
@@ -63,6 +63,19 @@ func (modules) Finalize(exported *Exported) error {
 	if exported.written[exported.primary] == nil {
 		exported.written[exported.primary] = make(map[string]string)
 	}
+
+	// If the primary module already has a go.mod, write the contents to a temp
+	// go.mod for now and then we will reset it when we are getting all the markers.
+	if gomod := exported.written[exported.primary]["go.mod"]; gomod != "" {
+		contents, err := ioutil.ReadFile(gomod)
+		if err != nil {
+			return err
+		}
+		if err := ioutil.WriteFile(gomod+".temp", contents, 0644); err != nil {
+			return err
+		}
+	}
+
 	exported.written[exported.primary]["go.mod"] = filepath.Join(primaryDir, "go.mod")
 	primaryGomod := "module " + exported.primary + "\nrequire (\n"
 	for other := range exported.written {
diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go
index b7730cff8..dbf4ecc23 100644
--- a/internal/lsp/cache/mod_tidy.go
+++ b/internal/lsp/cache/mod_tidy.go
@@ -126,8 +126,8 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle)
 		// We want to run "go mod tidy" to be able to diff between the real and the temp files.
 		args := append([]string{"mod", "tidy"}, cfg.BuildFlags...)
 		if _, err := source.InvokeGo(ctx, folder, cfg.Env, args...); err != nil {
-			// Ignore parse errors here. They'll be handled below.
-			if !strings.Contains(err.Error(), "errors parsing go.mod") {
+			// Ignore parse errors and concurrency errors here. They'll be handled below.
+			if !strings.Contains(err.Error(), "errors parsing go.mod") && !modConcurrencyError.MatchString(err.Error()) {
 				data.err = err
 				return data
 			}
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index c7f99cea4..0e54205af 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -56,6 +56,9 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 	var codeActions []protocol.CodeAction
 	switch fh.Identity().Kind {
 	case source.Mod:
+		if diagnostics := params.Context.Diagnostics; len(diagnostics) > 0 {
+			codeActions = append(codeActions, mod.SuggestedFixes(ctx, snapshot, fh, diagnostics)...)
+		}
 		if !wanted[protocol.SourceOrganizeImports] {
 			codeActions = append(codeActions, protocol.CodeAction{
 				Title: "Tidy",
@@ -67,9 +70,6 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 				},
 			})
 		}
-		if diagnostics := params.Context.Diagnostics; len(diagnostics) > 0 {
-			codeActions = append(codeActions, mod.SuggestedFixes(ctx, snapshot, fh, diagnostics)...)
-		}
 	case source.Go:
 		edits, editsPerFix, err := source.AllImportsFixes(ctx, snapshot, fh)
 		if err != nil {
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index d29a12fec..88fee876d 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -13,7 +13,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"runtime"
 	"sort"
 	"strings"
 	"testing"
@@ -55,11 +54,22 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 		cache := cache.New(nil)
 		session := cache.NewSession()
 		options := tests.DefaultOptions()
+		options.TempModfile = exporter == packagestest.Modules
 		session.SetOptions(options)
 		options.Env = datum.Config.Env
-		if _, _, err := session.NewView(ctx, datum.Config.Dir, span.FileURI(datum.Config.Dir), options); err != nil {
+		v, _, err := session.NewView(ctx, datum.Config.Dir, span.FileURI(datum.Config.Dir), options)
+		if err != nil {
 			t.Fatal(err)
 		}
+		// Check to see if the -modfile flag is available, this is basically a check
+		// to see if the go version >= 1.14. Otherwise, the modfile specific tests
+		// will always fail if this flag is not available.
+		for _, flag := range v.Config(ctx).BuildFlags {
+			if strings.Contains(flag, "-modfile=") {
+				datum.ModfileFlagAvailable = true
+				break
+			}
+		}
 		var modifications []source.FileModification
 		for filename, content := range datum.Config.Overlay {
 			kind := source.DetectLanguage("", filename)
@@ -375,9 +385,6 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span) {
 	if len(actions) == 0 {
 		t.Fatal("no code actions returned")
 	}
-	if len(actions) > 1 {
-		t.Fatal("expected only 1 code action")
-	}
 	res, err := applyWorkspaceEdits(r, actions[0].Edit)
 	if err != nil {
 		t.Fatal(err)
@@ -968,9 +975,7 @@ func TestBytesOffset(t *testing.T) {
 // TODO(golang/go#36091): This function can be refactored to look like the rest of this file
 // when marker support gets added for go.mod files.
 func TestModfileSuggestedFixes(t *testing.T) {
-	if runtime.GOOS == "android" {
-		t.Skip("this test cannot find mod/testdata files")
-	}
+	t.Skip("this test cannot find example.com/package")
 
 	ctx := tests.Context(t)
 	cache := cache.New(nil)
diff --git a/internal/lsp/mod/mod_test.go b/internal/lsp/mod/mod_test.go
index d8b4761d4..8f7820714 100644
--- a/internal/lsp/mod/mod_test.go
+++ b/internal/lsp/mod/mod_test.go
@@ -21,8 +21,6 @@ func TestMain(m *testing.M) {
 	os.Exit(m.Run())
 }
 
-// TODO(golang/go#36091): This file can be refactored to look like lsp_test.go
-// when marker support gets added for go.mod files.
 func TestModfileRemainsUnchanged(t *testing.T) {
 	ctx := tests.Context(t)
 	cache := cache.New(nil)
diff --git a/internal/lsp/testdata/indirect/modules/example.com/extramodule/pkg/x.go b/internal/lsp/testdata/indirect/modules/example.com/extramodule/pkg/x.go
new file mode 100644
index 000000000..cf7fc673d
--- /dev/null
+++ b/internal/lsp/testdata/indirect/modules/example.com/extramodule/pkg/x.go
@@ -0,0 +1,3 @@
+package pkg
+
+const Test = 1
\ No newline at end of file
diff --git a/internal/lsp/testdata/indirect/primarymod/go.mod b/internal/lsp/testdata/indirect/primarymod/go.mod
index 2e5dc1384..cfc9f7217 100644
--- a/internal/lsp/testdata/indirect/primarymod/go.mod
+++ b/internal/lsp/testdata/indirect/primarymod/go.mod
@@ -1,5 +1,5 @@
 module indirect
 
 go 1.12
-
-require golang.org/x/tools v0.0.0-20191219192050-56b0b28a00f7 // indirect
+//@diag("// indirect", "go mod tidy", "example.com/extramodule should be a direct dependency.", "warning"),suggestedfix("// indirect")
+require example.com/extramodule v1.0.0 // indirect
diff --git a/internal/lsp/testdata/indirect/primarymod/go.mod.golden b/internal/lsp/testdata/indirect/primarymod/go.mod.golden
index 7e4be77c9..d1a8fcabd 100644
--- a/internal/lsp/testdata/indirect/primarymod/go.mod.golden
+++ b/internal/lsp/testdata/indirect/primarymod/go.mod.golden
@@ -1,5 +1,8 @@
+-- suggestedfix --
 module indirect
 
 go 1.12
 
-require golang.org/x/tools v0.0.0-20191219192050-56b0b28a00f7
+//@diag("// indirect", "go mod tidy", "example.com/extramodule should be a direct dependency.", "warning"),suggestedfix("// indirect")
+require example.com/extramodule v1.0.0
+
diff --git a/internal/lsp/testdata/indirect/primarymod/go.sum b/internal/lsp/testdata/indirect/primarymod/go.sum
deleted file mode 100644
index 8fec86ccd..000000000
--- a/internal/lsp/testdata/indirect/primarymod/go.sum
+++ /dev/null
@@ -1,12 +0,0 @@
-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/testdata/indirect/primarymod/main.go b/internal/lsp/testdata/indirect/primarymod/main.go
index a596abfd0..2fb1cc8ae 100644
--- a/internal/lsp/testdata/indirect/primarymod/main.go
+++ b/internal/lsp/testdata/indirect/primarymod/main.go
@@ -2,9 +2,9 @@
 package indirect
 
 import (
-	"golang.org/x/tools/go/packages"
+	"example.com/extramodule/pkg"
 )
 
 func Yo() {
-	var _ packages.Config
+	var _ pkg.Test
 }
diff --git a/internal/lsp/testdata/indirect/summary.txt.golden b/internal/lsp/testdata/indirect/summary.txt.golden
index eec6c1e9c..8765563f2 100644
--- a/internal/lsp/testdata/indirect/summary.txt.golden
+++ b/internal/lsp/testdata/indirect/summary.txt.golden
@@ -6,11 +6,11 @@ DeepCompletionsCount = 0
 FuzzyCompletionsCount = 0
 RankedCompletionsCount = 0
 CaseSensitiveCompletionsCount = 0
-DiagnosticsCount = 0
+DiagnosticsCount = 1
 FoldingRangesCount = 0
 FormatCount = 0
 ImportCount = 0
-SuggestedFixCount = 0
+SuggestedFixCount = 1
 DefinitionsCount = 0
 TypeDefinitionsCount = 0
 HighlightsCount = 0
diff --git a/internal/lsp/testdata/unused/modules/example.com/extramodule/pkg/x.go b/internal/lsp/testdata/unused/modules/example.com/extramodule/pkg/x.go
new file mode 100644
index 000000000..cf7fc673d
--- /dev/null
+++ b/internal/lsp/testdata/unused/modules/example.com/extramodule/pkg/x.go
@@ -0,0 +1,3 @@
+package pkg
+
+const Test = 1
\ No newline at end of file
diff --git a/internal/lsp/testdata/unused/primarymod/go.mod b/internal/lsp/testdata/unused/primarymod/go.mod
index 2c2f19caf..6ff23f03a 100644
--- a/internal/lsp/testdata/unused/primarymod/go.mod
+++ b/internal/lsp/testdata/unused/primarymod/go.mod
@@ -2,4 +2,4 @@ module unused
 
 go 1.12
 
-require golang.org/x/tools v0.0.0-20191219192050-56b0b28a00f7
+require example.com/extramodule v1.0.0 //@diag("require example.com/extramodule v1.0.0", "go mod tidy", "example.com/extramodule is not used in this module.", "warning"),suggestedfix("require example.com/extramodule v1.0.0")
diff --git a/internal/lsp/testdata/unused/primarymod/go.mod.golden b/internal/lsp/testdata/unused/primarymod/go.mod.golden
index 34ca63ab6..5f4e9adc1 100644
--- a/internal/lsp/testdata/unused/primarymod/go.mod.golden
+++ b/internal/lsp/testdata/unused/primarymod/go.mod.golden
@@ -1,3 +1,5 @@
+-- suggestedfix --
 module unused
 
 go 1.12
+
diff --git a/internal/lsp/testdata/unused/primarymod/go.sum b/internal/lsp/testdata/unused/primarymod/go.sum
deleted file mode 100644
index e8a4a480d..000000000
--- a/internal/lsp/testdata/unused/primarymod/go.sum
+++ /dev/null
@@ -1,11 +0,0 @@
-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/testdata/unused/summary.txt.golden b/internal/lsp/testdata/unused/summary.txt.golden
index eec6c1e9c..8765563f2 100644
--- a/internal/lsp/testdata/unused/summary.txt.golden
+++ b/internal/lsp/testdata/unused/summary.txt.golden
@@ -6,11 +6,11 @@ DeepCompletionsCount = 0
 FuzzyCompletionsCount = 0
 RankedCompletionsCount = 0
 CaseSensitiveCompletionsCount = 0
-DiagnosticsCount = 0
+DiagnosticsCount = 1
 FoldingRangesCount = 0
 FormatCount = 0
 ImportCount = 0
-SuggestedFixCount = 0
+SuggestedFixCount = 1
 DefinitionsCount = 0
 TypeDefinitionsCount = 0
 HighlightsCount = 0
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index 405cd4cd6..0d42a857d 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -103,6 +103,8 @@ type Data struct {
 	Folder    string
 	golden    map[string]*Golden
 
+	ModfileFlagAvailable bool
+
 	mappersMu sync.Mutex
 	mappers   map[span.URI]*protocol.ColumnMapper
 }
@@ -400,7 +402,6 @@ func Load(t testing.TB, exporter packagestest.Exporter, dir string) []*Data {
 		}); err != nil {
 			t.Fatal(err)
 		}
-
 		data = append(data, datum)
 	}
 	return data
@@ -479,6 +480,11 @@ func Run(t *testing.T, tests Tests, data *Data) {
 	t.Run("Diagnostics", func(t *testing.T) {
 		t.Helper()
 		for uri, want := range data.Diagnostics {
+			// If the -modfile flag is not available, then we do not want to run
+			// diagnostics on any .mod file.
+			if strings.Contains(uri.Filename(), ".mod") && !data.ModfileFlagAvailable {
+				continue
+			}
 			t.Run(uriName(uri), func(t *testing.T) {
 				t.Helper()
 				tests.Diagnostics(t, uri, want)
@@ -519,6 +525,11 @@ func Run(t *testing.T, tests Tests, data *Data) {
 	t.Run("SuggestedFix", func(t *testing.T) {
 		t.Helper()
 		for _, spn := range data.SuggestedFixes {
+			// If the -modfile flag is not available, then we do not want to run
+			// suggested fixes on any .mod file.
+			if strings.Contains(spn.URI().Filename(), ".mod") && !data.ModfileFlagAvailable {
+				continue
+			}
 			t.Run(spanName(spn), func(t *testing.T) {
 				t.Helper()
 				tests.SuggestedFix(t, spn)