Skip to content

Commit

Permalink
gopls/internal/lsp: add quick-fixes to manage the go.work file
Browse files Browse the repository at this point in the history
Update OrphanedFileDiagnostics to provide suggested fixes for
diagnostics related to modules that are not activated by a relevant
go.work file.

Also remove the Snapshot.openFiles method, which was completely
redundant with Snapshot.overlays.

Fixes golang/go#53880

Change-Id: I7e7aed97fb0b93415fe3dc383b6daea15241f31b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/494738
Reviewed-by: Mouffull <dickmouth8340@gmail.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
findleyr committed May 15, 2023
1 parent 12a0517 commit a13793e
Show file tree
Hide file tree
Showing 18 changed files with 717 additions and 79 deletions.
15 changes: 15 additions & 0 deletions gopls/doc/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,21 @@ Args:
}
```

### **run `go work [args...]`, and apply the resulting go.work**
Identifier: `gopls.run_go_work_command`

edits to the current go.work file.

Args:

```
{
"ViewID": string,
"InitFirst": bool,
"Args": []string,
}
```

### **Run govulncheck.**
Identifier: `gopls.run_govulncheck`

Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cache/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ func (s *snapshot) workspaceLayoutError(ctx context.Context) (error, []*source.D

// Apply diagnostics about the workspace configuration to relevant open
// files.
openFiles := s.openFiles()
openFiles := s.overlays()

// If the snapshot does not have a valid build configuration, it may be
// that the user has opened a directory that contains multiple modules.
Expand Down
24 changes: 21 additions & 3 deletions gopls/internal/lsp/cache/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ type Session struct {
func (s *Session) ID() string { return s.id }
func (s *Session) String() string { return s.id }

// GoCommandRunner returns the gocommand Runner for this session.
func (s *Session) GoCommandRunner() *gocommand.Runner {
return s.gocmdRunner
}

// Options returns a copy of the SessionOptions for this session.
func (s *Session) Options() *source.Options {
s.optionsMu.Lock()
Expand Down Expand Up @@ -113,7 +118,8 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
return nil, nil, func() {}, err
}

wsModFiles, wsModFilesErr := computeWorkspaceModFiles(ctx, info.gomod, info.effectiveGOWORK(), info.effectiveGO111MODULE(), s)
gowork, _ := info.GOWORK()
wsModFiles, wsModFilesErr := computeWorkspaceModFiles(ctx, info.gomod, gowork, info.effectiveGO111MODULE(), s)

// We want a true background context and not a detached context here
// the spans need to be unrelated and no tag values should pollute it.
Expand Down Expand Up @@ -199,8 +205,8 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
return v, snapshot, snapshot.Acquire(), nil
}

// View returns a view with a matching name, if the session has one.
func (s *Session) View(name string) *View {
// ViewByName returns a view with a matching name, if the session has one.
func (s *Session) ViewByName(name string) *View {
s.viewMu.Lock()
defer s.viewMu.Unlock()
for _, view := range s.views {
Expand All @@ -211,6 +217,18 @@ func (s *Session) View(name string) *View {
return nil
}

// View returns the view with a matching id, if present.
func (s *Session) View(id string) (*View, error) {
s.viewMu.Lock()
defer s.viewMu.Unlock()
for _, view := range s.views {
if view.ID() == id {
return view, nil
}
}
return nil, fmt.Errorf("no view with ID %q", id)
}

// ViewOf returns a view corresponding to the given URI.
// If the file is not already associated with a view, pick one using some heuristics.
func (s *Session) ViewOf(uri span.URI) (*View, error) {
Expand Down
160 changes: 126 additions & 34 deletions gopls/internal/lsp/cache/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"golang.org/x/tools/go/packages"
"golang.org/x/tools/go/types/objectpath"
"golang.org/x/tools/gopls/internal/bug"
"golang.org/x/tools/gopls/internal/lsp/command"
"golang.org/x/tools/gopls/internal/lsp/filecache"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
Expand Down Expand Up @@ -191,6 +192,16 @@ type snapshot struct {
// detect ignored files.
ignoreFilterOnce sync.Once
ignoreFilter *ignoreFilter

// If non-nil, the result of computing orphaned file diagnostics.
//
// Only the field, not the map itself, is guarded by the mutex. The map must
// not be mutated.
//
// Used to save work across diagnostics+code action passes.
// TODO(rfindley): refactor all of this so there's no need to re-evaluate
// diagnostics during code-action.
orphanedFileDiagnostics map[span.URI]*source.Diagnostic
}

var globalSnapshotID uint64
Expand Down Expand Up @@ -293,7 +304,8 @@ func (s *snapshot) ModFiles() []span.URI {
}

func (s *snapshot) WorkFile() span.URI {
return s.view.effectiveGOWORK()
gowork, _ := s.view.GOWORK()
return gowork
}

func (s *snapshot) Templates() map[span.URI]source.FileHandle {
Expand Down Expand Up @@ -544,7 +556,7 @@ func (s *snapshot) goCommandInvocation(ctx context.Context, flags source.Invocat
// the main (workspace) module. Otherwise, we should use the module for
// the passed-in working dir.
if mode == source.LoadWorkspace {
if s.view.effectiveGOWORK() == "" && s.view.gomod != "" {
if gowork, _ := s.view.GOWORK(); gowork == "" && s.view.gomod != "" {
modURI = s.view.gomod
}
} else {
Expand Down Expand Up @@ -929,7 +941,7 @@ func (s *snapshot) fileWatchingGlobPatterns(ctx context.Context) map[string]stru
}

// If GOWORK is outside the folder, ensure we are watching it.
gowork := s.view.effectiveGOWORK()
gowork, _ := s.view.GOWORK()
if gowork != "" && !source.InDir(s.view.folder.Filename(), gowork.Filename()) {
patterns[gowork.Filename()] = struct{}{}
}
Expand Down Expand Up @@ -1351,19 +1363,6 @@ func (s *snapshot) IsOpen(uri span.URI) bool {
return open
}

func (s *snapshot) openFiles() []*Overlay {
s.mu.Lock()
defer s.mu.Unlock()

var open []*Overlay
s.files.Range(func(uri span.URI, fh source.FileHandle) {
if o, ok := fh.(*Overlay); ok {
open = append(open, o)
}
})
return open
}

func isFileOpen(fh source.FileHandle) bool {
_, open := fh.(*Overlay)
return open
Expand Down Expand Up @@ -1588,7 +1587,7 @@ func (s *snapshot) reloadOrphanedOpenFiles(ctx context.Context) error {
// that exist only in overlays. As a workaround, we search all of the files
// available in the snapshot and reload their metadata individually using a
// file= query if the metadata is unavailable.
open := s.openFiles()
open := s.overlays()
var files []*Overlay
for _, o := range open {
uri := o.URI()
Expand Down Expand Up @@ -1686,10 +1685,26 @@ func (s *snapshot) reloadOrphanedOpenFiles(ctx context.Context) error {
// TODO(rfindley): reconcile the definition of "orphaned" here with
// reloadOrphanedFiles. The latter does not include files with
// command-line-arguments packages.
func (s *snapshot) OrphanedFileDiagnostics(ctx context.Context) map[span.URI]*source.Diagnostic {
func (s *snapshot) OrphanedFileDiagnostics(ctx context.Context) (map[span.URI]*source.Diagnostic, error) {
// Orphaned file diagnostics are queried from code actions to produce
// quick-fixes (and may be queried many times, once for each file).
//
// Because they are non-trivial to compute, record them optimistically to
// avoid most redundant work.
//
// This is a hacky workaround: in the future we should avoid recomputing
// anything when codeActions provide a diagnostic: simply read the published
// diagnostic, if it exists.
s.mu.Lock()
meta := s.meta
existing := s.orphanedFileDiagnostics
s.mu.Unlock()
if existing != nil {
return existing, nil
}

if err := s.awaitLoaded(ctx); err != nil {
return nil, err
}

var files []*Overlay

Expand All @@ -1699,20 +1714,30 @@ searchOverlays:
if s.IsBuiltin(uri) || s.view.FileKind(o) != source.Go {
continue
}
for _, id := range meta.ids[o.URI()] {
if !source.IsCommandLineArguments(id) || meta.metadata[id].Standalone {
md, err := s.MetadataForFile(ctx, uri)
if err != nil {
return nil, err
}
for _, m := range md {
if !source.IsCommandLineArguments(m.ID) || m.Standalone {
continue searchOverlays
}
}
files = append(files, o)
}
if len(files) == 0 {
return nil
return nil, nil
}

loadedModFiles := make(map[span.URI]struct{})
ignoredFiles := make(map[span.URI]bool)
for _, meta := range meta.metadata {
loadedModFiles := make(map[span.URI]struct{}) // all mod files, including dependencies
ignoredFiles := make(map[span.URI]bool) // files reported in packages.Package.IgnoredFiles

meta, err := s.AllMetadata(ctx)
if err != nil {
return nil, err
}

for _, meta := range meta {
if meta.Module != nil && meta.Module.GoMod != "" {
gomod := span.URIFromPath(meta.Module.GoMod)
loadedModFiles[gomod] = struct{}{}
Expand Down Expand Up @@ -1740,24 +1765,85 @@ searchOverlays:
continue
}

var (
msg string // if non-empty, report a diagnostic with this message
suggestedFixes []source.SuggestedFix // associated fixes, if any
)

// If we have a relevant go.mod file, check whether the file is orphaned
// due to its go.mod file being inactive. We could also offer a
// prescriptive diagnostic in the case that there is no go.mod file, but it
// is harder to be precise in that case, and less important.
var msg string
if goMod, err := nearestModFile(ctx, fh.URI(), s); err == nil && goMod != "" {
if _, ok := loadedModFiles[goMod]; !ok {
modDir := filepath.Dir(goMod.Filename())
if rel, err := filepath.Rel(s.view.folder.Filename(), modDir); err == nil {
viewDir := s.view.folder.Filename()

// When the module is underneath the view dir, we offer
// "use all modules" quick-fixes.
inDir := source.InDir(viewDir, modDir)

if rel, err := filepath.Rel(viewDir, modDir); err == nil {
modDir = rel
}

var fix string
if s.view.goversion >= 18 {
if s.view.gowork != "" {
fix = fmt.Sprintf("To fix this problem, you can add this module to your go.work file (%s)", s.view.gowork)
if cmd, err := command.NewRunGoWorkCommandCommand("Run `go work use`", command.RunGoWorkArgs{
ViewID: s.view.ID(),
Args: []string{"use", modDir},
}); err == nil {
suggestedFixes = append(suggestedFixes, source.SuggestedFix{
Title: "Use this module in your go.work file",
Command: &cmd,
ActionKind: protocol.QuickFix,
})
}

if inDir {
if cmd, err := command.NewRunGoWorkCommandCommand("Run `go work use -r`", command.RunGoWorkArgs{
ViewID: s.view.ID(),
Args: []string{"use", "-r", "."},
}); err == nil {
suggestedFixes = append(suggestedFixes, source.SuggestedFix{
Title: "Use all modules in your workspace",
Command: &cmd,
ActionKind: protocol.QuickFix,
})
}
}
} else {
fix = "To fix this problem, you can add a go.work file that uses this directory."

if cmd, err := command.NewRunGoWorkCommandCommand("Run `go work init && go work use`", command.RunGoWorkArgs{
ViewID: s.view.ID(),
InitFirst: true,
Args: []string{"use", modDir},
}); err == nil {
suggestedFixes = []source.SuggestedFix{
{
Title: "Add a go.work file using this module",
Command: &cmd,
ActionKind: protocol.QuickFix,
},
}
}

if inDir {
if cmd, err := command.NewRunGoWorkCommandCommand("Run `go work init && go work use -r`", command.RunGoWorkArgs{
ViewID: s.view.ID(),
InitFirst: true,
Args: []string{"use", "-r", "."},
}); err == nil {
suggestedFixes = append(suggestedFixes, source.SuggestedFix{
Title: "Add a go.work file using all modules in your workspace",
Command: &cmd,
ActionKind: protocol.QuickFix,
})
}
}
}
} else {
fix = `To work with multiple modules simultaneously, please upgrade to Go 1.18 or
Expand Down Expand Up @@ -1794,16 +1880,22 @@ https://github.com/golang/tools/blob/master/gopls/doc/settings.md#buildflags-str
if msg != "" {
// Only report diagnostics if we detect an actual exclusion.
diagnostics[fh.URI()] = &source.Diagnostic{
URI: fh.URI(),
Range: rng,
Severity: protocol.SeverityWarning,
Source: source.ListError,
Message: msg,
URI: fh.URI(),
Range: rng,
Severity: protocol.SeverityWarning,
Source: source.ListError,
Message: msg,
SuggestedFixes: suggestedFixes,
}
}
}

return diagnostics
s.mu.Lock()
defer s.mu.Unlock()
if s.orphanedFileDiagnostics == nil { // another thread may have won the race
s.orphanedFileDiagnostics = diagnostics
}
return s.orphanedFileDiagnostics, nil
}

// TODO(golang/go#53756): this function needs to consider more than just the
Expand Down Expand Up @@ -1848,7 +1940,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
reinit := false
wsModFiles, wsModFilesErr := s.workspaceModFiles, s.workspaceModFilesErr

if workURI := s.view.effectiveGOWORK(); workURI != "" {
if workURI, _ := s.view.GOWORK(); workURI != "" {
if change, ok := changes[workURI]; ok {
wsModFiles, wsModFilesErr = computeWorkspaceModFiles(ctx, s.view.gomod, workURI, s.view.effectiveGO111MODULE(), &unappliedChanges{
originalSnapshot: s,
Expand Down
13 changes: 8 additions & 5 deletions gopls/internal/lsp/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,16 @@ func (w workspaceInformation) effectiveGO111MODULE() go111module {
}
}

// effectiveGOWORK returns the effective GOWORK value for this workspace, if
// GOWORK returns the effective GOWORK value for this workspace, if
// any, in URI form.
func (w workspaceInformation) effectiveGOWORK() span.URI {
//
// The second result reports whether the effective GOWORK value is "" because
// GOWORK=off.
func (w workspaceInformation) GOWORK() (span.URI, bool) {
if w.gowork == "off" || w.gowork == "" {
return ""
return "", w.gowork == "off"
}
return span.URIFromPath(w.gowork)
return span.URIFromPath(w.gowork), false
}

// GO111MODULE returns the value of GO111MODULE to use for running the go
Expand Down Expand Up @@ -540,7 +543,7 @@ func (v *View) relevantChange(c source.FileModification) bool {
//
// TODO(rfindley): Make sure the go.work files are always known
// to the view.
if c.URI == v.effectiveGOWORK() {
if gowork, _ := v.GOWORK(); gowork == c.URI {
return true
}

Expand Down
Loading

0 comments on commit a13793e

Please sign in to comment.