From f4cefd1cb5babc88ec8fae0d5548b0d63ab7c6a8 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 11 Sep 2020 00:41:36 -0400 Subject: [PATCH] internal/lsp: enable multi-module workspace mode by default in tests This change enables the multi-module workspace mode by default, so that we can catch all of the test failures and edge cases. It is still disabled in GOPATH mode and for any workspaces that contain a module with a vendor directory. A few minor changes had to be made to handle changes caused by the workspace module pseudoversions. Updates golang/go#32394 Change-Id: Ib433b269dfc435d73365677945057c1c2cbb1869 Reviewed-on: https://go-review.googlesource.com/c/tools/+/254317 Trust: Rebecca Stambler Run-TryBot: Rebecca Stambler TryBot-Result: Go Bot gopls-CI: kokoro Reviewed-by: Heschi Kreinick --- gopls/internal/regtest/codelens_test.go | 11 +++-- gopls/internal/regtest/diagnostics_test.go | 16 +++++-- gopls/internal/regtest/env.go | 8 +++- gopls/internal/regtest/watch_test.go | 32 +++++++++++-- gopls/internal/regtest/workspace_test.go | 2 - gopls/test/gopls_test.go | 2 + internal/lsp/cache/check.go | 14 +++++- internal/lsp/cache/mod.go | 4 +- internal/lsp/cache/pkg.go | 8 ++-- internal/lsp/cache/session.go | 53 +++++++++++++++++----- internal/lsp/cache/snapshot.go | 13 ++++-- internal/lsp/cache/view.go | 5 +- internal/lsp/cmd/cmd_test.go | 2 +- internal/lsp/fake/editor.go | 9 +--- internal/lsp/link.go | 4 +- internal/lsp/lsp_test.go | 3 +- internal/lsp/mod/diagnostics.go | 14 ++++-- internal/lsp/mod/mod_test.go | 4 +- internal/lsp/source/hover.go | 4 +- internal/lsp/source/source_test.go | 3 +- internal/lsp/source/view.go | 10 +++- internal/lsp/tests/tests.go | 5 +- 22 files changed, 159 insertions(+), 67 deletions(-) diff --git a/gopls/internal/regtest/codelens_test.go b/gopls/internal/regtest/codelens_test.go index e4c90bde9b..8402e9b84c 100644 --- a/gopls/internal/regtest/codelens_test.go +++ b/gopls/internal/regtest/codelens_test.go @@ -68,7 +68,7 @@ func TestUpdateCodelens(t *testing.T) { -- golang.org/x/hello@v1.3.3/go.mod -- module golang.org/x/hello -go 1.14 +go 1.12 -- golang.org/x/hello@v1.3.3/hi/hi.go -- package hi @@ -76,7 +76,7 @@ var Goodbye error -- golang.org/x/hello@v1.2.3/go.mod -- module golang.org/x/hello -go 1.14 +go 1.12 -- golang.org/x/hello@v1.2.3/hi/hi.go -- package hi @@ -87,9 +87,12 @@ var Goodbye error -- go.mod -- module mod.com -go 1.14 +go 1.12 require golang.org/x/hello v1.2.3 +-- go.sum -- +golang.org/x/hello v1.2.3 h1:jOtNXLsiCuLzU6KM3wRHidpc29IxcKpofHZiOW1hYKA= +golang.org/x/hello v1.2.3/go.mod h1:X79D30QqR94cGK8aIhQNhCZLq4mIr5Gimj5qekF08rY= -- main.go -- package main @@ -123,7 +126,7 @@ func main() { got := env.ReadWorkspaceFile("go.mod") const wantGoMod = `module mod.com -go 1.14 +go 1.12 require golang.org/x/hello v1.3.3 ` diff --git a/gopls/internal/regtest/diagnostics_test.go b/gopls/internal/regtest/diagnostics_test.go index fac3f712b3..50ae251fba 100644 --- a/gopls/internal/regtest/diagnostics_test.go +++ b/gopls/internal/regtest/diagnostics_test.go @@ -408,7 +408,7 @@ func TestResolveDiagnosticWithDownload(t *testing.T) { func TestMissingDependency(t *testing.T) { runner.Run(t, testPackageWithRequire, func(t *testing.T, env *Env) { env.OpenFile("print.go") - env.Await(LogMatching(protocol.Error, "initial workspace load failed")) + env.Await(LogMatching(protocol.Error, "initial workspace load failed", 1)) }) } @@ -1278,7 +1278,7 @@ func main() { // Test some secondary diagnostics func TestSecondaryDiagnostics(t *testing.T) { const dir = ` --- mod -- +-- go.mod -- module mod.com -- main.go -- package main @@ -1295,16 +1295,19 @@ func main() {} env.OpenFile("other.go") env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromInitialWorkspaceLoad), 1)) x := env.DiagnosticsFor("main.go") + if x == nil { + t.Fatalf("expected 1 diagnostic, got none") + } if len(x.Diagnostics) != 1 { - t.Errorf("main.go, got %d diagnostics, expected 1", len(x.Diagnostics)) + t.Fatalf("main.go, got %d diagnostics, expected 1", len(x.Diagnostics)) } keep := x.Diagnostics[0] y := env.DiagnosticsFor("other.go") if len(y.Diagnostics) != 1 { - t.Errorf("other.go: got %d diagnostics, expected 1", len(y.Diagnostics)) + t.Fatalf("other.go: got %d diagnostics, expected 1", len(y.Diagnostics)) } if len(y.Diagnostics[0].RelatedInformation) != 1 { - t.Errorf("got %d RelatedInformations, expected 1", len(y.Diagnostics[0].RelatedInformation)) + t.Fatalf("got %d RelatedInformations, expected 1", len(y.Diagnostics[0].RelatedInformation)) } // check that the RelatedInformation matches the error from main.go c := y.Diagnostics[0].RelatedInformation[0] @@ -1315,6 +1318,9 @@ func main() {} } func TestNotifyOrphanedFiles(t *testing.T) { + // Need GO111MODULE=on for this test to work with Go 1.12. + testenv.NeedsGo1Point(t, 13) + const files = ` -- go.mod -- module mod.com diff --git a/gopls/internal/regtest/env.go b/gopls/internal/regtest/env.go index 6bcfd5c4e6..356fbb521b 100644 --- a/gopls/internal/regtest/env.go +++ b/gopls/internal/regtest/env.go @@ -471,17 +471,21 @@ func NoErrorLogs() LogExpectation { // LogMatching asserts that the client has received a log message // of type typ matching the regexp re. -func LogMatching(typ protocol.MessageType, re string) LogExpectation { +func LogMatching(typ protocol.MessageType, re string, count int) LogExpectation { rec, err := regexp.Compile(re) if err != nil { panic(err) } check := func(msgs []*protocol.LogMessageParams) (Verdict, interface{}) { + var found int for _, msg := range msgs { if msg.Type == typ && rec.Match([]byte(msg.Message)) { - return Met, msg + found++ } } + if found == count { + return Met, nil + } return Unmet, nil } return LogExpectation{ diff --git a/gopls/internal/regtest/watch_test.go b/gopls/internal/regtest/watch_test.go index f42006097a..6e6919bcd0 100644 --- a/gopls/internal/regtest/watch_test.go +++ b/gopls/internal/regtest/watch_test.go @@ -386,7 +386,12 @@ package a runner.Run(t, pkg, func(t *testing.T, env *Env) { env.OpenFile("a/a.go") env.OpenFile("a/a_unneeded.go") - env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 2)) + env.Await( + OnceMet( + CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 2), + LogMatching(protocol.Info, "a_unneeded.go", 1), + ), + ) // Close and delete the open file, mimicking what an editor would do. env.CloseBuffer("a/a_unneeded.go") @@ -397,7 +402,13 @@ package a ) env.SaveBuffer("a/a.go") env.Await( - NoLogMatching(protocol.Info, "a_unneeded.go"), + OnceMet( + CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidSave), 1), + // There should only be one log message containing + // a_unneeded.go, from the initial workspace load, which we + // check for earlier. If there are more, there's a bug. + LogMatching(protocol.Info, "a_unneeded.go", 1), + ), EmptyDiagnostics("a/a.go"), ) }) @@ -407,18 +418,29 @@ package a runner.Run(t, pkg, func(t *testing.T, env *Env) { env.OpenFile("a/a.go") env.OpenFile("a/a_unneeded.go") - env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 2)) + env.Await( + OnceMet( + CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 2), + LogMatching(protocol.Info, "a_unneeded.go", 1), + ), + ) // Delete and then close the file. - env.CloseBuffer("a/a_unneeded.go") env.RemoveWorkspaceFile("a/a_unneeded.go") + env.CloseBuffer("a/a_unneeded.go") env.RegexpReplace("a/a.go", "var _ int", "fmt.Println(\"\")") env.Await( env.DiagnosticAtRegexp("a/a.go", "fmt"), ) env.SaveBuffer("a/a.go") env.Await( - NoLogMatching(protocol.Info, "a_unneeded.go"), + OnceMet( + CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidSave), 1), + // There should only be one log message containing + // a_unneeded.go, from the initial workspace load, which we + // check for earlier. If there are more, there's a bug. + LogMatching(protocol.Info, "a_unneeded.go", 1), + ), EmptyDiagnostics("a/a.go"), ) }) diff --git a/gopls/internal/regtest/workspace_test.go b/gopls/internal/regtest/workspace_test.go index 7b997aee0a..63bcdea40c 100644 --- a/gopls/internal/regtest/workspace_test.go +++ b/gopls/internal/regtest/workspace_test.go @@ -9,7 +9,6 @@ import ( "testing" "golang.org/x/tools/internal/lsp" - "golang.org/x/tools/internal/lsp/fake" ) const workspaceProxy = ` @@ -199,7 +198,6 @@ func Hello() int { ` withOptions( WithProxyFiles(workspaceModuleProxy), - WithEditorConfig(fake.EditorConfig{ExperimentalWorkspaceModule: true}), ).run(t, multiModule, func(t *testing.T, env *Env) { env.Await( CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromInitialWorkspaceLoad), 1), diff --git a/gopls/test/gopls_test.go b/gopls/test/gopls_test.go index 5f43202ace..f4754d4af9 100644 --- a/gopls/test/gopls_test.go +++ b/gopls/test/gopls_test.go @@ -12,6 +12,7 @@ import ( "golang.org/x/tools/gopls/internal/hooks" cmdtest "golang.org/x/tools/internal/lsp/cmd/test" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/lsp/tests" "golang.org/x/tools/internal/testenv" ) @@ -32,5 +33,6 @@ func TestCommandLine(t *testing.T) { func commandLineOptions(options *source.Options) { options.Staticcheck = true options.GoDiff = false + tests.DefaultOptions(options) hooks.Options(options) } diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 8311416c82..48b09124ba 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -15,6 +15,7 @@ import ( "strings" "sync" + "golang.org/x/mod/module" "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/lsp/debug/tag" @@ -256,7 +257,6 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source mode: mode, goFiles: make([]*source.ParsedGoFile, len(m.goFiles)), compiledGoFiles: make([]*source.ParsedGoFile, len(m.compiledGoFiles)), - module: m.module, imports: make(map[packagePath]*pkg), typesSizes: m.typesSizes, typesInfo: &types.Info{ @@ -268,6 +268,18 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source Scopes: make(map[ast.Node]*types.Scope), }, } + // If this is a replaced module in the workspace, the version is + // meaningless, and we don't want clients to access it. + if m.module != nil { + version := m.module.Version + if version == source.WorkspaceModuleVersion { + version = "" + } + pkg.version = &module.Version{ + Path: m.module.Path, + Version: version, + } + } var ( files = make([]*ast.File, len(m.compiledGoFiles)) parseErrors = make([]error, len(m.compiledGoFiles)) diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go index c689e2cd21..d21257041f 100644 --- a/internal/lsp/cache/mod.go +++ b/internal/lsp/cache/mod.go @@ -233,7 +233,7 @@ func (s *snapshot) ModWhy(ctx context.Context, fh source.FileHandle) (map[string for _, req := range pm.File.Require { args = append(args, req.Mod.Path) } - stdout, err := snapshot.RunGoCommand(ctx, "mod", args) + stdout, err := snapshot.runGoCommandWithConfig(ctx, cfg, "mod", args) if err != nil { return &modWhyData{err: err} } @@ -329,7 +329,7 @@ func (s *snapshot) ModUpgrade(ctx context.Context, fh source.FileHandle) (map[st // (see golang/go#38711). args = append([]string{"-mod", "readonly"}, args...) } - stdout, err := snapshot.RunGoCommand(ctx, "list", args) + stdout, err := snapshot.runGoCommandWithConfig(ctx, cfg, "list", args) if err != nil { return &modUpgradeData{err: err} } diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go index b29eb27b33..faa0b29e75 100644 --- a/internal/lsp/cache/pkg.go +++ b/internal/lsp/cache/pkg.go @@ -8,7 +8,7 @@ import ( "go/ast" "go/types" - "golang.org/x/tools/go/packages" + "golang.org/x/mod/module" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" errors "golang.org/x/xerrors" @@ -22,7 +22,7 @@ type pkg struct { compiledGoFiles []*source.ParsedGoFile errors []*source.Error imports map[packagePath]*pkg - module *packages.Module + version *module.Version typeErrors []types.Error types *types.Package typesInfo *types.Info @@ -133,6 +133,6 @@ func (p *pkg) Imports() []source.Package { return result } -func (p *pkg) Module() *packages.Module { - return p.module +func (p *pkg) Version() *module.Version { + return p.version } diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index d2c2611da3..141fc27d70 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -173,7 +173,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, name: name, folder: folder, root: folder, - modules: make(map[span.URI]*module), + modules: make(map[span.URI]*moduleRoot), filesByURI: make(map[span.URI]*fileBase), filesByBase: make(map[string][]*fileBase), } @@ -206,7 +206,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, } // Find all of the modules in the workspace. - if err := v.findAndBuildWorkspaceModule(ctx, options); err != nil { + if err := v.findWorkspaceModules(ctx, options); err != nil { return nil, nil, func() {}, err } @@ -214,6 +214,13 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, // check if the view has a valid build configuration. v.setBuildConfiguration() + // Build the workspace module, if needed. + if options.ExperimentalWorkspaceModule { + if err := v.buildWorkspaceModule(ctx); err != nil { + return nil, nil, func() {}, err + } + } + // We have v.goEnv now. v.processEnv = &imports.ProcessEnv{ GocmdRunner: s.gocmdRunner, @@ -240,13 +247,13 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, return v, v.snapshot, v.snapshot.generation.Acquire(ctx), nil } -// findAndBuildWorkspaceModule walks the view's root folder, looking for go.mod -// files. Any that are found are added to the view's set of modules, which are -// then used to construct the workspace module. +// findWorkspaceModules walks the view's root folder, looking for go.mod files. +// Any that are found are added to the view's set of modules, which are then +// used to construct the workspace module. // // It assumes that the caller has not yet created the view, and therefore does // not lock any of the internal data structures before accessing them. -func (v *View) findAndBuildWorkspaceModule(ctx context.Context, options source.Options) error { +func (v *View) findWorkspaceModules(ctx context.Context, options source.Options) error { // If the user is intentionally limiting their workspace scope, add their // folder to the roots and return early. if !options.ExpandWorkspaceToModule { @@ -257,11 +264,9 @@ func (v *View) findAndBuildWorkspaceModule(ctx context.Context, options source.O return nil } - v.workspaceMode |= usesWorkspaceModule | moduleMode - // Walk the view's folder to find all modules in the view. root := v.root.Filename() - if err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + return filepath.Walk(root, func(path string, info os.FileInfo, err error) error { if err != nil { return err } @@ -284,15 +289,39 @@ func (v *View) findAndBuildWorkspaceModule(ctx context.Context, options source.O // so add it to the view. modURI := span.URIFromPath(path) rootURI := span.URIFromPath(filepath.Dir(path)) - v.modules[rootURI] = &module{ + v.modules[rootURI] = &moduleRoot{ rootURI: rootURI, modURI: modURI, sumURI: span.URIFromPath(sumFilename(modURI)), } return nil - }); err != nil { - return err + }) +} + +func (v *View) buildWorkspaceModule(ctx context.Context) error { + // If the view has an invalid configuration, don't build the workspace + // module. + if !v.hasValidBuildConfiguration { + return nil } + // If the view is not in a module and contains no modules, but still has a + // valid workspace configuration, do not create the workspace module. + // It could be using GOPATH or a different build system entirely. + if v.modURI == "" && len(v.modules) == 0 && v.hasValidBuildConfiguration { + return nil + } + v.workspaceMode |= moduleMode + + // Don't default to multi-workspace mode if one of the modules contains a + // vendor directory. We still have to decide how to handle vendoring. + for _, mod := range v.modules { + if info, _ := os.Stat(filepath.Join(mod.rootURI.Filename(), "vendor")); info != nil { + return nil + } + } + + v.workspaceMode |= usesWorkspaceModule + // If the user does not have a gopls.mod, we need to create one, based on // modules we found in the user's workspace. var err error diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 45054901ef..81d5fc93d1 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -181,6 +181,10 @@ func (s *snapshot) RunGoCommandDirect(ctx context.Context, verb string, args []s func (s *snapshot) RunGoCommand(ctx context.Context, verb string, args []string) (*bytes.Buffer, error) { cfg := s.config(ctx) + return s.runGoCommandWithConfig(ctx, cfg, verb, args) +} + +func (s *snapshot) runGoCommandWithConfig(ctx context.Context, cfg *packages.Config, verb string, args []string) (*bytes.Buffer, error) { _, runner, inv, cleanup, err := s.goCommandInvocation(ctx, cfg, true, verb, args) if err != nil { return nil, err @@ -1248,15 +1252,13 @@ func (s *snapshot) buildBuiltinPackage(ctx context.Context, goFiles []string) er return nil } -const workspaceModuleVersion = "v0.0.0-00010101000000-000000000000" - // buildWorkspaceModule generates a workspace module given the modules in the // the workspace. func (s *snapshot) buildWorkspaceModule(ctx context.Context) (*modfile.File, error) { file := &modfile.File{} file.AddModuleStmt("gopls-workspace") - paths := make(map[string]*module) + paths := make(map[string]*moduleRoot) for _, mod := range s.view.modules { fh, err := s.view.snapshot.GetFile(ctx, mod.modURI) if err != nil { @@ -1266,9 +1268,12 @@ func (s *snapshot) buildWorkspaceModule(ctx context.Context) (*modfile.File, err if err != nil { return nil, err } + if parsed.File.Module == nil { + return nil, fmt.Errorf("no module declaration for %s", mod.modURI) + } path := parsed.File.Module.Mod.Path paths[path] = mod - file.AddNewRequire(path, workspaceModuleVersion, false) + file.AddNewRequire(path, source.WorkspaceModuleVersion, false) if err := file.AddReplace(path, "", mod.rootURI.Filename(), ""); err != nil { return nil, err } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 5011a36fa4..91d537d913 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -72,7 +72,7 @@ type View struct { // the snapshot and invalidated on file changes. // modules is the set of modules currently in this workspace. - modules map[span.URI]*module + modules map[span.URI]*moduleRoot // workspaceModule is an in-memory representation of the go.mod file for // the workspace module. @@ -177,7 +177,8 @@ type builtinPackageData struct { parsed *source.BuiltinPackage err error } -type module struct { + +type moduleRoot struct { rootURI span.URI modURI, sumURI span.URI } diff --git a/internal/lsp/cmd/cmd_test.go b/internal/lsp/cmd/cmd_test.go index be054954a2..81e20f2052 100644 --- a/internal/lsp/cmd/cmd_test.go +++ b/internal/lsp/cmd/cmd_test.go @@ -28,7 +28,7 @@ func TestCommandLine(t *testing.T) { packagestest.TestAll(t, cmdtest.TestCommandLine( "../testdata", - nil, + tests.DefaultOptions, ), ) } diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go index d5cd277904..892774c7e2 100644 --- a/internal/lsp/fake/editor.go +++ b/internal/lsp/fake/editor.go @@ -84,10 +84,6 @@ type EditorConfig struct { // EnableStaticcheck enables staticcheck analyzers. EnableStaticcheck bool - - // ExperimentalWorkspaceModule enables the experimental support for - // multi-module workspaces. - ExperimentalWorkspaceModule bool } // NewEditor Creates a new Editor. @@ -196,9 +192,8 @@ func (e *Editor) configuration() map[string]interface{} { if e.Config.EnableStaticcheck { config["staticcheck"] = true } - if e.Config.ExperimentalWorkspaceModule { - config["experimentalWorkspaceModule"] = true - } + // Default to using the experimental workspace module mode. + config["experimentalWorkspaceModule"] = true return config } diff --git a/internal/lsp/link.go b/internal/lsp/link.go index 6c0b561fb6..adacc3aff4 100644 --- a/internal/lsp/link.go +++ b/internal/lsp/link.go @@ -179,10 +179,10 @@ func moduleAtVersion(ctx context.Context, snapshot source.Snapshot, target strin if err != nil { return "", "", false } - if impPkg.Module() == nil { + if impPkg.Version() == nil { return "", "", false } - version, modpath := impPkg.Module().Version, impPkg.Module().Path + version, modpath := impPkg.Version().Version, impPkg.Version().Path if modpath == "" || version == "" { return "", "", false } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 20459f11ef..eccc56ba85 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -50,7 +50,8 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { cache := cache.New(ctx, nil) session := cache.NewSession(ctx) - options := tests.DefaultOptions() + options := source.DefaultOptions() + tests.DefaultOptions(&options) session.SetOptions(options) options.Env = datum.Config.Env view, _, release, err := session.NewView(ctx, datum.Config.Dir, span.URIFromPath(datum.Config.Dir), options) diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index af01caec9c..7c85022669 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/internal/lsp/mod/diagnostics.go @@ -89,11 +89,17 @@ func ExtractGoCommandError(ctx context.Context, snapshot source.Snapshot, fh sou if match == nil || len(match) < 3 { continue } - v.Path = match[1] - v.Version = match[2] - if err := module.Check(v.Path, v.Version); err == nil { - break + 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 { + continue } + if err := module.Check(path, version); err != nil { + continue + } + v.Path, v.Version = path, version + break } pm, err := snapshot.ParseMod(ctx, fh) if err != nil { diff --git a/internal/lsp/mod/mod_test.go b/internal/lsp/mod/mod_test.go index 4c17deceb7..1103cd87e2 100644 --- a/internal/lsp/mod/mod_test.go +++ b/internal/lsp/mod/mod_test.go @@ -11,6 +11,7 @@ import ( "testing" "golang.org/x/tools/internal/lsp/cache" + "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/tests" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/testenv" @@ -27,7 +28,8 @@ func TestModfileRemainsUnchanged(t *testing.T) { ctx := tests.Context(t) cache := cache.New(ctx, nil) session := cache.NewSession(ctx) - options := tests.DefaultOptions() + options := source.DefaultOptions() + tests.DefaultOptions(&options) options.TempModfile = true options.Env = []string{"GOPACKAGESDRIVER=off", "GOROOT="} diff --git a/internal/lsp/source/hover.go b/internal/lsp/source/hover.go index d331e72f2e..7e4a10c809 100644 --- a/internal/lsp/source/hover.go +++ b/internal/lsp/source/hover.go @@ -201,10 +201,10 @@ func moduleAtVersion(path string, i *IdentifierInfo) (string, string, bool) { if err != nil { return "", "", false } - if impPkg.Module() == nil { + if impPkg.Version() == nil { return "", "", false } - version, modpath := impPkg.Module().Version, impPkg.Module().Path + version, modpath := impPkg.Version().Version, impPkg.Version().Path if modpath == "" || version == "" { return "", "", false } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index c35e9160fb..60081a09b8 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -52,7 +52,8 @@ func testSource(t *testing.T, exporter packagestest.Exporter) { cache := cache.New(ctx, nil) session := cache.NewSession(ctx) - options := tests.DefaultOptions() + options := source.DefaultOptions() + tests.DefaultOptions(&options) options.Env = datum.Config.Env view, _, release, err := session.NewView(ctx, "source_test", span.URIFromPath(datum.Config.Dir), options) release() diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 099d87014e..20ee4b5786 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -14,8 +14,8 @@ import ( "io" "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/imports" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/span" @@ -473,7 +473,7 @@ type Package interface { GetImport(pkgPath string) (Package, error) MissingDependencies() []string Imports() []Package - Module() *packages.Module + Version() *module.Version } type Error struct { @@ -508,3 +508,9 @@ var ( InconsistentVendoring = errors.New("inconsistent vendoring") PackagesLoadError = errors.New("packages.Load error") ) + +// WorkspaceModuleVersion is the nonexistent pseudoversion 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" diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index 7a9d582a2f..65a3a48c21 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -221,8 +221,7 @@ func Context(t testing.TB) context.Context { return context.Background() } -func DefaultOptions() source.Options { - o := source.DefaultOptions() +func DefaultOptions(o *source.Options) { o.SupportedCodeActions = map[source.FileKind]map[protocol.CodeActionKind]bool{ source.Go: { protocol.SourceOrganizeImports: true, @@ -241,7 +240,7 @@ func DefaultOptions() source.Options { o.InsertTextFormat = protocol.SnippetTextFormat o.CompletionBudget = time.Minute o.HierarchicalDocumentSymbolSupport = true - return o + o.ExperimentalWorkspaceModule = true } var (