internal/lsp: use the go command to fix go.mod files

Modifying go.mod files directly leaves go.sum unchanged, and therefore
in need of updates later. Leaving work for the users to clean up isn't
ideal, so it'd be better to use the go command to make modifications.

Unfortunately, the go command has something of a mind of its own. The
most obvious problem is that using go get to add a new require adds a
// indirect comment to that new require, and there's no way to prevent
it. The only thing we can do is add the require first, then use go get
to do nothing but update the go.sum file.

The other inherent problem is that the go command operates on files as
they exist on disk, not the in-memory versions. As discussed, we issue
an error for this case. The alternative would be to work on temporary
files based on the in-memory contents, but that would be much larger
change, so I'd rather not at least right now.

To support Commands for quick fixes, add a new Command field to
source.SuggestedFix, and use it when forming the CodeAction response.

Fixes golang/go#38209.

Change-Id: I0c13ea39199368623e7494e222ba38587323d417
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265981
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Heschi Kreinick 2020-10-27 15:35:36 -04:00
parent 49729134c3
commit 2c115999a7
11 changed files with 91 additions and 101 deletions

View File

@ -41,12 +41,24 @@ undeclared_name adds a variable declaration for an undeclared
name.
### **Add dependency**
Identifier: `gopls.add_dependency`
add_dependency adds a dependency.
### **Upgrade dependency**
Identifier: `gopls.upgrade_dependency`
upgrade_dependency upgrades a dependency.
### **Remove dependency**
Identifier: `gopls.remove_dependency`
remove_dependency removes a dependency.
### **Run go mod vendor**
Identifier: `gopls.vendor`

View File

@ -109,25 +109,7 @@ func main() {
`
runner.Run(t, shouldUpdateDep, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
lenses := env.CodeLens("go.mod")
want := "Upgrade dependency to v1.3.3"
var found protocol.CodeLens
for _, lens := range lenses {
if lens.Command.Title == want {
found = lens
break
}
}
if found.Command.Command == "" {
t.Fatalf("did not find lens %q, got %v", want, lenses)
}
if _, err := env.Editor.ExecuteCommand(env.Ctx, &protocol.ExecuteCommandParams{
Command: found.Command.Command,
Arguments: found.Command.Arguments,
}); err != nil {
t.Fatal(err)
}
env.Await(NoOutstandingWork())
env.ExecuteCodeLensCommand("go.mod", source.CommandUpgradeDependency)
got := env.ReadWorkspaceFile("go.mod")
const wantGoMod = `module mod.com
@ -182,7 +164,6 @@ func main() {
runner.Run(t, shouldRemoveDep, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
env.ExecuteCodeLensCommand("go.mod", source.CommandTidy)
env.Await(NoOutstandingWork())
got := env.ReadWorkspaceFile("go.mod")
const wantGoMod = `module mod.com

View File

@ -716,7 +716,6 @@ go 1.12
WithProxyFiles(ardanLabsProxy),
).run(t, emptyFile, func(t *testing.T, env *Env) {
env.OpenFile("main.go")
env.OpenFile("go.mod")
env.EditBuffer("main.go", fake.NewEdit(0, 0, 0, 0, `package main
import "github.com/ardanlabs/conf"
@ -734,6 +733,7 @@ func main() {
),
)
env.ApplyQuickFixes("main.go", d.Diagnostics)
env.CheckForFileChanges()
env.Await(
EmptyDiagnostics("main.go"),
)

View File

@ -141,9 +141,8 @@ require random.org v1.2.3
randomDiag = diag
}
}
env.OpenFile("go.mod")
env.ApplyQuickFixes("main.go", []protocol.Diagnostic{randomDiag})
if got := env.Editor.BufferText("go.mod"); got != want {
if got := env.ReadWorkspaceFile("go.mod"); got != want {
t.Fatalf("unexpected go.mod content:\n%s", tests.Diff(want, got))
}
})
@ -223,7 +222,7 @@ go 1.14
),
)
env.ApplyQuickFixes("go.mod", d.Diagnostics)
if got := env.Editor.BufferText("go.mod"); got != want {
if got := env.ReadWorkspaceFile("go.mod"); got != want {
t.Fatalf("unexpected go.mod content:\n%s", tests.Diff(want, got))
}
})
@ -268,7 +267,6 @@ func _() {
caire.RemoveTempImage()
}`
runner.Run(t, repro, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
env.OpenFile("main.go")
var d protocol.PublishDiagnosticsParams
env.Await(
@ -287,7 +285,7 @@ require (
google.golang.org/protobuf v1.20.0
)
`
if got := env.Editor.BufferText("go.mod"); got != want {
if got := env.ReadWorkspaceFile("go.mod"); got != want {
t.Fatalf("TestNewDepWithUnusedDep failed:\n%s", tests.Diff(want, got))
}
}, WithProxyFiles(proxy))
@ -321,6 +319,8 @@ func main() {
}, WithProxyFiles(proxy))
}
// Tests golang/go#39784: a missing indirect dependency, necessary
// due to blah@v2.0.0's incomplete go.mod file.
func TestBadlyVersionedModule(t *testing.T) {
testenv.NeedsGo1Point(t, 14)
@ -369,6 +369,8 @@ func TestBlah(t *testing.T) {}
-- go.mod --
module mod.com
go 1.12
require (
example.com/blah/v2 v2.0.0
)
@ -394,12 +396,17 @@ func main() {
env.ApplyQuickFixes("main.go", d.Diagnostics)
const want = `module mod.com
go 1.12
require (
example.com/blah v1.0.0
example.com/blah/v2 v2.0.0
)
`
if got := env.Editor.BufferText("go.mod"); got != want {
// We need to read from disk even though go.mod is open -- the regtests
// currently don't apply on-disk changes to open but unmodified buffers
// like most editors would.
if got := env.ReadWorkspaceFile("go.mod"); got != want {
t.Fatalf("suggested fixes failed:\n%s", tests.Diff(want, got))
}
}, WithProxyFiles(badModule))

View File

@ -306,7 +306,7 @@ func unusedError(m *protocol.ColumnMapper, req *modfile.Require, computeEdits di
if err != nil {
return source.Error{}, err
}
edits, err := dropDependency(req, m, computeEdits)
args, err := source.MarshalArgs(m.URI, []string{req.Mod.Path + "@none"})
if err != nil {
return source.Error{}, err
}
@ -317,8 +317,10 @@ func unusedError(m *protocol.ColumnMapper, req *modfile.Require, computeEdits di
URI: m.URI,
SuggestedFixes: []source.SuggestedFix{{
Title: fmt.Sprintf("Remove dependency: %s", req.Mod.Path),
Edits: map[span.URI][]protocol.TextEdit{
m.URI: edits,
Command: &protocol.Command{
Title: source.CommandRemoveDependency.Title,
Command: source.CommandRemoveDependency.ID(),
Arguments: args,
},
}},
}, nil
@ -371,48 +373,27 @@ func missingModuleError(snapshot source.Snapshot, pm *source.ParsedModule, req *
if err != nil {
return source.Error{}, err
}
edits, err := addRequireFix(pm.Mapper, req, snapshot.View().Options().ComputeEdits)
args, err := source.MarshalArgs(pm.Mapper.URI, []string{req.Mod.Path + "@" + req.Mod.Version})
if err != nil {
return source.Error{}, err
}
fix := &source.SuggestedFix{
Title: fmt.Sprintf("Add %s to your go.mod file", req.Mod.Path),
Edits: map[span.URI][]protocol.TextEdit{
pm.Mapper.URI: edits,
},
}
return source.Error{
URI: pm.Mapper.URI,
Range: rng,
Message: fmt.Sprintf("%s is not in your go.mod file", req.Mod.Path),
Category: source.GoModTidy,
Kind: source.ModTidyError,
SuggestedFixes: []source.SuggestedFix{*fix},
URI: pm.Mapper.URI,
Range: rng,
Message: fmt.Sprintf("%s is not in your go.mod file", req.Mod.Path),
Category: source.GoModTidy,
Kind: source.ModTidyError,
SuggestedFixes: []source.SuggestedFix{{
Title: fmt.Sprintf("Add %s to your go.mod file", req.Mod.Path),
Command: &protocol.Command{
Title: source.CommandAddDependency.Title,
Command: source.CommandAddDependency.ID(),
Arguments: args,
},
}},
}, nil
}
// dropDependency returns the edits to remove the given require from the go.mod
// file.
func dropDependency(req *modfile.Require, m *protocol.ColumnMapper, computeEdits diff.ComputeEdits) ([]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("", m.Content, nil)
if err != nil {
return nil, err
}
if err := copied.DropRequire(req.Mod.Path); 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 := computeEdits(m.URI, string(m.Content), string(newContent))
return source.ToProtocolEdits(m, diff)
}
// switchDirectness gets the edits needed to change an indirect dependency to
// direct and vice versa.
func switchDirectness(req *modfile.Require, m *protocol.ColumnMapper, computeEdits diff.ComputeEdits) ([]protocol.TextEdit, error) {
@ -470,28 +451,6 @@ func missingModuleForImport(snapshot source.Snapshot, m *protocol.ColumnMapper,
}, nil
}
// addRequireFix creates edits for adding a given require to a go.mod file.
func addRequireFix(m *protocol.ColumnMapper, req *modfile.Require, computeEdits diff.ComputeEdits) ([]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("", m.Content, nil)
if err != nil {
return nil, err
}
// Calculate the quick fix edits that need to be made to the go.mod file.
if err := copied.AddRequire(req.Mod.Path, req.Mod.Version); err != nil {
return nil, err
}
copied.SortBlocks()
newContents, err := copied.Format()
if err != nil {
return nil, err
}
// Calculate the edits to be made due to the change.
diff := computeEdits(m.URI, string(m.Content), string(newContents))
return source.ToProtocolEdits(m, diff)
}
func rangeFromPositions(m *protocol.ColumnMapper, s, e modfile.Position) (protocol.Range, error) {
toPoint := func(offset int) (span.Point, error) {
l, c, err := m.Converter.ToPosition(offset)

View File

@ -508,6 +508,7 @@ func moduleQuickFixes(ctx context.Context, snapshot source.Snapshot, fh source.V
Kind: protocol.QuickFix,
Diagnostics: []protocol.Diagnostic{*diag},
Edit: protocol.WorkspaceEdit{},
Command: fix.Command,
}
for uri, edits := range fix.Edits {
if uri != modFH.URI() {

View File

@ -54,9 +54,10 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
}
if unsaved {
switch params.Command {
case source.CommandTest.ID(), source.CommandGenerate.ID(), source.CommandToggleDetails.ID():
case source.CommandTest.ID(), source.CommandGenerate.ID(), source.CommandToggleDetails.ID(),
source.CommandAddDependency.ID(), source.CommandUpgradeDependency.ID(), source.CommandRemoveDependency.ID():
// TODO(PJW): for Toggle, not an error if it is being disabled
err := errors.New("unsaved files in the view")
err := errors.New("all files must be saved first")
s.showCommandError(ctx, command.Title, err)
return nil, err
}
@ -189,14 +190,23 @@ func (s *Server) runCommand(ctx context.Context, work *workDone, command *source
if command == source.CommandVendor {
a = "vendor"
}
return s.directGoModCommand(ctx, uri, "mod", []string{a}...)
case source.CommandUpgradeDependency:
return s.directGoModCommand(ctx, uri, "mod", a)
case source.CommandAddDependency, source.CommandUpgradeDependency, source.CommandRemoveDependency:
var uri protocol.DocumentURI
var goCmdArgs []string
if err := source.UnmarshalArgs(args, &uri, &goCmdArgs); err != nil {
return err
}
return s.directGoModCommand(ctx, uri, "get", goCmdArgs...)
if command == source.CommandAddDependency {
// Using go get to create a new dependency results in an
// `// indirect` comment we don't want. The only way to avoid it is
// to add the require as direct first. Then we can use go get to
// update go.sum and tidy up.
if err := s.directGoModCommand(ctx, uri, "mod", append([]string{"edit", "-require"}, goCmdArgs...)...); err != nil {
return err
}
}
return s.directGoModCommand(ctx, uri, "get", append([]string{"-d"}, goCmdArgs...)...)
case source.CommandToggleDetails:
var fileURI span.URI
if err := source.UnmarshalArgs(args, &fileURI); err != nil {

File diff suppressed because one or more lines are too long

View File

@ -63,7 +63,9 @@ var Commands = []*Command{
CommandTest,
CommandTidy,
CommandUndeclaredName,
CommandAddDependency,
CommandUpgradeDependency,
CommandRemoveDependency,
CommandVendor,
CommandExtractVariable,
CommandExtractFunction,
@ -97,12 +99,24 @@ var (
Title: "Run go mod vendor",
}
// CommandAddDependency adds a dependency.
CommandAddDependency = &Command{
Name: "add_dependency",
Title: "Add dependency",
}
// CommandUpgradeDependency upgrades a dependency.
CommandUpgradeDependency = &Command{
Name: "upgrade_dependency",
Title: "Upgrade dependency",
}
// CommandRemoveDependency removes a dependency.
CommandRemoveDependency = &Command{
Name: "remove_dependency",
Title: "Remove dependency",
}
// CommandRegenerateCgo regenerates cgo definitions.
CommandRegenerateCgo = &Command{
Name: "regenerate_cgo",

View File

@ -28,8 +28,9 @@ type Diagnostic struct {
}
type SuggestedFix struct {
Title string
Edits map[span.URI][]protocol.TextEdit
Title string
Edits map[span.URI][]protocol.TextEdit
Command *protocol.Command
}
type RelatedInformation struct {

View File

@ -546,14 +546,19 @@ func (err *ErrorList) Error() string {
return b.String()
}
// An Error corresponds to an LSP Diagnostic.
// https://microsoft.github.io/language-server-protocol/specification#diagnostic
type Error struct {
URI span.URI
Range protocol.Range
Kind ErrorKind
Message string
Category string // only used by analysis errors so far
URI span.URI
Range protocol.Range
Kind ErrorKind
Message string
Category string // only used by analysis errors so far
Related []RelatedInformation
// SuggestedFixes is used to generate quick fixes for a CodeAction request.
// It isn't part of the Diagnostic type.
SuggestedFixes []SuggestedFix
Related []RelatedInformation
}
// GoModTidy is the source for a diagnostic computed by running `go mod tidy`.