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 <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Rebecca Stambler 2020-09-11 00:41:36 -04:00
parent 2db8f0ff89
commit f4cefd1cb5
22 changed files with 159 additions and 67 deletions

View File

@ -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
`

View File

@ -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

View File

@ -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{

View File

@ -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"),
)
})

View File

@ -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),

View File

@ -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)
}

View File

@ -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))

View File

@ -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}
}

View File

@ -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
}

View File

@ -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

View File

@ -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
}

View File

@ -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
}

View File

@ -28,7 +28,7 @@ func TestCommandLine(t *testing.T) {
packagestest.TestAll(t,
cmdtest.TestCommandLine(
"../testdata",
nil,
tests.DefaultOptions,
),
)
}

View File

@ -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
}

View File

@ -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
}

View File

@ -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)

View File

@ -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 {

View File

@ -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="}

View File

@ -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
}

View File

@ -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()

View File

@ -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"

View File

@ -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 (