diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 3da975ddef..c95920ba2d 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -297,7 +297,11 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif if err != nil { return nil, err } + forceReloadMetadata := false for _, c := range changes { + if c.Action == source.InvalidateMetadata { + forceReloadMetadata = true + } // Do nothing if the file is open in the editor and we receive // an on-disk action. The editor is the source of truth. if s.isOpen(c.URI) && c.OnDisk { @@ -330,7 +334,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif } var snapshots []source.Snapshot for view, uris := range views { - snapshots = append(snapshots, view.invalidateContent(ctx, uris)) + snapshots = append(snapshots, view.invalidateContent(ctx, uris, forceReloadMetadata)) } return snapshots, nil } @@ -348,8 +352,8 @@ func (s *Session) updateOverlays(ctx context.Context, changes []source.FileModif defer s.overlayMu.Unlock() for _, c := range changes { - // Don't update overlays for on-disk changes. - if c.OnDisk { + // Don't update overlays for on-disk changes or metadata invalidations. + if c.OnDisk || c.Action == source.InvalidateMetadata { continue } diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 72af612b2f..80e3bbc92e 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -664,7 +664,7 @@ func contains(views []*view, view *view) bool { return false } -func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.FileHandle) *snapshot { +func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.FileHandle, forceReloadMetadata bool) *snapshot { s.mu.Lock() defer s.mu.Unlock() @@ -714,7 +714,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Fi // Check if the file's package name or imports have changed, // and if so, invalidate this file's packages' metadata. - invalidateMetadata := s.shouldInvalidateMetadata(ctx, originalFH, currentFH) + invalidateMetadata := forceReloadMetadata || s.shouldInvalidateMetadata(ctx, originalFH, currentFH) // Invalidate the previous modTidyHandle if any of the files have been // saved or if any of the metadata has been invalidated. diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 673b1e72ea..53b17aa223 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -586,7 +586,7 @@ func (v *view) awaitInitialized(ctx context.Context) { // invalidateContent invalidates the content of a Go file, // including any position and type information that depends on it. // It returns true if we were already tracking the given file, false otherwise. -func (v *view) invalidateContent(ctx context.Context, uris map[span.URI]source.FileHandle) source.Snapshot { +func (v *view) invalidateContent(ctx context.Context, uris map[span.URI]source.FileHandle, forceReloadMetadata bool) source.Snapshot { // Detach the context so that content invalidation cannot be canceled. ctx = xcontext.Detach(ctx) @@ -601,7 +601,7 @@ func (v *view) invalidateContent(ctx context.Context, uris map[span.URI]source.F v.snapshotMu.Lock() defer v.snapshotMu.Unlock() - v.snapshot = v.snapshot.clone(ctx, uris) + v.snapshot = v.snapshot.clone(ctx, uris, forceReloadMetadata) return v.snapshot } diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 632779723b..9fa6cc8737 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -49,6 +49,13 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom return nil, err } go s.runGenerate(xcontext.Detach(ctx), dir, recursive) + case source.CommandRegenerateCgo: + mod := source.FileModification{ + URI: protocol.DocumentURI(params.Arguments[0].(string)).SpanURI(), + Action: source.InvalidateMetadata, + } + _, err := s.didModifyFiles(ctx, []source.FileModification{mod}, FromRegenerateCgo) + return nil, err case source.CommandTidy: if len(params.Arguments) == 0 || len(params.Arguments) > 1 { return nil, errors.Errorf("expected one file URI for call to `go mod tidy`, got %v", params.Arguments) diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go index 79be3a8712..f945645f7b 100644 --- a/internal/lsp/fake/editor.go +++ b/internal/lsp/fake/editor.go @@ -22,9 +22,9 @@ import ( type Editor struct { Config EditorConfig - // server, client, and sandbox are concurrency safe and written only + // Server, client, and sandbox are concurrency safe and written only // at construction time, so do not require synchronization. - server protocol.Server + Server protocol.Server client *Client sandbox *Sandbox @@ -81,7 +81,7 @@ func NewEditor(ws *Sandbox, config EditorConfig) *Editor { // It returns the editor, so that it may be called as follows: // editor, err := NewEditor(s).Connect(ctx, conn) func (e *Editor) Connect(ctx context.Context, conn *jsonrpc2.Conn, hooks ClientHooks) (*Editor, error) { - e.server = protocol.ServerDispatcher(conn) + e.Server = protocol.ServerDispatcher(conn) e.client = &Client{editor: e, hooks: hooks} go conn.Run(ctx, protocol.Handlers( @@ -96,8 +96,8 @@ func (e *Editor) Connect(ctx context.Context, conn *jsonrpc2.Conn, hooks ClientH // Shutdown issues the 'shutdown' LSP notification. func (e *Editor) Shutdown(ctx context.Context) error { - if e.server != nil { - if err := e.server.Shutdown(ctx); err != nil { + if e.Server != nil { + if err := e.Server.Shutdown(ctx); err != nil { return fmt.Errorf("Shutdown: %w", err) } } @@ -106,10 +106,10 @@ func (e *Editor) Shutdown(ctx context.Context) error { // Exit issues the 'exit' LSP notification. func (e *Editor) Exit(ctx context.Context) error { - if e.server != nil { + if e.Server != nil { // Not all LSP clients issue the exit RPC, but we do so here to ensure that // we gracefully handle it on multi-session servers. - if err := e.server.Exit(ctx); err != nil { + if err := e.Server.Exit(ctx); err != nil { return fmt.Errorf("Exit: %w", err) } } @@ -158,8 +158,8 @@ func (e *Editor) initialize(ctx context.Context) error { params.Trace = "messages" // TODO: support workspace folders. - if e.server != nil { - resp, err := e.server.Initialize(ctx, params) + if e.Server != nil { + resp, err := e.Server.Initialize(ctx, params) if err != nil { return fmt.Errorf("initialize: %w", err) } @@ -167,7 +167,7 @@ func (e *Editor) initialize(ctx context.Context) error { e.serverCapabilities = resp.Capabilities e.mu.Unlock() - if err := e.server.Initialized(ctx, &protocol.InitializedParams{}); err != nil { + if err := e.Server.Initialized(ctx, &protocol.InitializedParams{}); err != nil { return fmt.Errorf("initialized: %w", err) } } @@ -176,14 +176,14 @@ func (e *Editor) initialize(ctx context.Context) error { } func (e *Editor) onFileChanges(ctx context.Context, evts []FileEvent) { - if e.server == nil { + if e.Server == nil { return } var lspevts []protocol.FileEvent for _, evt := range evts { lspevts = append(lspevts, evt.ProtocolEvent) } - e.server.DidChangeWatchedFiles(ctx, &protocol.DidChangeWatchedFilesParams{ + e.Server.DidChangeWatchedFiles(ctx, &protocol.DidChangeWatchedFilesParams{ Changes: lspevts, }) } @@ -200,8 +200,8 @@ func (e *Editor) OpenFile(ctx context.Context, path string) error { item := textDocumentItem(e.sandbox.Workdir, buf) e.mu.Unlock() - if e.server != nil { - if err := e.server.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ + if e.Server != nil { + if err := e.Server.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ TextDocument: item, }); err != nil { return fmt.Errorf("DidOpen: %w", err) @@ -242,8 +242,8 @@ func (e *Editor) CreateBuffer(ctx context.Context, path, content string) error { item := textDocumentItem(e.sandbox.Workdir, buf) e.mu.Unlock() - if e.server != nil { - if err := e.server.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ + if e.Server != nil { + if err := e.Server.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ TextDocument: item, }); err != nil { return fmt.Errorf("DidOpen: %w", err) @@ -263,8 +263,8 @@ func (e *Editor) CloseBuffer(ctx context.Context, path string) error { delete(e.buffers, path) e.mu.Unlock() - if e.server != nil { - if err := e.server.DidClose(ctx, &protocol.DidCloseTextDocumentParams{ + if e.Server != nil { + if err := e.Server.DidClose(ctx, &protocol.DidCloseTextDocumentParams{ TextDocument: e.textDocumentIdentifier(path), }); err != nil { return fmt.Errorf("DidClose: %w", err) @@ -307,8 +307,8 @@ func (e *Editor) SaveBufferWithoutActions(ctx context.Context, path string) erro e.mu.Unlock() docID := e.textDocumentIdentifier(buf.path) - if e.server != nil { - if err := e.server.WillSave(ctx, &protocol.WillSaveTextDocumentParams{ + if e.Server != nil { + if err := e.Server.WillSave(ctx, &protocol.WillSaveTextDocumentParams{ TextDocument: docID, Reason: protocol.Manual, }); err != nil { @@ -318,7 +318,7 @@ func (e *Editor) SaveBufferWithoutActions(ctx context.Context, path string) erro if err := e.sandbox.Workdir.WriteFile(ctx, path, content); err != nil { return fmt.Errorf("writing %q: %w", path, err) } - if e.server != nil { + if e.Server != nil { params := &protocol.DidSaveTextDocumentParams{ TextDocument: protocol.VersionedTextDocumentIdentifier{ Version: float64(buf.version), @@ -328,7 +328,7 @@ func (e *Editor) SaveBufferWithoutActions(ctx context.Context, path string) erro if includeText { params.Text = &content } - if err := e.server.DidSave(ctx, params); err != nil { + if err := e.Server.DidSave(ctx, params); err != nil { return fmt.Errorf("DidSave: %w", err) } } @@ -496,8 +496,8 @@ func (e *Editor) editBufferLocked(ctx context.Context, path string, edits []Edit }, ContentChanges: evts, } - if e.server != nil { - if err := e.server.DidChange(ctx, params); err != nil { + if e.Server != nil { + if err := e.Server.DidChange(ctx, params); err != nil { return fmt.Errorf("DidChange: %w", err) } } @@ -514,7 +514,7 @@ func (e *Editor) GoToDefinition(ctx context.Context, path string, pos Pos) (stri params.TextDocument.URI = e.sandbox.Workdir.URI(path) params.Position = pos.toProtocolPosition() - resp, err := e.server.Definition(ctx, params) + resp, err := e.Server.Definition(ctx, params) if err != nil { return "", Pos{}, fmt.Errorf("definition: %w", err) } @@ -534,7 +534,7 @@ func (e *Editor) Symbol(ctx context.Context, query string) ([]SymbolInformation, params := &protocol.WorkspaceSymbolParams{} params.Query = query - resp, err := e.server.Symbol(ctx, params) + resp, err := e.Server.Symbol(ctx, params) if err != nil { return nil, fmt.Errorf("symbol: %w", err) } @@ -572,7 +572,7 @@ func (e *Editor) ApplyQuickFixes(ctx context.Context, path string, diagnostics [ } func (e *Editor) codeAction(ctx context.Context, path string, diagnostics []protocol.Diagnostic, only ...protocol.CodeActionKind) error { - if e.server == nil { + if e.Server == nil { return nil } params := &protocol.CodeActionParams{} @@ -581,7 +581,7 @@ func (e *Editor) codeAction(ctx context.Context, path string, diagnostics []prot if diagnostics != nil { params.Context.Diagnostics = diagnostics } - actions, err := e.server.CodeAction(ctx, params) + actions, err := e.Server.CodeAction(ctx, params) if err != nil { return fmt.Errorf("textDocument/codeAction: %w", err) } @@ -623,7 +623,7 @@ func convertEdits(protocolEdits []protocol.TextEdit) []Edit { // FormatBuffer gofmts a Go file. func (e *Editor) FormatBuffer(ctx context.Context, path string) error { - if e.server == nil { + if e.Server == nil { return nil } e.mu.Lock() @@ -631,7 +631,7 @@ func (e *Editor) FormatBuffer(ctx context.Context, path string) error { e.mu.Unlock() params := &protocol.DocumentFormattingParams{} params.TextDocument.URI = e.sandbox.Workdir.URI(path) - resp, err := e.server.Formatting(ctx, params) + resp, err := e.Server.Formatting(ctx, params) if err != nil { return fmt.Errorf("textDocument/formatting: %w", err) } @@ -662,7 +662,7 @@ func (e *Editor) checkBufferPosition(path string, pos Pos) error { // change, so must be followed by a call to Workdir.CheckForFileChanges once // the generate command has completed. func (e *Editor) RunGenerate(ctx context.Context, dir string) error { - if e.server == nil { + if e.Server == nil { return nil } absDir := e.sandbox.Workdir.filePath(dir) @@ -670,7 +670,7 @@ func (e *Editor) RunGenerate(ctx context.Context, dir string) error { Command: "generate", Arguments: []interface{}{absDir, false}, } - if _, err := e.server.ExecuteCommand(ctx, params); err != nil { + if _, err := e.Server.ExecuteCommand(ctx, params); err != nil { return fmt.Errorf("running generate: %v", err) } // Unfortunately we can't simply poll the workdir for file changes here, @@ -682,7 +682,7 @@ func (e *Editor) RunGenerate(ctx context.Context, dir string) error { // CodeLens execute a codelens request on the server. func (e *Editor) CodeLens(ctx context.Context, path string) ([]protocol.CodeLens, error) { - if e.server == nil { + if e.Server == nil { return nil, nil } e.mu.Lock() @@ -694,7 +694,7 @@ func (e *Editor) CodeLens(ctx context.Context, path string) ([]protocol.CodeLens params := &protocol.CodeLensParams{ TextDocument: e.textDocumentIdentifier(path), } - lens, err := e.server.CodeLens(ctx, params) + lens, err := e.Server.CodeLens(ctx, params) if err != nil { return nil, err } diff --git a/internal/lsp/regtest/cgo_test.go b/internal/lsp/regtest/cgo_test.go new file mode 100644 index 0000000000..1761ab648f --- /dev/null +++ b/internal/lsp/regtest/cgo_test.go @@ -0,0 +1,60 @@ +//+build go1.15,cgo + +package regtest + +import ( + "runtime" + "testing" + + "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" +) + +func TestRegenerateCgo(t *testing.T) { + // The android builders have a complex setup which causes this test to fail. See discussion on + // golang.org/cl/214943 for more details. + if runtime.GOOS == "android" { + t.Skip("android not supported") + } + const workspace = ` +-- go.mod -- +module example.com +-- cgo.go -- +package x + +/* +int fortythree() { return 42; } +*/ +import "C" + +func Foo() { + print(C.fortytwo()) +} +` + runner.Run(t, workspace, func(t *testing.T, env *Env) { + // Open the file. We should have a nonexistant symbol. + env.OpenFile("cgo.go") + env.Await(env.DiagnosticAtRegexp("cgo.go", `C\.(fortytwo)`)) // could not determine kind of name for C.fortytwo + + // Fix the C function name. We haven't regenerated cgo, so nothing should be fixed. + env.RegexpReplace("cgo.go", `int fortythree`, "int fortytwo") + env.SaveBuffer("cgo.go") + env.Await(env.DiagnosticAtRegexp("cgo.go", `C\.(fortytwo)`)) + + // Regenerate cgo, fixing the diagnostic. + lenses := env.CodeLens("cgo.go") + var lens protocol.CodeLens + for _, l := range lenses { + if l.Command.Command == source.CommandRegenerateCgo { + lens = l + } + } + if _, err := env.Editor.Server.ExecuteCommand(env.Ctx, &protocol.ExecuteCommandParams{ + Command: lens.Command.Command, + Arguments: lens.Command.Arguments, + }); err != nil { + t.Fatal(err) + } + env.Await(EmptyDiagnostics("cgo.go")) + }) +} diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go index 3d838fa599..98e08f3fd1 100644 --- a/internal/lsp/regtest/runner.go +++ b/internal/lsp/regtest/runner.go @@ -199,7 +199,7 @@ func (r *Runner) Run(t *testing.T, filedata string, test func(t *testing.T, e *E ls.printBuffers(t.Name(), os.Stderr) } if err := env.Editor.Shutdown(ctx); err != nil { - panic(err) + t.Logf("Shutdown: %v", err) } }() test(t, env) diff --git a/internal/lsp/source/code_lens.go b/internal/lsp/source/code_lens.go index 21a1ecc908..f95c171493 100644 --- a/internal/lsp/source/code_lens.go +++ b/internal/lsp/source/code_lens.go @@ -16,6 +16,14 @@ import ( "golang.org/x/tools/internal/lsp/protocol" ) +type lensFunc func(context.Context, Snapshot, FileHandle, *ast.File, *protocol.ColumnMapper) ([]protocol.CodeLens, error) + +var lensFuncs = map[string]lensFunc{ + CommandGenerate: goGenerateCodeLens, + CommandTest: runTestCodeLens, + CommandRegenerateCgo: regenerateCgoLens, +} + // CodeLens computes code lens for Go source code. func CodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.CodeLens, error) { f, _, m, _, err := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseFull).Parse(ctx) @@ -23,25 +31,19 @@ func CodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol return nil, err } - var codeLens []protocol.CodeLens + var result []protocol.CodeLens + for lens, lf := range lensFuncs { + if !snapshot.View().Options().EnabledCodeLens[lens] { + continue + } + added, err := lf(ctx, snapshot, fh, f, m) - if snapshot.View().Options().EnabledCodeLens[CommandGenerate] { - ggcl, err := goGenerateCodeLens(ctx, snapshot, fh, f, m) if err != nil { return nil, err } - codeLens = append(codeLens, ggcl...) + result = append(result, added...) } - - if snapshot.View().Options().EnabledCodeLens[CommandTest] { - rtcl, err := runTestCodeLens(ctx, snapshot, fh, f, m) - if err != nil { - return nil, err - } - codeLens = append(codeLens, rtcl...) - } - - return codeLens, nil + return result, nil } var testMatcher = regexp.MustCompile("^Test[^a-z]") @@ -159,3 +161,30 @@ func goGenerateCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle, f } return nil, nil } + +func regenerateCgoLens(ctx context.Context, snapshot Snapshot, fh FileHandle, f *ast.File, m *protocol.ColumnMapper) ([]protocol.CodeLens, error) { + var c *ast.ImportSpec + for _, imp := range f.Imports { + if imp.Path.Value == `"C"` { + c = imp + } + } + if c == nil { + return nil, nil + } + fset := snapshot.View().Session().Cache().FileSet() + rng, err := newMappedRange(fset, m, c.Pos(), c.EndPos).Range() + if err != nil { + return nil, err + } + return []protocol.CodeLens{ + { + Range: rng, + Command: protocol.Command{ + Title: "regenerate cgo definitions", + Command: CommandRegenerateCgo, + Arguments: []interface{}{fh.Identity().URI}, + }, + }, + }, nil +} diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index c5f1da0e93..d88654d27d 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -61,6 +61,7 @@ const ( CommandTidy = "tidy" // CommandUpgradeDependency is a gopls command to upgrade a dependency. CommandUpgradeDependency = "upgrade.dependency" + CommandRegenerateCgo = "regenerate_cgo" ) // DefaultOptions is the options that are used for Gopls execution independent @@ -90,10 +91,11 @@ func DefaultOptions() Options { Sum: {}, }, SupportedCommands: []string{ - CommandTest, // for "go test" commands - CommandTidy, // for go.mod files - CommandUpgradeDependency, // for go.mod dependency upgrades - CommandGenerate, // for "go generate" commands + CommandTest, + CommandTidy, + CommandUpgradeDependency, + CommandGenerate, + CommandRegenerateCgo, }, }, UserOptions: UserOptions{ @@ -108,6 +110,7 @@ func DefaultOptions() Options { EnabledCodeLens: map[string]bool{ CommandGenerate: true, CommandUpgradeDependency: true, + CommandRegenerateCgo: true, }, }, DebuggingOptions: DebuggingOptions{ diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 24d692d366..f8fb82c52e 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -220,13 +220,14 @@ type FileModification struct { type FileAction int const ( - Open = FileAction(iota) + UnknownFileAction = FileAction(iota) + Open Change Close Save Create Delete - UnknownFileAction + InvalidateMetadata ) // Cache abstracts the core logic of dealing with the environment from the diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index bb79cbdf14..eaee00d423 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -30,6 +30,7 @@ const ( FromDidSave // FromDidClose is a file modification caused by closing a file. FromDidClose + FromRegenerateCgo ) func (m ModificationSource) String() string { @@ -42,6 +43,8 @@ func (m ModificationSource) String() string { return "files changed on disk" case FromDidSave: return "saved files" + case FromRegenerateCgo: + return "regenerate cgo" default: return "unknown file modification" }