From ffec97847facd6cb97e4033a5756ef7f94da6e49 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 6 Oct 2020 21:08:57 -0400 Subject: [PATCH] internal/lsp: handle major versions above v0/v1 in workspace module mode Workspace mode did not yet support major versions other than v0/v1. To do so, we have to check the major version before creating the fake gopls workspace pseudoversion that goes in the workspace module. Fixes golang/go#41807 Change-Id: I108fe504fdf9e9a0ce23f7102991c9ae78f12a9f Reviewed-on: https://go-review.googlesource.com/c/tools/+/260004 Trust: Rebecca Stambler Run-TryBot: Rebecca Stambler gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Heschi Kreinick --- gopls/internal/regtest/workspace_test.go | 56 ++++++++++++++++++++++++ internal/lsp/cache/check.go | 2 +- internal/lsp/cache/snapshot.go | 10 ++++- internal/lsp/mod/diagnostics.go | 2 +- internal/lsp/source/view.go | 13 +++++- 5 files changed, 78 insertions(+), 5 deletions(-) diff --git a/gopls/internal/regtest/workspace_test.go b/gopls/internal/regtest/workspace_test.go index 35027e8075..4e9559dba6 100644 --- a/gopls/internal/regtest/workspace_test.go +++ b/gopls/internal/regtest/workspace_test.go @@ -455,3 +455,59 @@ var _ = fmt.Printf env.GoToDefinition("/tmp/foo.go", env.RegexpSearch("/tmp/foo.go", `Printf`)) }) } + +func TestMultiModuleV2(t *testing.T) { + const multiModule = ` +-- moda/a/go.mod -- +module a.com + +require b.com/v2 v2.0.0 +-- moda/a/a.go -- +package a + +import ( + "b.com/v2/b" +) + +func main() { + var x int + _ = b.Hi() +} +-- modb/go.mod -- +module b.com + +-- modb/b/b.go -- +package b + +func Hello() int { + var x int +} +-- modb/v2/go.mod -- +module b.com/v2 + +-- modb/v2/b/b.go -- +package b + +func Hi() int { + var x int +} +-- modc/go.mod -- +module gopkg.in/yaml.v1 // test gopkg.in versions +-- modc/main.go -- +package main + +func main() { + var x int +} +` + withOptions( + WithModes(Experimental), + ).run(t, multiModule, func(t *testing.T, env *Env) { + env.Await( + env.DiagnosticAtRegexp("moda/a/a.go", "x"), + env.DiagnosticAtRegexp("modb/b/b.go", "x"), + env.DiagnosticAtRegexp("modb/v2/b/b.go", "x"), + env.DiagnosticAtRegexp("modc/main.go", "x"), + ) + }) +} diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 688d52b143..709554c25f 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -278,7 +278,7 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source // meaningless, and we don't want clients to access it. if m.module != nil { version := m.module.Version - if version == source.WorkspaceModuleVersion { + if source.IsWorkspaceModuleVersion(version) { version = "" } pkg.version = &module.Version{ diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 5e11c528e0..5a542a50b0 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -20,6 +20,7 @@ import ( "sync" "golang.org/x/mod/modfile" + "golang.org/x/mod/module" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/event" @@ -1545,7 +1546,14 @@ func (s *snapshot) BuildWorkspaceModFile(ctx context.Context) (*modfile.File, er } path := parsed.File.Module.Mod.Path paths[path] = mod - file.AddNewRequire(path, source.WorkspaceModuleVersion, false) + // If the module's path includes a major version, we expect it to have + // a matching major version. + _, majorVersion, _ := module.SplitPathVersion(path) + if majorVersion == "" { + majorVersion = "/v0" + } + majorVersion = strings.TrimLeft(majorVersion, "/.") // handle gopkg.in versions + file.AddNewRequire(path, source.WorkspaceModuleVersion(majorVersion), false) if err := file.AddReplace(path, "", mod.rootURI.Filename(), ""); err != nil { return nil, err } diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index d4d3cad6f8..b834c7e717 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/internal/lsp/mod/diagnostics.go @@ -88,7 +88,7 @@ func ExtractGoCommandError(ctx context.Context, snapshot source.Snapshot, fh sou path, version := match[1], match[2] // Any module versions that come from the workspace module should not // be shown to the user. - if version == source.WorkspaceModuleVersion { + if source.IsWorkspaceModuleVersion(version) { continue } if err := module.Check(path, version); err != nil { diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index bbe95c4d94..b8c2153356 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -548,8 +548,17 @@ var ( PackagesLoadError = errors.New("packages.Load error") ) -// WorkspaceModuleVersion is the nonexistent pseudoversion used in the +// WorkspaceModuleVersion is the nonexistent pseudoversion suffix used in the // construction of the workspace module. It is exported so that we can make // sure not to show this version to end users in error messages, to avoid // confusion. -const WorkspaceModuleVersion = "v0.0.0-goplsworkspace" +// The major version is not included, as that depends on the module path. +const workspaceModuleVersion = ".0.0-goplsworkspace" + +func IsWorkspaceModuleVersion(version string) bool { + return strings.HasSuffix(version, workspaceModuleVersion) +} + +func WorkspaceModuleVersion(majorVersion string) string { + return majorVersion + workspaceModuleVersion +}