gopls/internal/lsp: add quick-fixes to manage the go.work file
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>
diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md
index be031e9..8fe677b 100644
--- a/gopls/doc/commands.md
+++ b/gopls/doc/commands.md
@@ -289,6 +289,21 @@
}
```
+### **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`
diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go
index 6f60c3b..521dc1e 100644
--- a/gopls/internal/lsp/cache/load.go
+++ b/gopls/internal/lsp/cache/load.go
@@ -354,7 +354,7 @@
// 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.
diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go
index 17f9bdb..eaad67c 100644
--- a/gopls/internal/lsp/cache/session.go
+++ b/gopls/internal/lsp/cache/session.go
@@ -46,6 +46,11 @@
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()
@@ -113,7 +118,8 @@
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.
@@ -199,8 +205,8 @@
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 {
@@ -211,6 +217,18 @@
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) {
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 2035364..45a32a1 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -31,6 +31,7 @@
"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"
@@ -191,6 +192,16 @@
// 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
@@ -293,7 +304,8 @@
}
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 {
@@ -544,7 +556,7 @@
// 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 {
@@ -929,7 +941,7 @@
}
// 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{}{}
}
@@ -1351,19 +1363,6 @@
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
@@ -1588,7 +1587,7 @@
// 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()
@@ -1686,10 +1685,26 @@
// 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
@@ -1699,20 +1714,30 @@
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{}{}
@@ -1740,15 +1765,25 @@
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
}
@@ -1756,8 +1791,59 @@
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
@@ -1794,16 +1880,22 @@
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
@@ -1848,7 +1940,7 @@
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,
diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go
index 884b0fc..db2c1dc 100644
--- a/gopls/internal/lsp/cache/view.go
+++ b/gopls/internal/lsp/cache/view.go
@@ -145,13 +145,16 @@
}
}
-// 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
@@ -540,7 +543,7 @@
//
// 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
}
diff --git a/gopls/internal/lsp/code_action.go b/gopls/internal/lsp/code_action.go
index 5819565..8658ba55 100644
--- a/gopls/internal/lsp/code_action.go
+++ b/gopls/internal/lsp/code_action.go
@@ -176,6 +176,18 @@
},
})
}
+
+ diags, err := snapshot.OrphanedFileDiagnostics(ctx)
+ if err != nil {
+ return nil, err
+ }
+ if d, ok := diags[fh.URI()]; ok {
+ quickFixes, err := codeActionsMatchingDiagnostics(ctx, snapshot, diagnostics, []*source.Diagnostic{d})
+ if err != nil {
+ return nil, err
+ }
+ codeActions = append(codeActions, quickFixes...)
+ }
}
if ctx.Err() != nil {
return nil, ctx.Err()
diff --git a/gopls/internal/lsp/command.go b/gopls/internal/lsp/command.go
index 6fa8312..7236087 100644
--- a/gopls/internal/lsp/command.go
+++ b/gopls/internal/lsp/command.go
@@ -22,6 +22,7 @@
"golang.org/x/mod/modfile"
"golang.org/x/tools/go/ast/astutil"
+ "golang.org/x/tools/gopls/internal/bug"
"golang.org/x/tools/gopls/internal/govulncheck"
"golang.org/x/tools/gopls/internal/lsp/cache"
"golang.org/x/tools/gopls/internal/lsp/command"
@@ -69,6 +70,7 @@
async bool // whether to run the command asynchronously. Async commands can only return errors.
requireSave bool // whether all files must be saved for the command to work
progress string // title to use for progress reporting. If empty, no progress will be reported.
+ forView string // view to resolve to a snapshot; incompatible with forURI
forURI protocol.DocumentURI // URI to resolve to a snapshot. If unset, snapshot will be nil.
}
@@ -103,6 +105,9 @@
}
}
var deps commandDeps
+ if cfg.forURI != "" && cfg.forView != "" {
+ return bug.Errorf("internal error: forURI=%q, forView=%q", cfg.forURI, cfg.forView)
+ }
if cfg.forURI != "" {
var ok bool
var release func()
@@ -114,6 +119,17 @@
}
return fmt.Errorf("invalid file URL: %v", cfg.forURI)
}
+ } else if cfg.forView != "" {
+ view, err := c.s.session.View(cfg.forView)
+ if err != nil {
+ return err
+ }
+ var release func()
+ deps.snapshot, release, err = view.Snapshot()
+ if err != nil {
+ return err
+ }
+ defer release()
}
ctx, cancel := context.WithCancel(xcontext.Detach(ctx))
if cfg.progress != "" {
@@ -576,40 +592,26 @@
}
modURI := snapshot.GoModForFile(uri)
sumURI := span.URIFromPath(strings.TrimSuffix(modURI.Filename(), ".mod") + ".sum")
- modEdits, err := applyFileEdits(ctx, snapshot, modURI, newModBytes)
+ modEdits, err := collectFileEdits(ctx, snapshot, modURI, newModBytes)
if err != nil {
return err
}
- sumEdits, err := applyFileEdits(ctx, snapshot, sumURI, newSumBytes)
+ sumEdits, err := collectFileEdits(ctx, snapshot, sumURI, newSumBytes)
if err != nil {
return err
}
- changes := append(sumEdits, modEdits...)
- if len(changes) == 0 {
- return nil
- }
- documentChanges := []protocol.DocumentChanges{} // must be a slice
- for _, change := range changes {
- change := change
- documentChanges = append(documentChanges, protocol.DocumentChanges{
- TextDocumentEdit: &change,
- })
- }
- response, err := s.client.ApplyEdit(ctx, &protocol.ApplyWorkspaceEditParams{
- Edit: protocol.WorkspaceEdit{
- DocumentChanges: documentChanges,
- },
- })
- if err != nil {
- return err
- }
- if !response.Applied {
- return fmt.Errorf("edits not applied because of %s", response.FailureReason)
- }
- return nil
+ return applyFileEdits(ctx, s.client, append(sumEdits, modEdits...))
}
-func applyFileEdits(ctx context.Context, snapshot source.Snapshot, uri span.URI, newContent []byte) ([]protocol.TextDocumentEdit, error) {
+// collectFileEdits collects any file edits required to transform the snapshot
+// file specified by uri to the provided new content.
+//
+// If the file is not open, collectFileEdits simply writes the new content to
+// disk.
+//
+// TODO(rfindley): fix this API asymmetry. It should be up to the caller to
+// write the file or apply the edits.
+func collectFileEdits(ctx context.Context, snapshot source.Snapshot, uri span.URI, newContent []byte) ([]protocol.TextDocumentEdit, error) {
fh, err := snapshot.ReadFile(ctx, uri)
if err != nil {
return nil, err
@@ -618,6 +620,7 @@
if err != nil && !os.IsNotExist(err) {
return nil, err
}
+
if bytes.Equal(oldContent, newContent) {
return nil, nil
}
@@ -647,6 +650,31 @@
}}, nil
}
+func applyFileEdits(ctx context.Context, cli protocol.Client, edits []protocol.TextDocumentEdit) error {
+ if len(edits) == 0 {
+ return nil
+ }
+ documentChanges := []protocol.DocumentChanges{} // must be a slice
+ for _, change := range edits {
+ change := change
+ documentChanges = append(documentChanges, protocol.DocumentChanges{
+ TextDocumentEdit: &change,
+ })
+ }
+ response, err := cli.ApplyEdit(ctx, &protocol.ApplyWorkspaceEditParams{
+ Edit: protocol.WorkspaceEdit{
+ DocumentChanges: documentChanges,
+ },
+ })
+ if err != nil {
+ return err
+ }
+ if !response.Applied {
+ return fmt.Errorf("edits not applied because of %s", response.FailureReason)
+ }
+ return nil
+}
+
func runGoGetModule(invoke func(...string) (*bytes.Buffer, error), addRequire bool, args []string) error {
if addRequire {
if err := addModuleRequire(invoke, args); err != nil {
@@ -1038,3 +1066,82 @@
return stats
}
+
+// RunGoWorkCommand invokes `go work <args>` with the provided arguments.
+//
+// args.InitFirst controls whether to first run `go work init`. This allows a
+// single command to both create and recursively populate a go.work file -- as
+// of writing there is no `go work init -r`.
+//
+// Some thought went into implementing this command. Unlike the go.mod commands
+// above, this command simply invokes the go command and relies on the client
+// to notify gopls of file changes via didChangeWatchedFile notifications.
+// We could instead run these commands with GOWORK set to a temp file, but that
+// poses the following problems:
+// - directory locations in the resulting temp go.work file will be computed
+// relative to the directory containing that go.work. If the go.work is in a
+// tempdir, the directories will need to be translated to/from that dir.
+// - it would be simpler to use a temp go.work file in the workspace
+// directory, or whichever directory contains the real go.work file, but
+// that sets a bad precedent of writing to a user-owned directory. We
+// shouldn't start doing that.
+// - Sending workspace edits to create a go.work file would require using
+// the CreateFile resource operation, which would need to be tested in every
+// client as we haven't used it before. We don't have time for that right
+// now.
+//
+// Therefore, we simply require that the current go.work file is saved (if it
+// exists), and delegate to the go command.
+func (c *commandHandler) RunGoWorkCommand(ctx context.Context, args command.RunGoWorkArgs) error {
+ return c.run(ctx, commandConfig{
+ progress: "Running go work command",
+ forView: args.ViewID,
+ }, func(ctx context.Context, deps commandDeps) (runErr error) {
+ snapshot := deps.snapshot
+ view := snapshot.View().(*cache.View)
+ viewDir := view.Folder().Filename()
+
+ // If the user has explicitly set GOWORK=off, we should warn them
+ // explicitly and avoid potentially misleading errors below.
+ goworkURI, off := view.GOWORK()
+ if off {
+ return fmt.Errorf("cannot modify go.work files when GOWORK=off")
+ }
+ gowork := goworkURI.Filename()
+
+ if goworkURI != "" {
+ fh, err := snapshot.ReadFile(ctx, goworkURI)
+ if err != nil {
+ return fmt.Errorf("reading current go.work file: %v", err)
+ }
+ if !fh.Saved() {
+ return fmt.Errorf("must save workspace file %s before running go work commands", goworkURI)
+ }
+ } else {
+ if !args.InitFirst {
+ // If go.work does not exist, we should have detected that and asked
+ // for InitFirst.
+ return bug.Errorf("internal error: cannot run go work command: required go.work file not found")
+ }
+ gowork = filepath.Join(viewDir, "go.work")
+ if err := c.invokeGoWork(ctx, viewDir, gowork, []string{"init"}); err != nil {
+ return fmt.Errorf("running `go work init`: %v", err)
+ }
+ }
+
+ return c.invokeGoWork(ctx, viewDir, gowork, args.Args)
+ })
+}
+
+func (c *commandHandler) invokeGoWork(ctx context.Context, viewDir, gowork string, args []string) error {
+ inv := gocommand.Invocation{
+ Verb: "work",
+ Args: args,
+ WorkingDir: viewDir,
+ Env: append(os.Environ(), fmt.Sprintf("GOWORK=%s", gowork)),
+ }
+ if _, err := c.s.session.GoCommandRunner().Run(ctx, inv); err != nil {
+ return fmt.Errorf("running go work command: %v", err)
+ }
+ return nil
+}
diff --git a/gopls/internal/lsp/command/command_gen.go b/gopls/internal/lsp/command/command_gen.go
index a6f9940..8003b17 100644
--- a/gopls/internal/lsp/command/command_gen.go
+++ b/gopls/internal/lsp/command/command_gen.go
@@ -34,6 +34,7 @@
RegenerateCgo Command = "regenerate_cgo"
RemoveDependency Command = "remove_dependency"
ResetGoModDiagnostics Command = "reset_go_mod_diagnostics"
+ RunGoWorkCommand Command = "run_go_work_command"
RunGovulncheck Command = "run_govulncheck"
RunTests Command = "run_tests"
StartDebugging Command = "start_debugging"
@@ -62,6 +63,7 @@
RegenerateCgo,
RemoveDependency,
ResetGoModDiagnostics,
+ RunGoWorkCommand,
RunGovulncheck,
RunTests,
StartDebugging,
@@ -162,6 +164,12 @@
return nil, err
}
return nil, s.ResetGoModDiagnostics(ctx, a0)
+ case "gopls.run_go_work_command":
+ var a0 RunGoWorkArgs
+ if err := UnmarshalArgs(params.Arguments, &a0); err != nil {
+ return nil, err
+ }
+ return nil, s.RunGoWorkCommand(ctx, a0)
case "gopls.run_govulncheck":
var a0 VulncheckArgs
if err := UnmarshalArgs(params.Arguments, &a0); err != nil {
@@ -404,6 +412,18 @@
}, nil
}
+func NewRunGoWorkCommandCommand(title string, a0 RunGoWorkArgs) (protocol.Command, error) {
+ args, err := MarshalArgs(a0)
+ if err != nil {
+ return protocol.Command{}, err
+ }
+ return protocol.Command{
+ Title: title,
+ Command: "gopls.run_go_work_command",
+ Arguments: args,
+ }, nil
+}
+
func NewRunGovulncheckCommand(title string, a0 VulncheckArgs) (protocol.Command, error) {
args, err := MarshalArgs(a0)
if err != nil {
diff --git a/gopls/internal/lsp/command/interface.go b/gopls/internal/lsp/command/interface.go
index 969ed8a..1342e84 100644
--- a/gopls/internal/lsp/command/interface.go
+++ b/gopls/internal/lsp/command/interface.go
@@ -170,6 +170,10 @@
// This command is intended for internal use only, by the gopls stats
// command.
WorkspaceStats(context.Context) (WorkspaceStatsResult, error)
+
+ // RunGoWorkCommand: run `go work [args...]`, and apply the resulting go.work
+ // edits to the current go.work file.
+ RunGoWorkCommand(context.Context, RunGoWorkArgs) error
}
type RunTestsArgs struct {
@@ -447,3 +451,9 @@
CompiledGoFiles int // total number of compiled Go files across all packages
Modules int // total number of unique modules
}
+
+type RunGoWorkArgs struct {
+ ViewID string // ID of the view to run the command from
+ InitFirst bool // Whether to run `go work init` first
+ Args []string // Args to pass to `go work`
+}
diff --git a/gopls/internal/lsp/diagnostics.go b/gopls/internal/lsp/diagnostics.go
index 26f2242..90c2232 100644
--- a/gopls/internal/lsp/diagnostics.go
+++ b/gopls/internal/lsp/diagnostics.go
@@ -391,8 +391,14 @@
// Orphaned files.
// Confirm that every opened file belongs to a package (if any exist in
// the workspace). Otherwise, add a diagnostic to the file.
- for uri, diag := range snapshot.OrphanedFileDiagnostics(ctx) {
- s.storeDiagnostics(snapshot, uri, orphanedSource, []*source.Diagnostic{diag}, true)
+ if diags, err := snapshot.OrphanedFileDiagnostics(ctx); err == nil {
+ for uri, diag := range diags {
+ s.storeDiagnostics(snapshot, uri, orphanedSource, []*source.Diagnostic{diag}, true)
+ }
+ } else {
+ if ctx.Err() == nil {
+ event.Error(ctx, "computing orphaned file diagnostics", err, source.SnapshotLabels(snapshot)...)
+ }
}
}
diff --git a/gopls/internal/lsp/lsp_test.go b/gopls/internal/lsp/lsp_test.go
index bbe1f1c..ed3baa2 100644
--- a/gopls/internal/lsp/lsp_test.go
+++ b/gopls/internal/lsp/lsp_test.go
@@ -241,7 +241,7 @@
func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnostic) {
// Get the diagnostics for this view if we have not done it before.
- v := r.server.session.View(r.data.Config.Dir)
+ v := r.server.session.ViewByName(r.data.Config.Dir)
r.collectDiagnostics(v)
tests.CompareDiagnostics(t, uri, want, r.diagnostics[uri])
}
diff --git a/gopls/internal/lsp/regtest/marker.go b/gopls/internal/lsp/regtest/marker.go
index ddd05af..29722c9 100644
--- a/gopls/internal/lsp/regtest/marker.go
+++ b/gopls/internal/lsp/regtest/marker.go
@@ -1484,7 +1484,7 @@
// Apply the fix it suggests.
changed, err := codeAction(mark.run.env, loc.URI, loc.Range, actionKind, nil)
if err != nil {
- mark.errorf("suggestedfix failed: %v. (Use @suggestedfixerr for expected errors.)", err)
+ mark.errorf("codeAction failed: %v", err)
return
}
diff --git a/gopls/internal/lsp/source/api_json.go b/gopls/internal/lsp/source/api_json.go
index d6fddc9..281772b 100644
--- a/gopls/internal/lsp/source/api_json.go
+++ b/gopls/internal/lsp/source/api_json.go
@@ -769,6 +769,12 @@
ArgDoc: "{\n\t\"URIArg\": {\n\t\t\"URI\": string,\n\t},\n\t// Optional: source of the diagnostics to reset.\n\t// If not set, all resettable go.mod diagnostics will be cleared.\n\t\"DiagnosticSource\": string,\n}",
},
{
+ Command: "gopls.run_go_work_command",
+ Title: "run `go work [args...]`, and apply the resulting go.work",
+ Doc: "edits to the current go.work file.",
+ ArgDoc: "{\n\t\"ViewID\": string,\n\t\"InitFirst\": bool,\n\t\"Args\": []string,\n}",
+ },
+ {
Command: "gopls.run_govulncheck",
Title: "Run govulncheck.",
Doc: "Run vulnerability check (`govulncheck`).",
diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go
index 2a16ad6..e77f9d2 100644
--- a/gopls/internal/lsp/source/view.go
+++ b/gopls/internal/lsp/source/view.go
@@ -209,7 +209,9 @@
// OrphanedFileDiagnostics reports diagnostics for files that have no package
// associations or which only have only command-line-arguments packages.
- OrphanedFileDiagnostics(ctx context.Context) map[span.URI]*Diagnostic
+ //
+ // The caller must not mutate the result.
+ OrphanedFileDiagnostics(ctx context.Context) (map[span.URI]*Diagnostic, error)
// -- package type-checking --
diff --git a/gopls/internal/lsp/workspace.go b/gopls/internal/lsp/workspace.go
index 53cdcac..818135e 100644
--- a/gopls/internal/lsp/workspace.go
+++ b/gopls/internal/lsp/workspace.go
@@ -17,7 +17,7 @@
func (s *Server) didChangeWorkspaceFolders(ctx context.Context, params *protocol.DidChangeWorkspaceFoldersParams) error {
event := params.Event
for _, folder := range event.Removed {
- view := s.session.View(folder.Name)
+ view := s.session.ViewByName(folder.Name)
if view != nil {
s.session.RemoveView(view)
} else {
diff --git a/gopls/internal/regtest/marker/testdata/quickfix/addgowork.txt b/gopls/internal/regtest/marker/testdata/diagnostics/addgowork.txt
similarity index 84%
rename from gopls/internal/regtest/marker/testdata/quickfix/addgowork.txt
rename to gopls/internal/regtest/marker/testdata/diagnostics/addgowork.txt
index dbd1a95..2cb7d2b 100644
--- a/gopls/internal/regtest/marker/testdata/quickfix/addgowork.txt
+++ b/gopls/internal/regtest/marker/testdata/diagnostics/addgowork.txt
@@ -1,6 +1,7 @@
-This test demonstrates the quick-fix for adding a go.work file.
+This test demonstrates diagnostics for adding a go.work file.
-TODO(rfindley): actually add quick-fixes here.
+Quick-fixes change files on disk, so are tested by regtests.
+
TODO(rfindley): improve the "cannot find package" import errors.
-- flags --
diff --git a/gopls/internal/regtest/marker/testdata/quickfix/usemodule.txt b/gopls/internal/regtest/marker/testdata/diagnostics/usemodule.txt
similarity index 79%
rename from gopls/internal/regtest/marker/testdata/quickfix/usemodule.txt
rename to gopls/internal/regtest/marker/testdata/diagnostics/usemodule.txt
index 62f4efc..35d2e43 100644
--- a/gopls/internal/regtest/marker/testdata/quickfix/usemodule.txt
+++ b/gopls/internal/regtest/marker/testdata/diagnostics/usemodule.txt
@@ -1,6 +1,7 @@
-This test demonstrates the quick-fix for using a module directory.
+This test demonstrates diagnostics for a module that is missing from the
+go.work file.
-TODO(rfindley): actually add quick-fixes here.
+Quick-fixes change files on disk, so are tested by regtests.
-- flags --
-min_go=go1.18
@@ -11,6 +12,7 @@
use (
./a
)
+
-- a/go.mod --
module mod.com/a
diff --git a/gopls/internal/regtest/workspace/quickfix_test.go b/gopls/internal/regtest/workspace/quickfix_test.go
new file mode 100644
index 0000000..5cb08f0
--- /dev/null
+++ b/gopls/internal/regtest/workspace/quickfix_test.go
@@ -0,0 +1,344 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package workspace
+
+import (
+ "fmt"
+ "strings"
+ "testing"
+
+ "golang.org/x/tools/gopls/internal/lsp/protocol"
+ "golang.org/x/tools/gopls/internal/lsp/tests/compare"
+ "golang.org/x/tools/internal/testenv"
+
+ . "golang.org/x/tools/gopls/internal/lsp/regtest"
+)
+
+func TestQuickFix_UseModule(t *testing.T) {
+ testenv.NeedsGo1Point(t, 18) // needs go.work
+
+ const files = `
+-- go.work --
+go 1.20
+
+use (
+ ./a
+)
+-- a/go.mod --
+module mod.com/a
+
+go 1.18
+
+-- a/main.go --
+package main
+
+import "mod.com/a/lib"
+
+func main() {
+ _ = lib.C
+}
+
+-- a/lib/lib.go --
+package lib
+
+const C = "b"
+-- b/go.mod --
+module mod.com/b
+
+go 1.18
+
+-- b/main.go --
+package main
+
+import "mod.com/b/lib"
+
+func main() {
+ _ = lib.C
+}
+
+-- b/lib/lib.go --
+package lib
+
+const C = "b"
+`
+
+ for _, title := range []string{
+ "Use this module",
+ "Use all modules",
+ } {
+ t.Run(title, func(t *testing.T) {
+ Run(t, files, func(t *testing.T, env *Env) {
+ env.OpenFile("b/main.go")
+ var d protocol.PublishDiagnosticsParams
+ env.AfterChange(ReadDiagnostics("b/main.go", &d))
+ fixes := env.GetQuickFixes("b/main.go", d.Diagnostics)
+ var toApply []protocol.CodeAction
+ for _, fix := range fixes {
+ if strings.Contains(fix.Title, title) {
+ toApply = append(toApply, fix)
+ }
+ }
+ if len(toApply) != 1 {
+ t.Fatalf("codeAction: got %d quick fixes matching %q, want 1; got: %v", len(toApply), title, toApply)
+ }
+ env.ApplyCodeAction(toApply[0])
+ env.AfterChange(NoDiagnostics())
+ want := `go 1.20
+
+use (
+ ./a
+ ./b
+)
+`
+ got := env.ReadWorkspaceFile("go.work")
+ if diff := compare.Text(want, got); diff != "" {
+ t.Errorf("unexpeced go.work content:\n%s", diff)
+ }
+ })
+ })
+ }
+}
+
+func TestQuickFix_AddGoWork(t *testing.T) {
+ testenv.NeedsGo1Point(t, 18) // needs go.work
+
+ v := goVersion(t)
+ const files = `
+-- a/go.mod --
+module mod.com/a
+
+go 1.18
+
+-- a/main.go --
+package main
+
+import "mod.com/a/lib"
+
+func main() {
+ _ = lib.C
+}
+
+-- a/lib/lib.go --
+package lib
+
+const C = "b"
+-- b/go.mod --
+module mod.com/b
+
+go 1.18
+
+-- b/main.go --
+package main
+
+import "mod.com/b/lib"
+
+func main() {
+ _ = lib.C
+}
+
+-- b/lib/lib.go --
+package lib
+
+const C = "b"
+`
+
+ tests := []struct {
+ name string
+ file string
+ title string
+ want string
+ }{
+ {
+ "use b",
+ "b/main.go",
+ "Add a go.work file using this module",
+ fmt.Sprintf(`go 1.%d
+
+use ./b
+`, v),
+ },
+ {
+ "use a",
+ "a/main.go",
+ "Add a go.work file using this module",
+ fmt.Sprintf(`go 1.%d
+
+use ./a
+`, v),
+ },
+ {
+ "use all",
+ "a/main.go",
+ "Add a go.work file using all modules",
+ fmt.Sprintf(`go 1.%d
+
+use (
+ ./a
+ ./b
+)
+`, v),
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ Run(t, files, func(t *testing.T, env *Env) {
+ env.OpenFile(test.file)
+ var d protocol.PublishDiagnosticsParams
+ env.AfterChange(ReadDiagnostics(test.file, &d))
+ fixes := env.GetQuickFixes(test.file, d.Diagnostics)
+ var toApply []protocol.CodeAction
+ for _, fix := range fixes {
+ if strings.Contains(fix.Title, test.title) {
+ toApply = append(toApply, fix)
+ }
+ }
+ if len(toApply) != 1 {
+ t.Fatalf("codeAction: got %d quick fixes matching %q, want 1; got: %v", len(toApply), test.title, toApply)
+ }
+ env.ApplyCodeAction(toApply[0])
+ env.AfterChange(
+ NoDiagnostics(ForFile(test.file)),
+ )
+
+ got := env.ReadWorkspaceFile("go.work")
+ if diff := compare.Text(test.want, got); diff != "" {
+ t.Errorf("unexpected go.work content:\n%s", diff)
+ }
+ })
+ })
+ }
+}
+
+func TestQuickFix_UnsavedGoWork(t *testing.T) {
+ testenv.NeedsGo1Point(t, 18) // needs go.work
+
+ const files = `
+-- go.work --
+go 1.21
+
+use (
+ ./a
+)
+-- a/go.mod --
+module mod.com/a
+
+go 1.18
+
+-- a/main.go --
+package main
+
+func main() {}
+-- b/go.mod --
+module mod.com/b
+
+go 1.18
+
+-- b/main.go --
+package main
+
+func main() {}
+`
+
+ for _, title := range []string{
+ "Use this module",
+ "Use all modules",
+ } {
+ t.Run(title, func(t *testing.T) {
+ Run(t, files, func(t *testing.T, env *Env) {
+ env.OpenFile("go.work")
+ env.OpenFile("b/main.go")
+ env.RegexpReplace("go.work", "go 1.21", "go 1.21 // arbitrary comment")
+ var d protocol.PublishDiagnosticsParams
+ env.AfterChange(ReadDiagnostics("b/main.go", &d))
+ fixes := env.GetQuickFixes("b/main.go", d.Diagnostics)
+ var toApply []protocol.CodeAction
+ for _, fix := range fixes {
+ if strings.Contains(fix.Title, title) {
+ toApply = append(toApply, fix)
+ }
+ }
+ if len(toApply) != 1 {
+ t.Fatalf("codeAction: got %d quick fixes matching %q, want 1; got: %v", len(toApply), title, toApply)
+ }
+ fix := toApply[0]
+ err := env.Editor.ApplyCodeAction(env.Ctx, fix)
+ if err == nil {
+ t.Fatalf("codeAction(%q) succeeded unexpectedly", fix.Title)
+ }
+
+ if got := err.Error(); !strings.Contains(got, "must save") {
+ t.Errorf("codeAction(%q) returned error %q, want containing \"must save\"", fix.Title, err)
+ }
+ })
+ })
+ }
+}
+
+func TestQuickFix_GOWORKOff(t *testing.T) {
+ testenv.NeedsGo1Point(t, 18) // needs go.work
+
+ const files = `
+-- go.work --
+go 1.21
+
+use (
+ ./a
+)
+-- a/go.mod --
+module mod.com/a
+
+go 1.18
+
+-- a/main.go --
+package main
+
+func main() {}
+-- b/go.mod --
+module mod.com/b
+
+go 1.18
+
+-- b/main.go --
+package main
+
+func main() {}
+`
+
+ for _, title := range []string{
+ "Use this module",
+ "Use all modules",
+ } {
+ t.Run(title, func(t *testing.T) {
+ WithOptions(
+ EnvVars{"GOWORK": "off"},
+ ).Run(t, files, func(t *testing.T, env *Env) {
+ env.OpenFile("go.work")
+ env.OpenFile("b/main.go")
+ var d protocol.PublishDiagnosticsParams
+ env.AfterChange(ReadDiagnostics("b/main.go", &d))
+ fixes := env.GetQuickFixes("b/main.go", d.Diagnostics)
+ var toApply []protocol.CodeAction
+ for _, fix := range fixes {
+ if strings.Contains(fix.Title, title) {
+ toApply = append(toApply, fix)
+ }
+ }
+ if len(toApply) != 1 {
+ t.Fatalf("codeAction: got %d quick fixes matching %q, want 1; got: %v", len(toApply), title, toApply)
+ }
+ fix := toApply[0]
+ err := env.Editor.ApplyCodeAction(env.Ctx, fix)
+ if err == nil {
+ t.Fatalf("codeAction(%q) succeeded unexpectedly", fix.Title)
+ }
+
+ if got := err.Error(); !strings.Contains(got, "GOWORK=off") {
+ t.Errorf("codeAction(%q) returned error %q, want containing \"GOWORK=off\"", fix.Title, err)
+ }
+ })
+ })
+ }
+}
