internal/lsp: set correct directness when adding new requires
When adding a require, we should add an `// indirect` comment if that's
what go mod tidy would do.
It's possible I should split Add out from Update and Remove, but this
was quick and easy and I'm not too worried about it for now.
Also minimize the test that covered this case, which was way more
complicated than it needed to be AFAICT.
Fixes golang/go#38914.
Change-Id: I89c44f8573873227c4c9e637d1d31d8c1a6530aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267578
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>
diff --git a/gopls/internal/regtest/modfile_test.go b/gopls/internal/regtest/modfile_test.go
index a69c260..b1bc508 100644
--- a/gopls/internal/regtest/modfile_test.go
+++ b/gopls/internal/regtest/modfile_test.go
@@ -324,9 +324,7 @@
func TestBadlyVersionedModule(t *testing.T) {
testenv.NeedsGo1Point(t, 14)
- const badModule = `
--- example.com/blah/@v/list --
-v1.0.0
+ const proxy = `
-- example.com/blah/@v/v1.0.0.mod --
module example.com
@@ -335,17 +333,6 @@
package blah
const Name = "Blah"
--- example.com/blah@v1.0.0/blah_test.go --
-package blah_test
-
-import (
- "testing"
-)
-
-func TestBlah(t *testing.T) {}
-
--- example.com/blah/v2/@v/list --
-v2.0.0
-- example.com/blah/v2/@v/v2.0.0.mod --
module example.com
@@ -353,37 +340,26 @@
-- example.com/blah/v2@v2.0.0/blah.go --
package blah
+import "example.com/blah"
+
+var _ = blah.Name
const Name = "Blah"
--- example.com/blah/v2@v2.0.0/blah_test.go --
-package blah_test
-
-import (
- "testing"
-
- "example.com/blah"
-)
-
-func TestBlah(t *testing.T) {}
`
- const pkg = `
+ const files = `
-- go.mod --
module mod.com
go 1.12
-require (
- example.com/blah/v2 v2.0.0
-)
+require example.com/blah/v2 v2.0.0
-- main.go --
package main
import "example.com/blah/v2"
-func main() {
- println(blah.Name)
-}
+var _ = blah.Name
`
- runner.Run(t, pkg, func(t *testing.T, env *Env) {
+ withOptions(WithProxyFiles(proxy)).run(t, files, func(t *testing.T, env *Env) {
env.OpenFile("main.go")
env.OpenFile("go.mod")
var d protocol.PublishDiagnosticsParams
@@ -399,7 +375,7 @@
go 1.12
require (
- example.com/blah v1.0.0
+ example.com/blah v1.0.0 // indirect
example.com/blah/v2 v2.0.0
)
`
@@ -409,7 +385,7 @@
if got := env.ReadWorkspaceFile("go.mod"); got != want {
t.Fatalf("suggested fixes failed:\n%s", tests.Diff(want, got))
}
- }, WithProxyFiles(badModule))
+ })
}
// Reproduces golang/go#38232.
diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go
index cfe299b..76479ee 100644
--- a/internal/lsp/cache/mod_tidy.go
+++ b/internal/lsp/cache/mod_tidy.go
@@ -353,7 +353,7 @@
if err != nil {
return source.Error{}, err
}
- args, err := source.MarshalArgs(m.URI, []string{req.Mod.Path + "@none"})
+ args, err := source.MarshalArgs(m.URI, false, []string{req.Mod.Path + "@none"})
if err != nil {
return source.Error{}, err
}
@@ -425,7 +425,7 @@
return source.Error{}, err
}
}
- args, err := source.MarshalArgs(pm.Mapper.URI, []string{req.Mod.Path + "@" + req.Mod.Version})
+ args, err := source.MarshalArgs(pm.Mapper.URI, !req.Indirect, []string{req.Mod.Path + "@" + req.Mod.Version})
if err != nil {
return source.Error{}, err
}
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index ce7b1cd..a622329 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -200,13 +200,14 @@
case source.CommandAddDependency, source.CommandUpgradeDependency, source.CommandRemoveDependency:
var uri protocol.DocumentURI
var goCmdArgs []string
- if err := source.UnmarshalArgs(args, &uri, &goCmdArgs); err != nil {
+ var addRequire bool
+ if err := source.UnmarshalArgs(args, &uri, &addRequire, &goCmdArgs); err != nil {
return err
}
- if command == source.CommandAddDependency {
+ if addRequire {
// 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
+ // `// indirect` comment we may not 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
diff --git a/internal/lsp/mod/code_lens.go b/internal/lsp/mod/code_lens.go
index 3b3533d..dc2e414 100644
--- a/internal/lsp/mod/code_lens.go
+++ b/internal/lsp/mod/code_lens.go
@@ -52,7 +52,7 @@
if err != nil {
return nil, err
}
- upgradeDepArgs, err := source.MarshalArgs(fh.URI(), []string{dep})
+ upgradeDepArgs, err := source.MarshalArgs(fh.URI(), false, []string{dep})
if err != nil {
return nil, err
}
@@ -69,7 +69,7 @@
// If there is at least 1 upgrade, add "Upgrade all dependencies" to
// the module statement.
if len(allUpgrades) > 0 {
- upgradeDepArgs, err := source.MarshalArgs(fh.URI(), append([]string{"-u"}, allUpgrades...))
+ upgradeDepArgs, err := source.MarshalArgs(fh.URI(), false, append([]string{"-u"}, allUpgrades...))
if err != nil {
return nil, err
}
