internal/lsp: remove dependencies using text edits when necessary

golang/go#43335 explains the issues with using `go get module@none`,
which will only be resolved in Go 1.17. In the meantime, we use the
go command whenever possible, but if the module is not tidied, we
have to use textual edits instead. This means the go.sum file will not
be accurately updated to remove the dependency, but unfortunately, I
don't believe there is anything that we can do in that case.

Fixes golang/go#43335

Change-Id: I771f68f34a6136e73e9dd82b692ed4c235c3b293
Reviewed-on: https://go-review.googlesource.com/c/tools/+/279716
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
gopls-CI: kokoro <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: stamblerre

gopls/internal/regtest/modfile_test.go

@@ -816,3 +816,126 @@ func hello() {}
 		}
 	})
 }
+
+func TestRemoveUnusedDependency(t *testing.T) {
+	testenv.NeedsGo1Point(t, 14)
+
+	const proxy = `
+-- [email protected]/go.mod --
+module hasdep.com
+
+go 1.12
+
+require example.com v1.2.3
+-- [email protected]/a/a.go --
+package a
+-- [email protected]/go.mod --
+module example.com
+
+go 1.12
+-- [email protected]/blah/blah.go --
+package blah
+
+const Name = "Blah"
+-- [email protected]/go.mod --
+module random.com
+
+go 1.12
+-- [email protected]/blah/blah.go --
+package blah
+
+const Name = "Blah"
+`
+	t.Run("almost tidied", func(t *testing.T) {
+		const mod = `
+-- go.mod --
+module mod.com
+
+go 1.12
+
+require hasdep.com v1.2.3
+-- go.sum --
+example.com v1.2.3 h1:ihBTGWGjTU3V4ZJ9OmHITkU9WQ4lGdQkMjgyLFk0FaY=
+example.com v1.2.3/go.mod h1:Y2Rc5rVWjWur0h3pd9aEvK5Pof8YKDANh9gHA2Maujo=
+hasdep.com v1.2.3 h1:00y+N5oD+SpKoqV1zP2VOPawcW65Zb9NebANY3GSzGI=
+hasdep.com v1.2.3/go.mod h1:ePVZOlez+KZEOejfLPGL2n4i8qiAjrkhQZ4wcImqAes=
+-- main.go --
+package main
+
+func main() {}
+`
+		withOptions(
+			ProxyFiles(proxy),
+		).run(t, mod, func(t *testing.T, env *Env) {
+			d := &protocol.PublishDiagnosticsParams{}
+			env.Await(
+				OnceMet(
+					env.DiagnosticAtRegexp("go.mod", "require hasdep.com v1.2.3"),
+					ReadDiagnostics("go.mod", d),
+				),
+			)
+			const want = `module mod.com
+
+go 1.12
+`
+			env.ApplyQuickFixes("go.mod", d.Diagnostics)
+			if got := env.ReadWorkspaceFile("go.mod"); got != want {
+				t.Fatalf("unexpected content in go.mod:\n%s", tests.Diff(t, want, got))
+			}
+		})
+	})
+
+	t.Run("not tidied", func(t *testing.T) {
+		const mod = `
+-- go.mod --
+module mod.com
+
+go 1.12
+
+require hasdep.com v1.2.3
+require random.com v1.2.3
+-- go.sum --
+example.com v1.2.3 h1:ihBTGWGjTU3V4ZJ9OmHITkU9WQ4lGdQkMjgyLFk0FaY=
+example.com v1.2.3/go.mod h1:Y2Rc5rVWjWur0h3pd9aEvK5Pof8YKDANh9gHA2Maujo=
+hasdep.com v1.2.3 h1:00y+N5oD+SpKoqV1zP2VOPawcW65Zb9NebANY3GSzGI=
+hasdep.com v1.2.3/go.mod h1:ePVZOlez+KZEOejfLPGL2n4i8qiAjrkhQZ4wcImqAes=
+random.com v1.2.3 h1:PzYTykzqqH6+qU0dIgh9iPFbfb4Mm8zNBjWWreRKtx0=
+random.com v1.2.3/go.mod h1:8EGj+8a4Hw1clAp8vbaeHAsKE4sbm536FP7nKyXO+qQ=
+-- main.go --
+package main
+
+func main() {}
+`
+		withOptions(
+			ProxyFiles(proxy),
+		).run(t, mod, func(t *testing.T, env *Env) {
+			d := &protocol.PublishDiagnosticsParams{}
+			env.OpenFile("go.mod")
+			pos := env.RegexpSearch("go.mod", "require hasdep.com v1.2.3")
+			env.Await(
+				OnceMet(
+					DiagnosticAt("go.mod", pos.Line, pos.Column),
+					ReadDiagnostics("go.mod", d),
+				),
+			)
+			const want = `module mod.com
+
+go 1.12
+
+require random.com v1.2.3
+`
+			var diagnostics []protocol.Diagnostic
+			for _, d := range d.Diagnostics {
+				if d.Range.Start.Line != float64(pos.Line) {
+					continue
+				}
+				diagnostics = append(diagnostics, d)
+			}
+			env.ApplyQuickFixes("go.mod", diagnostics)
+			if got := env.Editor.BufferText("go.mod"); got != want {
+				t.Fatalf("unexpected content in go.mod:\n%s", tests.Diff(t, want, got))
+			}
+		})
+	})
+
+}

internal/lsp/cache/mod_tidy.go

@@ -274,13 +274,6 @@ func modTidyErrors(ctx context.Context, snapshot source.Snapshot, pm *source.Par
 		}
 		delete(unused, req.Mod.Path)
 	}
-	for _, req := range unused {
-		srcErr, err := unusedError(pm.Mapper, req, snapshot.View().Options().ComputeEdits)
-		if err != nil {
-			return nil, err
-		}
-		errors = append(errors, srcErr)
-	}
 	for _, req := range wrongDirectness {
 		// Handle dependencies that are incorrectly labeled indirect and
 		// vice versa.
@@ -380,16 +373,25 @@ func modTidyErrors(ctx context.Context, snapshot source.Snapshot, pm *source.Par
 			}
 		}
 	}
+	// Finally, add errors for any unused dependencies.
+	onlyError := len(errors) == 0 && len(unused) == 1
+	for _, req := range unused {
+		srcErr, err := unusedError(pm.Mapper, req, onlyError, snapshot.View().Options().ComputeEdits)
+		if err != nil {
+			return nil, err
+		}
+		errors = append(errors, srcErr)
+	}
 	return errors, nil
 }
 
 // unusedError returns a source.Error for an unused require.
-func unusedError(m *protocol.ColumnMapper, req *modfile.Require, computeEdits diff.ComputeEdits) (*source.Error, error) {
+func unusedError(m *protocol.ColumnMapper, req *modfile.Require, onlyError bool, computeEdits diff.ComputeEdits) (*source.Error, error) {
 	rng, err := rangeFromPositions(m, req.Syntax.Start, req.Syntax.End)
 	if err != nil {
 		return nil, err
 	}
-	args, err := source.MarshalArgs(m.URI, false, []string{req.Mod.Path + "@none"})
+	args, err := source.MarshalArgs(m.URI, onlyError, req.Mod.Path)
 	if err != nil {
 		return nil, err
 	}

internal/lsp/command.go

@@ -14,6 +14,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"golang.org/x/mod/modfile"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/gocommand"
 	"golang.org/x/tools/internal/lsp/cache"
@@ -216,7 +217,7 @@ func (s *Server) runCommand(ctx context.Context, work *workDone, command *source
 			return err
 		}
 		return runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, uri.SpanURI(), "list", []string{"all"})
-	case source.CommandAddDependency, source.CommandUpgradeDependency, source.CommandRemoveDependency:
+	case source.CommandAddDependency, source.CommandUpgradeDependency:
 		var uri protocol.DocumentURI
 		var goCmdArgs []string
 		var addRequire bool
@@ -229,6 +230,56 @@ func (s *Server) runCommand(ctx context.Context, work *workDone, command *source
 			return err
 		}
 		return s.runGoGetModule(ctx, snapshot, uri.SpanURI(), addRequire, goCmdArgs)
+	case source.CommandRemoveDependency:
+		var uri protocol.DocumentURI
+		var modulePath string
+		var onlyError bool
+		if err := source.UnmarshalArgs(args, &uri, &onlyError, &modulePath); err != nil {
+			return err
+		}
+		snapshot, fh, ok, release, err := s.beginFileRequest(ctx, uri, source.UnknownKind)
+		defer release()
+		if !ok {
+			return err
+		}
+		// If the module is tidied apart from the one unused diagnostic, we can
+		// run `go get module@none`, and then run `go mod tidy`. Otherwise, we
+		// must make textual edits.
+		// TODO(rstambler): In Go 1.17+, we will be able to use the go command
+		// without checking if the module is tidy.
+		if onlyError {
+			if err := s.runGoGetModule(ctx, snapshot, uri.SpanURI(), false, []string{modulePath + "@none"}); err != nil {
+				return err
+			}
+			return runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, uri.SpanURI(), "mod", []string{"tidy"})
+		}
+		pm, err := snapshot.ParseMod(ctx, fh)
+		if err != nil {
+			return err
+		}
+		edits, err := dropDependency(snapshot, pm, modulePath)
+		if err != nil {
+			return err
+		}
+		response, err := s.client.ApplyEdit(ctx, &protocol.ApplyWorkspaceEditParams{
+			Edit: protocol.WorkspaceEdit{
+				DocumentChanges: []protocol.TextDocumentEdit{{
+					TextDocument: protocol.VersionedTextDocumentIdentifier{
+						Version: fh.Version(),
+						TextDocumentIdentifier: protocol.TextDocumentIdentifier{
+							URI: protocol.URIFromSpanURI(fh.URI()),
+						},
+					},
+					Edits: edits,
+				}},
+			},
+		})
+		if err != nil {
+			return err
+		}
+		if !response.Applied {
+			return fmt.Errorf("edits not applied because of %s", response.FailureReason)
+		}
 	case source.CommandGoGetPackage:
 		var uri protocol.DocumentURI
 		var pkg string
@@ -303,6 +354,31 @@ func (s *Server) runCommand(ctx context.Context, work *workDone, command *source
 	return nil
 }
 
+// dropDependency returns the edits to remove the given require from the go.mod
+// file.
+func dropDependency(snapshot source.Snapshot, pm *source.ParsedModule, modulePath string) ([]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("", pm.Mapper.Content, nil)
+	if err != nil {
+		return nil, err
+	}
+	if err := copied.DropRequire(modulePath); err != nil {
+		return nil, err
+	}
+	copied.Cleanup()
+	newContent, err := copied.Format()
+	if err != nil {
+		return nil, err
+	}
+	// Calculate the edits to be made due to the change.
+	diff, err := snapshot.View().Options().ComputeEdits(pm.URI, string(pm.Mapper.Content), string(newContent))
+	if err != nil {
+		return nil, err
+	}
+	return source.ToProtocolEdits(pm.Mapper, diff)
+}
+
 func (s *Server) runTests(ctx context.Context, snapshot source.Snapshot, uri protocol.DocumentURI, work *workDone, tests, benchmarks []string) error {
 	pkgs, err := snapshot.PackagesForFile(ctx, uri.SpanURI(), source.TypecheckWorkspace)
 	if err != nil {