mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp: add new go.mod requires to files with unused require 

This change addresses an underlying issue with the go.mod code, which is
that it was modifying go.mod files without cloning them. This could've
resulted in some ugly race conditions.

We also handle the fact that new dependencies weren't being added
cleanly to files that already had unused dependencies.

Fixes golang/go#39041

Change-Id: I96ee0052d8d29a25e24f0bda9688e780a0fa7442
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241443
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Rebecca Stambler 2020-07-09 02:02:35 -04:00
parent 55a0fde516
commit 125cc70a40
3 changed files with 120 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
internal/lsp/cache/mod_tidy.go vendored
View File

@ -150,7 +150,7 @@ func (s *snapshot) ModTidyHandle(ctx context.Context) (source.ModTidyHandle, err
missingDeps[req.Mod.Path] = req
}
}
diagnostics, err := modRequireErrors(pmh.Mod().URI(), original, m, missingDeps, unusedDeps, options)
diagnostics, err := modRequireErrors(pmh.Mod().URI(), m, missingDeps, unusedDeps, options)
if err != nil {
return &modTidyData{err: err}
}
@ -175,7 +175,7 @@ func (s *snapshot) ModTidyHandle(ctx context.Context) (source.ModTidyHandle, err
// modRequireErrors extracts the errors that occur on the require directives.
// It checks for directness issues and unused dependencies.
func modRequireErrors(uri span.URI, parsed *modfile.File, m *protocol.ColumnMapper, missingDeps, unusedDeps map[string]*modfile.Require, options source.Options) ([]source.Error, error) {
func modRequireErrors(uri span.URI, m *protocol.ColumnMapper, missingDeps, unusedDeps map[string]*modfile.Require, options source.Options) ([]source.Error, error) {
var errors []source.Error
for dep, req := range unusedDeps {
if req.Syntax == nil {
@ -183,7 +183,7 @@ func modRequireErrors(uri span.URI, parsed *modfile.File, m *protocol.ColumnMapp
}
// Handle dependencies that are incorrectly labeled indirect and vice versa.
if missingDeps[dep] != nil && req.Indirect != missingDeps[dep].Indirect {
directErr, err := modDirectnessErrors(uri, parsed, m, req, options)
directErr, err := modDirectnessErrors(uri, m, req, options)
if err != nil {
return nil, err
}
@ -195,7 +195,7 @@ func modRequireErrors(uri span.URI, parsed *modfile.File, m *protocol.ColumnMapp
if err != nil {
return nil, err
}
edits, err := dropDependencyEdits(uri, parsed, m, req, options)
edits, err := dropDependencyEdits(uri, m, req, options)
if err != nil {
return nil, err
}
@ -219,7 +219,7 @@ func modRequireErrors(uri span.URI, parsed *modfile.File, m *protocol.ColumnMapp
const ModTidyError = "go mod tidy"
// modDirectnessErrors extracts errors when a dependency is labeled indirect when it should be direct and vice versa.
func modDirectnessErrors(uri span.URI, parsed *modfile.File, m *protocol.ColumnMapper, req *modfile.Require, options source.Options) (source.Error, error) {
func modDirectnessErrors(uri span.URI, m *protocol.ColumnMapper, req *modfile.Require, options source.Options) (source.Error, error) {
rng, err := rangeFromPositions(uri, m, req.Syntax.Start, req.Syntax.End)
if err != nil {
return source.Error{}, err
@ -235,7 +235,7 @@ func modDirectnessErrors(uri span.URI, parsed *modfile.File, m *protocol.ColumnM
return source.Error{}, err
}
}
edits, err := changeDirectnessEdits(uri, parsed, m, req, false, options)
edits, err := changeDirectnessEdits(uri, m, req, false, options)
if err != nil {
return source.Error{}, err
}
@ -253,7 +253,7 @@ func modDirectnessErrors(uri span.URI, parsed *modfile.File, m *protocol.ColumnM
}, nil
}
// If the dependency should be indirect, add the // indirect.
edits, err := changeDirectnessEdits(uri, parsed, m, req, true, options)
edits, err := changeDirectnessEdits(uri, m, req, true, options)
if err != nil {
return source.Error{}, err
}
@ -283,19 +283,23 @@ func modDirectnessErrors(uri span.URI, parsed *modfile.File, m *protocol.ColumnM
// module t
//
// go 1.11
func dropDependencyEdits(uri span.URI, parsed *modfile.File, m *protocol.ColumnMapper, req *modfile.Require, options source.Options) ([]protocol.TextEdit, error) {
if err := parsed.DropRequire(req.Mod.Path); err != nil {
return nil, err
}
parsed.Cleanup()
newContents, err := parsed.Format()
func dropDependencyEdits(uri span.URI, m *protocol.ColumnMapper, req *modfile.Require, options source.Options) ([]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
}
// Reset the *modfile.File back to before we dropped the dependency.
parsed.AddNewRequire(req.Mod.Path, req.Mod.Version, req.Indirect)
// Calculate the edits to be made due to the change.
diff := options.ComputeEdits(uri, string(m.Content), string(newContents))
diff := options.ComputeEdits(uri, string(m.Content), string(newContent))
edits, err := source.ToProtocolEdits(m, diff)
if err != nil {
return nil, err
@ -317,33 +321,34 @@ func dropDependencyEdits(uri span.URI, parsed *modfile.File, m *protocol.ColumnM
// go 1.11
//
// require golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee // indirect
func changeDirectnessEdits(uri span.URI, parsed *modfile.File, m *protocol.ColumnMapper, req *modfile.Require, indirect bool, options source.Options) ([]protocol.TextEdit, error) {
var newReq []*modfile.Require
prevIndirect := false
// Change the directness in the matching require statement.
for _, r := range parsed.Require {
if req.Mod.Path == r.Mod.Path {
prevIndirect = req.Indirect
req.Indirect = indirect
}
newReq = append(newReq, r)
}
parsed.SetRequire(newReq)
parsed.Cleanup()
newContents, err := parsed.Format()
func changeDirectnessEdits(uri span.URI, m *protocol.ColumnMapper, req *modfile.Require, indirect bool, options source.Options) ([]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
}
// Change the dependency back to the way it was before we got the newContents.
for _, r := range parsed.Require {
if req.Mod.Path == r.Mod.Path {
req.Indirect = prevIndirect
// Change the directness in the matching require statement. To avoid
// reordering the require statements, rewrite all of them.
var requires []*modfile.Require
for _, r := range copied.Require {
if r.Mod.Path == req.Mod.Path {
requires = append(requires, &modfile.Require{
Mod: r.Mod,
Syntax: r.Syntax,
Indirect: indirect,
})
continue
}
newReq = append(newReq, r)
requires = append(requires, r)
}
copied.SetRequire(requires)
newContent, err := copied.Format()
if err != nil {
return nil, err
}
parsed.SetRequire(newReq)
// Calculate the edits to be made due to the change.
diff := options.ComputeEdits(uri, string(m.Content), string(newContents))
diff := options.ComputeEdits(uri, string(m.Content), string(newContent))
edits, err := source.ToProtocolEdits(m, diff)
if err != nil {
return nil, err

20
internal/lsp/mod/diagnostics.go
View File

@ -143,7 +143,7 @@ func SuggestedGoFixes(ctx context.Context, snapshot source.Snapshot) (map[string
if err != nil {
return nil, err
}
file, m, _, err := pmh.Parse(ctx)
_, m, _, err := pmh.Parse(ctx)
if err != nil {
return nil, err
}
@ -154,17 +154,19 @@ func SuggestedGoFixes(ctx context.Context, snapshot source.Snapshot) (map[string
}
textDocumentEdits := make(map[string]protocol.TextDocumentEdit)
for dep, req := range missingDeps {
// Calculate the quick fix edits that need to be made to the go.mod file.
if err := file.AddRequire(req.Mod.Path, req.Mod.Version); err != nil {
return nil, err
}
file.Cleanup()
newContents, err := file.Format()
// We need a private copy of the parsed go.mod file, since we're going to
// modify it.
copied, err := modfile.Parse("", oldContents, nil)
if err != nil {
return nil, err
}
// Reset the *modfile.File back to before we added the dependency.
if err := file.DropRequire(req.Mod.Path); err != nil {
// 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.

67
internal/lsp/regtest/modfile_test.go
View File

@ -103,3 +103,70 @@ require example.com v1.2.3
}
}, WithProxy(proxy))
}
// Test to reproduce golang/go#39041. It adds a new require to a go.mod file
// that already has an unused require.
func TestNewDepWithUnusedDep(t *testing.T) {
testenv.NeedsGo1Point(t, 14)
const proxy = `
-- github.com/esimov/caire@v1.2.5/go.mod --
module github.com/esimov/caire
go 1.12
-- github.com/esimov/caire@v1.2.5/caire.go --
package caire
func RemoveTempImage() {}
-- google.golang.org/protobuf@v1.20.0/go.mod --
module google.golang.org/protobuf
go 1.12
-- google.golang.org/protobuf@v1.20.0/hello/hello.go --
package hello
`
const repro = `
-- go.mod --
module mod.com
go 1.14
require google.golang.org/protobuf v1.20.0
-- main.go --
package main
import (
"github.com/esimov/caire"
)
func _() {
caire.RemoveTempImage()
}`
runner.Run(t, repro, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
env.OpenFile("main.go")
d := env.Await(
env.DiagnosticAtRegexp("main.go", `"github.com/esimov/caire"`),
)
if len(d) == 0 {
t.Fatalf("no diagnostics")
}
params, ok := d[0].(*protocol.PublishDiagnosticsParams)
if !ok {
t.Fatalf("expected diagnostic of type PublishDiagnosticParams, got %T", d[0])
}
env.ApplyQuickFixes("main.go", params.Diagnostics)
want := `module mod.com
go 1.14
require (
github.com/esimov/caire v1.2.5
google.golang.org/protobuf v1.20.0
)
`
if got := env.Editor.BufferText("go.mod"); got != want {
t.Fatalf("TestNewDepWithUnusedDep failed:\n%s", tests.Diff(want, got))
}
}, WithProxy(proxy))
}