diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index be031e91729..8fe677b259b 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -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` diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go index 6f60c3b6b07..521dc1ee63e 100644 --- a/gopls/internal/lsp/cache/load.go +++ b/gopls/internal/lsp/cache/load.go @@ -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. diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go index 17f9bdb7bbd..eaad67c8e06 100644 --- a/gopls/internal/lsp/cache/session.go +++ b/gopls/internal/lsp/cache/session.go @@ -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() @@ -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. @@ -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 { @@ -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) { diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go index 20353647461..45a32a142ff 100644 --- a/gopls/internal/lsp/cache/snapshot.go +++ b/gopls/internal/lsp/cache/snapshot.go @@ -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" @@ -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 @@ -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 { @@ -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 { @@ -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{}{} } @@ -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 @@ -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() @@ -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 @@ -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{}{} @@ -1740,15 +1765,25 @@ 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 } @@ -1756,8 +1791,59 @@ searchOverlays: 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 @@ 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 @@ -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, diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go index 884b0fcda2c..db2c1dc34f0 100644 --- a/gopls/internal/lsp/cache/view.go +++ b/gopls/internal/lsp/cache/view.go @@ -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 @@ -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 } diff --git a/gopls/internal/lsp/code_action.go b/gopls/internal/lsp/code_action.go index 5819565407f..8658ba5588b 100644 --- a/gopls/internal/lsp/code_action.go +++ b/gopls/internal/lsp/code_action.go @@ -176,6 +176,18 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara }, }) } + + 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 6fa831201f4..7236087ddbd 100644 --- a/gopls/internal/lsp/command.go +++ b/gopls/internal/lsp/command.go @@ -22,6 +22,7 @@ import ( "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 @@ type commandConfig struct { 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 @@ func (c *commandHandler) run(ctx context.Context, cfg commandConfig, run command } } 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 @@ func (c *commandHandler) run(ctx context.Context, cfg commandConfig, run command } 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 @@ func (s *Server) runGoModUpdateCommands(ctx context.Context, snapshot source.Sna } 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 @@ func applyFileEdits(ctx context.Context, snapshot source.Snapshot, uri span.URI, if err != nil && !os.IsNotExist(err) { return nil, err } + if bytes.Equal(oldContent, newContent) { return nil, nil } @@ -647,6 +650,31 @@ func applyFileEdits(ctx context.Context, snapshot source.Snapshot, uri span.URI, }}, 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 @@ func collectPackageStats(md []*source.Metadata) command.PackageStats { return stats } + +// RunGoWorkCommand invokes `go work ` 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 a6f9940ad16..8003b17ff86 100644 --- a/gopls/internal/lsp/command/command_gen.go +++ b/gopls/internal/lsp/command/command_gen.go @@ -34,6 +34,7 @@ const ( 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 @@ var Commands = []Command{ RegenerateCgo, RemoveDependency, ResetGoModDiagnostics, + RunGoWorkCommand, RunGovulncheck, RunTests, StartDebugging, @@ -162,6 +164,12 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte 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 @@ func NewResetGoModDiagnosticsCommand(title string, a0 ResetGoModDiagnosticsArgs) }, 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 969ed8ae242..1342e843810 100644 --- a/gopls/internal/lsp/command/interface.go +++ b/gopls/internal/lsp/command/interface.go @@ -170,6 +170,10 @@ type Interface interface { // 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 @@ type PackageStats struct { 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 26f22422607..90c22321c69 100644 --- a/gopls/internal/lsp/diagnostics.go +++ b/gopls/internal/lsp/diagnostics.go @@ -391,8 +391,14 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, analyze // 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 bbe1f1ca5f8..ed3baa20eb6 100644 --- a/gopls/internal/lsp/lsp_test.go +++ b/gopls/internal/lsp/lsp_test.go @@ -241,7 +241,7 @@ func (r *runner) CodeLens(t *testing.T, uri span.URI, want []protocol.CodeLens) 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 ddd05af8c3d..29722c943d4 100644 --- a/gopls/internal/lsp/regtest/marker.go +++ b/gopls/internal/lsp/regtest/marker.go @@ -1484,7 +1484,7 @@ func codeActionMarker(mark marker, actionKind string, start, end protocol.Locati // 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 d6fddc995e7..281772b889a 100644 --- a/gopls/internal/lsp/source/api_json.go +++ b/gopls/internal/lsp/source/api_json.go @@ -768,6 +768,12 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "Reset diagnostics in the go.mod file of a module.", 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.", diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go index 2a16ad60676..e77f9d2dec8 100644 --- a/gopls/internal/lsp/source/view.go +++ b/gopls/internal/lsp/source/view.go @@ -209,7 +209,9 @@ type Snapshot interface { // 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 53cdcacdaf9..818135e94a2 100644 --- a/gopls/internal/lsp/workspace.go +++ b/gopls/internal/lsp/workspace.go @@ -17,7 +17,7 @@ import ( 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 dbd1a954bea..2cb7d2bf81b 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. + +Quick-fixes change files on disk, so are tested by regtests. -TODO(rfindley): actually add quick-fixes here. 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 62f4efcf9c8..35d2e43bf23 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 @@ go 1.21 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 00000000000..5cb08f06480 --- /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) + } + }) + }) + } +}