Skip to content

Commit

Permalink
internal/lsp: allow multiple go.mod files in a view
Browse files Browse the repository at this point in the history 
This change allows a view to have multiple go.mod files associated
with it. This doesn't actually make any changes in internal/lsp/cache
with regards to the view's modURI, but it does do the necessary plumbing
in the client packages.

The next CL will delete modURI.

Updates #32394

Change-Id: I2312efd69c364aed4244ee3769679885a1ebc7e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/256941
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
  • Loading branch information
@stamblerre
stamblerre committed Sep 25, 2020
1 parent e843550 commit 5d1fdd8
Show file tree
Hide file tree
Showing 15 changed files with 171 additions and 137 deletions.
11 changes: 4 additions & 7 deletions gopls/internal/regtest/diagnostics_test.go
View file
Expand Up @@ -59,15 +59,13 @@ func TestDiagnosticErrorInEditedFile(t *testing.T) {
})
}

const onlyMod = `
func TestMissingImportDiagsClearOnFirstFile(t *testing.T) {
const onlyMod = `
-- go.mod --
module mod.com
go 1.12
`

func TestMissingImportDiagsClearOnFirstFile(t *testing.T) {
t.Parallel()
runner.Run(t, onlyMod, func(t *testing.T, env *Env) {
env.CreateBuffer("main.go", `package main
Expand All @@ -85,12 +83,11 @@ func m() {
})
}

const brokenFile = `package main
func TestDiagnosticErrorInNewFile(t *testing.T) {
const brokenFile = `package main
const Foo = "abc
`

func TestDiagnosticErrorInNewFile(t *testing.T) {
runner.Run(t, brokenFile, func(t *testing.T, env *Env) {
env.CreateBuffer("broken.go", brokenFile)
env.Await(env.DiagnosticAtRegexp("broken.go", "\"abc"))
Expand Down
11 changes: 7 additions & 4 deletions internal/lsp/cache/load.go
View file
Expand Up @@ -88,8 +88,6 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
ctx, done := event.Start(ctx, "cache.view.load", tag.Query.Of(query))
defer done()

cfg := s.config(ctx)

cleanup := func() {}

var modFH, sumFH source.FileHandle
Expand All @@ -107,6 +105,8 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
}
}

wdir := s.view.rootURI.Filename()
var buildFlags []string
switch {
case s.view.workspaceMode&usesWorkspaceModule != 0:
var (
Expand All @@ -117,16 +117,19 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
if err != nil {
return err
}
cfg.Dir = tmpDir.Filename()
wdir = tmpDir.Filename()
case s.view.workspaceMode&tempModfile != 0:
var tmpURI span.URI
tmpURI, cleanup, err = tempModFile(modFH, sumFH)
if err != nil {
return err
}
cfg.BuildFlags = append(cfg.BuildFlags, fmt.Sprintf("-modfile=%s", tmpURI.Filename()))
buildFlags = append(buildFlags, fmt.Sprintf("-modfile=%s", tmpURI.Filename()))
}

cfg := s.config(ctx, wdir)
cfg.BuildFlags = append(cfg.BuildFlags, buildFlags...)

modMod, err := s.view.needsModEqualsMod(ctx, modFH)
if err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions internal/lsp/cache/mod.go
View file
Expand Up @@ -206,7 +206,7 @@ func (s *snapshot) ModWhy(ctx context.Context, fh source.FileHandle) (map[string
return handle.why(ctx, s)
}
// Make sure to use the module root as the working directory.
cfg := s.configWithDir(ctx, filepath.Dir(fh.URI().Filename()))
cfg := s.config(ctx, filepath.Dir(fh.URI().Filename()))
key := modKey{
sessionID: s.view.session.id,
cfg: hashConfig(cfg),
Expand Down Expand Up @@ -298,7 +298,7 @@ func (s *snapshot) ModUpgrade(ctx context.Context, fh source.FileHandle) (map[st
return handle.upgrades(ctx, s)
}
// Use the module root as the working directory.
cfg := s.configWithDir(ctx, filepath.Dir(fh.URI().Filename()))
cfg := s.config(ctx, filepath.Dir(fh.URI().Filename()))
key := modKey{
sessionID: s.view.session.id,
cfg: hashConfig(cfg),
Expand Down
4 changes: 2 additions & 2 deletions internal/lsp/cache/mod_tidy.go
View file
Expand Up @@ -75,7 +75,7 @@ func (s *snapshot) ModTidy(ctx context.Context, fh source.FileHandle) (*source.T
s.mu.Unlock()

// Make sure to use the module root in the configuration.
cfg := s.configWithDir(ctx, filepath.Dir(fh.URI().Filename()))
cfg := s.config(ctx, filepath.Dir(fh.URI().Filename()))
key := modTidyKey{
sessionID: s.view.session.id,
view: s.view.folder.Filename(),
Expand Down Expand Up @@ -108,7 +108,7 @@ func (s *snapshot) ModTidy(ctx context.Context, fh source.FileHandle) (*source.T
}
// Get a new config to avoid races, since it may be modified by
// goCommandInvocation.
cfg := s.configWithDir(ctx, filepath.Dir(fh.URI().Filename()))
cfg := s.config(ctx, filepath.Dir(fh.URI().Filename()))
tmpURI, runner, inv, cleanup, err := snapshot.goCommandInvocation(ctx, cfg, true, "mod", []string{"tidy"})
if err != nil {
return &modTidyData{err: err}
Expand Down
50 changes: 32 additions & 18 deletions internal/lsp/cache/snapshot.go
View file
Expand Up @@ -125,18 +125,13 @@ func (s *snapshot) FileSet() *token.FileSet {
return s.view.session.cache.fset
}

// config returns a *packages.Config with the working directory set to the
// view's root.
func (s *snapshot) config(ctx context.Context) *packages.Config {
return s.configWithDir(ctx, s.view.rootURI.Filename())
}

// configWithDir returns the configuration used for the snapshot's interaction
// with the go/packages API. It uses the given working directory.
// config returns the configuration used for the snapshot's interaction with
// the go/packages API. It uses the given working directory.
//
// TODO(rstambler): go/packages requires that we do not provide overlays for
// multiple modules in on config, so buildOverlay needs to filter overlays by
// module.
func (s *snapshot) configWithDir(ctx context.Context, dir string) *packages.Config {
func (s *snapshot) config(ctx context.Context, dir string) *packages.Config {
s.view.optionsMu.Lock()
env, buildFlags := s.view.envLocked()
verboseOutput := s.view.options.VerboseOutput
Expand Down Expand Up @@ -174,8 +169,8 @@ func (s *snapshot) configWithDir(ctx context.Context, dir string) *packages.Conf
return cfg
}

func (s *snapshot) RunGoCommandDirect(ctx context.Context, verb string, args []string) error {
cfg := s.config(ctx)
func (s *snapshot) RunGoCommandDirect(ctx context.Context, wd, verb string, args []string) error {
cfg := s.config(ctx, wd)
_, runner, inv, cleanup, err := s.goCommandInvocation(ctx, cfg, false, verb, args)
if err != nil {
return err
Expand All @@ -186,11 +181,6 @@ func (s *snapshot) RunGoCommandDirect(ctx context.Context, verb string, args []s
return err
}

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 {
Expand All @@ -201,8 +191,8 @@ func (s *snapshot) runGoCommandWithConfig(ctx context.Context, cfg *packages.Con
return runner.Run(ctx, *inv)
}

func (s *snapshot) RunGoCommandPiped(ctx context.Context, verb string, args []string, stdout, stderr io.Writer) error {
cfg := s.config(ctx)
func (s *snapshot) RunGoCommandPiped(ctx context.Context, wd, verb string, args []string, stdout, stderr io.Writer) error {
cfg := s.config(ctx, wd)
_, runner, inv, cleanup, err := s.goCommandInvocation(ctx, cfg, true, verb, args)
if err != nil {
return err
Expand Down Expand Up @@ -607,6 +597,30 @@ func (s *snapshot) CachedImportPaths(ctx context.Context) (map[string]source.Pac
return results, nil
}

func (s *snapshot) GoModForFile(ctx context.Context, fh source.FileHandle) (source.VersionedFileHandle, error) {
if fh.Kind() != source.Go {
return nil, fmt.Errorf("expected Go file, got %s", fh.Kind())
}
if len(s.modules) == 0 {
if s.view.modURI == "" {
return nil, errors.New("no modules in this view")
}
return s.GetFile(ctx, s.view.modURI)
}
var match span.URI
for _, m := range s.modules {
// Add an os.PathSeparator to the end of each directory to make sure
// that foo/apple/banana does not match foo/a.
if !strings.HasPrefix(fh.URI().Filename()+string(os.PathSeparator), m.rootURI.Filename()+string(os.PathSeparator)) {
continue
}
if len(m.modURI) > len(match) {
match = m.modURI
}
}
return s.GetFile(ctx, match)
}

func (s *snapshot) getPackage(id packageID, mode source.ParseMode) *packageHandle {
s.mu.Lock()
defer s.mu.Unlock()
Expand Down
9 changes: 6 additions & 3 deletions internal/lsp/cache/view.go
View file
Expand Up @@ -213,8 +213,11 @@ func (v *View) ValidBuildConfiguration() bool {
return v.hasValidBuildConfiguration
}

func (v *View) ModFile() span.URI {
return v.modURI
func (v *View) ModFiles() []span.URI {
if v.modURI == "" {
return nil
}
return []span.URI{v.modURI}
}

// tempModFile creates a temporary go.mod file based on the contents of the
Expand Down Expand Up @@ -276,7 +279,7 @@ func (v *View) Name() string {
return v.name
}

// Folder returns the root of this view.
// Folder returns the folder at the base of this view.
func (v *View) Folder() span.URI {
return v.folder
}
Expand Down
43 changes: 21 additions & 22 deletions internal/lsp/code_action.go
View file
Expand Up @@ -51,12 +51,8 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
var codeActions []protocol.CodeAction
switch fh.Kind() {
case source.Mod:
// TODO: Support code actions for views with multiple modules.
if snapshot.View().ModFile() == "" {
return nil, nil
}
if diagnostics := params.Context.Diagnostics; len(diagnostics) > 0 {
modQuickFixes, err := moduleQuickFixes(ctx, snapshot, diagnostics)
modQuickFixes, err := moduleQuickFixes(ctx, snapshot, fh, diagnostics)
if err == source.ErrTmpModfileUnsupported {
return nil, nil
}
Expand All @@ -66,7 +62,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
codeActions = append(codeActions, modQuickFixes...)
}
if wanted[protocol.SourceOrganizeImports] {
action, err := goModTidy(ctx, snapshot)
action, err := goModTidy(ctx, snapshot, fh)
if err == source.ErrTmpModfileUnsupported {
return nil, nil
}
Expand Down Expand Up @@ -138,7 +134,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara

// If there are any diagnostics relating to the go.mod file,
// add their corresponding quick fixes.
modQuickFixes, err := moduleQuickFixes(ctx, snapshot, diagnostics)
modQuickFixes, err := moduleQuickFixes(ctx, snapshot, fh, diagnostics)
if err != nil {
// Not a fatal error.
event.Error(ctx, "module suggested fixes failed", err, tag.Directory.Of(snapshot.View().Folder()))
Expand Down Expand Up @@ -457,10 +453,17 @@ func documentChanges(fh source.VersionedFileHandle, edits []protocol.TextEdit) [
}
}

func moduleQuickFixes(ctx context.Context, snapshot source.Snapshot, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) {
modFH, err := snapshot.GetFile(ctx, snapshot.View().ModFile())
if err != nil {
return nil, err
func moduleQuickFixes(ctx context.Context, snapshot source.Snapshot, fh source.VersionedFileHandle, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) {
var modFH source.VersionedFileHandle
switch fh.Kind() {
case source.Mod:
modFH = fh
case source.Go:
var err error
modFH, err = snapshot.GoModForFile(ctx, fh)
if err != nil {
return nil, err
}
}
tidied, err := snapshot.ModTidy(ctx, modFH)
if err == source.ErrTmpModfileUnsupported {
Expand Down Expand Up @@ -512,21 +515,17 @@ func sameDiagnostic(d protocol.Diagnostic, e source.Error) bool {
return d.Message == e.Message && protocol.CompareRange(d.Range, e.Range) == 0 && d.Source == e.Category
}

func goModTidy(ctx context.Context, snapshot source.Snapshot) (*protocol.CodeAction, error) {
modFH, err := snapshot.GetFile(ctx, snapshot.View().ModFile())
func goModTidy(ctx context.Context, snapshot source.Snapshot, fh source.VersionedFileHandle) (*protocol.CodeAction, error) {
tidied, err := snapshot.ModTidy(ctx, fh)
if err != nil {
return nil, err
}
tidied, err := snapshot.ModTidy(ctx, modFH)
if err != nil {
return nil, err
}
left, err := modFH.Read()
left, err := fh.Read()
if err != nil {
return nil, err
}
right := tidied.TidiedContent
edits := snapshot.View().Options().ComputeEdits(modFH.URI(), string(left), string(right))
edits := snapshot.View().Options().ComputeEdits(fh.URI(), string(left), string(right))
protocolEdits, err := source.ToProtocolEdits(tidied.Parsed.Mapper, edits)
if err != nil {
return nil, err
Expand All @@ -537,13 +536,13 @@ func goModTidy(ctx context.Context, snapshot source.Snapshot) (*protocol.CodeAct
Edit: protocol.WorkspaceEdit{
DocumentChanges: []protocol.TextDocumentEdit{{
TextDocument: protocol.VersionedTextDocumentIdentifier{
Version: modFH.Version(),
Version: fh.Version(),
TextDocumentIdentifier: protocol.TextDocumentIdentifier{
URI: protocol.URIFromSpanURI(modFH.URI()),
URI: protocol.URIFromSpanURI(fh.URI()),
},
},
Edits: protocolEdits,
}},
},
}, nil
}, err
}
17 changes: 10 additions & 7 deletions internal/lsp/command.go
View file
Expand Up @@ -262,9 +262,10 @@ func (s *Server) directGoModCommand(ctx context.Context, uri protocol.DocumentUR
if err != nil {
return err
}
wdir := filepath.Dir(uri.SpanURI().Filename())
snapshot, release := view.Snapshot(ctx)
defer release()
return snapshot.RunGoCommandDirect(ctx, verb, args)
return snapshot.RunGoCommandDirect(ctx, wdir, verb, args)
}

func (s *Server) runTests(ctx context.Context, snapshot source.Snapshot, uri protocol.DocumentURI, work *workDone, tests, benchmarks []string) error {
Expand All @@ -282,11 +283,13 @@ func (s *Server) runTests(ctx context.Context, snapshot source.Snapshot, uri pro
ew := &eventWriter{ctx: ctx, operation: "test"}
out := io.MultiWriter(ew, workDoneWriter{work}, buf)

wdir := filepath.Dir(uri.SpanURI().Filename())

// Run `go test -run Func` on each test.
var failedTests int
for _, funcName := range tests {
args := []string{pkgPath, "-v", "-count=1", "-run", fmt.Sprintf("^%s$", funcName)}
if err := snapshot.RunGoCommandPiped(ctx, "test", args, out, out); err != nil {
if err := snapshot.RunGoCommandPiped(ctx, wdir, "test", args, out, out); err != nil {
if errors.Is(err, context.Canceled) {
return err
}
Expand All @@ -298,7 +301,7 @@ func (s *Server) runTests(ctx context.Context, snapshot source.Snapshot, uri pro
var failedBenchmarks int
for _, funcName := range benchmarks {
args := []string{pkgPath, "-v", "-run=^$", "-bench", fmt.Sprintf("^%s$", funcName)}
if err := snapshot.RunGoCommandPiped(ctx, "test", args, out, out); err != nil {
if err := snapshot.RunGoCommandPiped(ctx, wdir, "test", args, out, out); err != nil {
if errors.Is(err, context.Canceled) {
return err
}
Expand Down Expand Up @@ -342,15 +345,15 @@ func (s *Server) runGoGenerate(ctx context.Context, snapshot source.Snapshot, ur

er := &eventWriter{ctx: ctx, operation: "generate"}
args := []string{"-x"}
dir := uri.Filename()
pattern := "."
if recursive {
dir = filepath.Join(dir, "...")
pattern = "..."
}
args = append(args, dir)
args = append(args, pattern)

stderr := io.MultiWriter(er, workDoneWriter{work})

if err := snapshot.RunGoCommandPiped(ctx, "generate", args, er, stderr); err != nil {
if err := snapshot.RunGoCommandPiped(ctx, uri.Filename(), "generate", args, er, stderr); err != nil {
return err
}
return nil
Expand Down

0 comments on commit 5d1fdd8

Please sign in to comment.