internal/lsp/cache: parse go.mod file before running go mod tidy

This change reorders the logic within ModTidyHandle and ParseModHandle to parse the modfile first before we copy the contents to the temporary go.mod file. This was causing issues where a go.mod would be in a bad state and then we would try to run "go mod tidy" on the corrupted file.

Change-Id: I1df8fb70f5f3e2bcff306a58b16bc96c32debf2a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219480
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go
index 4d23481..1241c8b 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -124,46 +124,37 @@
 		cfg:   hashConfig(cfg),
 	}
 	h := s.view.session.cache.store.Bind(key, func(ctx context.Context) interface{} {
-		data := &modTidyData{}
-
-		uri := fh.Identity().URI
-
-		ctx, done := trace.StartSpan(ctx, "cache.ParseModHandle", telemetry.File.Of(uri))
+		ctx, done := trace.StartSpan(ctx, "cache.ParseModHandle", telemetry.File.Of(realURI))
 		defer done()
 
+		data := &modTidyData{}
 		contents, _, err := fh.Read(ctx)
 		if err != nil {
 			data.err = err
 			return data
 		}
-		// If the filehandle passed in is equal to the view's go.mod file, and we have
-		// a tempModfile, copy the real go.mod file content into the temp go.mod file.
-		if realURI == uri && tempURI != "" {
+		parsedFile, err := modfile.Parse(realURI.Filename(), contents, nil)
+		if err != nil {
+			data.err = err
+			return data
+		}
+		// If we have a tempModfile, copy the real go.mod file content into the temp go.mod file.
+		if tempURI != "" {
 			if err := ioutil.WriteFile(tempURI.Filename(), contents, os.ModePerm); err != nil {
 				data.err = err
 				return data
 			}
 		}
-		mapper := &protocol.ColumnMapper{
-			URI:       uri,
-			Converter: span.NewContentConverter(uri.Filename(), contents),
-			Content:   contents,
-		}
-		parsedFile, err := modfile.Parse(uri.Filename(), contents, nil)
-		if err != nil {
-			data.err = err
-			return data
-		}
 		data = &modTidyData{
 			origfh:         fh,
 			origParsedFile: parsedFile,
-			origMapper:     mapper,
+			origMapper: &protocol.ColumnMapper{
+				URI:       realURI,
+				Converter: span.NewContentConverter(realURI.Filename(), contents),
+				Content:   contents,
+			},
 		}
-		// Only check if any dependencies can be upgraded if the passed in
-		// go.mod file is equal to the view's go.mod file.
-		if realURI == uri {
-			data.upgrades, data.err = dependencyUpgrades(ctx, cfg, folder, data)
-		}
+		data.upgrades, data.err = dependencyUpgrades(ctx, cfg, folder, data)
 		return data
 	})
 	return &parseModHandle{
@@ -255,27 +246,11 @@
 		ctx, done := trace.StartSpan(ctx, "cache.ModTidyHandle", telemetry.File.Of(realURI))
 		defer done()
 
-		// Copy the real go.mod file content into the temp go.mod file.
 		realContents, _, err := realfh.Read(ctx)
 		if err != nil {
 			data.err = err
 			return data
 		}
-		if err := ioutil.WriteFile(tempURI.Filename(), realContents, os.ModePerm); err != nil {
-			data.err = err
-			return data
-		}
-
-		// 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 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
-			}
-		}
-
 		realMapper := &protocol.ColumnMapper{
 			URI:       realURI,
 			Converter: span.NewContentConverter(realURI.Filename(), realContents),
@@ -291,6 +266,22 @@
 			return data
 		}
 
+		// Copy the real go.mod file content into the temp go.mod file.
+		if err := ioutil.WriteFile(tempURI.Filename(), realContents, os.ModePerm); err != nil {
+			data.err = err
+			return data
+		}
+
+		// 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 concurrency errors here.
+			if !modConcurrencyError.MatchString(err.Error()) {
+				data.err = err
+				return data
+			}
+		}
+
 		// Go directly to disk to get the temporary mod file, since it is always on disk.
 		tempContents, err := ioutil.ReadFile(tempURI.Filename())
 		if err != nil {