diff --git a/internal/lsp/cache/external.go b/internal/lsp/cache/external.go index a75ec078f45..2498bf68429 100644 --- a/internal/lsp/cache/external.go +++ b/internal/lsp/cache/external.go @@ -28,16 +28,16 @@ type nativeFileHandle struct { } func (fs *nativeFileSystem) GetFile(uri span.URI, kind source.FileKind) source.FileHandle { - version := "DOES NOT EXIST" + identifier := "DOES NOT EXIST" if fi, err := os.Stat(uri.Filename()); err == nil { - version = fi.ModTime().String() + identifier = fi.ModTime().String() } return &nativeFileHandle{ fs: fs, identity: source.FileIdentity{ - URI: uri, - Version: version, - Kind: kind, + URI: uri, + Identifier: identifier, + Kind: kind, }, } } diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 2174df4e5ab..e81ee9408f2 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -46,6 +46,7 @@ type overlay struct { uri span.URI data []byte hash string + version float64 kind source.FileKind // sameContentOnDisk is true if a file has been saved on disk, @@ -424,9 +425,10 @@ func (o *overlay) FileSystem() source.FileSystem { func (o *overlay) Identity() source.FileIdentity { return source.FileIdentity{ - URI: o.uri, - Version: o.hash, - Kind: o.kind, + URI: o.uri, + Identifier: o.hash, + Version: o.version, + Kind: o.kind, } } func (o *overlay) Read(ctx context.Context) ([]byte, string, error) { diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 5bd8d10e163..03a10ac363c 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -274,7 +274,7 @@ func (v *view) storeModFileVersions() { func (v *view) fileVersion(filename string, kind source.FileKind) string { uri := span.FileURI(filename) f := v.session.GetFile(uri, kind) - return f.Identity().Version + return f.Identity().Identifier } func (v *view) Shutdown(ctx context.Context) { diff --git a/internal/lsp/cmd/imports.go b/internal/lsp/cmd/imports.go index 19531df41f2..2127e25ab88 100644 --- a/internal/lsp/cmd/imports.go +++ b/internal/lsp/cmd/imports.go @@ -70,15 +70,19 @@ func (t *imports) Run(ctx context.Context, args ...string) error { } var edits []protocol.TextEdit for _, a := range actions { - if a.Title == "Organize Imports" { - edits = (*a.Edit.Changes)[string(uri)] + if a.Title != "Organize Imports" { + continue + } + for _, c := range a.Edit.DocumentChanges { + if c.TextDocument.URI == string(uri) { + edits = append(edits, c.Edits...) + } } } sedits, err := source.FromProtocolEdits(file.mapper, edits) if err != nil { return errors.Errorf("%v: %v", edits, err) } - newContent := diff.ApplyEdits(string(file.mapper.Content), sedits) filename := file.uri.Filename() diff --git a/internal/lsp/cmd/rename.go b/internal/lsp/cmd/rename.go index 47e26beb324..98f7d69c1df 100644 --- a/internal/lsp/cmd/rename.go +++ b/internal/lsp/cmd/rename.go @@ -64,42 +64,39 @@ func (r *rename) Run(ctx context.Context, args ...string) error { if file.err != nil { return file.err } - loc, err := file.mapper.Location(from) if err != nil { return err } - p := protocol.RenameParams{ TextDocument: protocol.TextDocumentIdentifier{URI: loc.URI}, Position: loc.Range.Start, NewName: args[1], } - we, err := conn.Rename(ctx, &p) + edit, err := conn.Rename(ctx, &p) if err != nil { return err } - - // Make output order predictable - var keys []string - for u := range *we.Changes { - keys = append(keys, u) + var orderedURIs []string + edits := map[span.URI][]protocol.TextEdit{} + for _, c := range edit.DocumentChanges { + uri := span.URI(c.TextDocument.URI) + edits[uri] = append(edits[uri], c.Edits...) + orderedURIs = append(orderedURIs, c.TextDocument.URI) } - sort.Strings(keys) - changeCount := len(keys) + sort.Strings(orderedURIs) + changeCount := len(orderedURIs) - for _, u := range keys { - edits := (*we.Changes)[u] - uri := span.NewURI(u) + for _, u := range orderedURIs { + uri := span.URI(u) cmdFile := conn.AddFile(ctx, uri) filename := cmdFile.uri.Filename() // convert LSP-style edits to []diff.TextEdit cuz Spans are handy - renameEdits, err := source.FromProtocolEdits(cmdFile.mapper, edits) + renameEdits, err := source.FromProtocolEdits(cmdFile.mapper, edits[uri]) if err != nil { return errors.Errorf("%v: %v", edits, err) } - newContent := diff.ApplyEdits(string(cmdFile.mapper.Content), renameEdits) switch { @@ -114,7 +111,7 @@ func (r *rename) Run(ctx context.Context, args ...string) error { diffs := diff.ToUnified(filename+".orig", filename, string(cmdFile.mapper.Content), renameEdits) fmt.Print(diffs) default: - if len(keys) > 1 { + if len(orderedURIs) > 1 { fmt.Printf("%s:\n", filepath.Base(filename)) } fmt.Print(string(newContent)) diff --git a/internal/lsp/cmd/suggested_fix.go b/internal/lsp/cmd/suggested_fix.go index 25ee8b62baf..15c6bc9983d 100644 --- a/internal/lsp/cmd/suggested_fix.go +++ b/internal/lsp/cmd/suggested_fix.go @@ -88,8 +88,13 @@ func (s *suggestedfix) Run(ctx context.Context, args ...string) error { } var edits []protocol.TextEdit for _, a := range actions { - if a.IsPreferred || s.All { - edits = (*a.Edit.Changes)[string(uri)] + if !a.IsPreferred && !s.All { + continue + } + for _, c := range a.Edit.DocumentChanges { + if c.TextDocument.URI == string(uri) { + edits = append(edits, c.Edits...) + } } } diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index 0d3966146b6..cf665052287 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -28,12 +28,12 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara } snapshot := view.Snapshot() + fh := snapshot.Handle(ctx, f) // Determine the supported actions for this file kind. - fileKind := snapshot.Handle(ctx, f).Identity().Kind - supportedCodeActions, ok := view.Options().SupportedCodeActions[fileKind] + supportedCodeActions, ok := view.Options().SupportedCodeActions[fh.Identity().Kind] if !ok { - return nil, fmt.Errorf("no supported code actions for %v file kind", fileKind) + return nil, fmt.Errorf("no supported code actions for %v file kind", fh.Identity().Kind) } // The Only field of the context specifies which code actions the client wants. @@ -52,7 +52,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara } var codeActions []protocol.CodeAction - switch fileKind { + switch fh.Identity().Kind { case source.Mod: if !wanted[protocol.SourceOrganizeImports] { return nil, nil @@ -92,9 +92,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara Title: importFixTitle(importFix.Fix), Kind: protocol.QuickFix, Edit: &protocol.WorkspaceEdit{ - Changes: &map[string][]protocol.TextEdit{ - string(uri): importFix.Edits, - }, + DocumentChanges: documentChanges(fh, importFix.Edits), }, Diagnostics: fixDiagnostics, }) @@ -107,9 +105,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara Title: "Organize Imports", Kind: protocol.SourceOrganizeImports, Edit: &protocol.WorkspaceEdit{ - Changes: &map[string][]protocol.TextEdit{ - string(uri): edits, - }, + DocumentChanges: documentChanges(fh, edits), }, }) } @@ -223,19 +219,37 @@ func quickFixes(ctx context.Context, s source.Snapshot, f source.File, diagnosti continue } for _, fix := range srcErr.SuggestedFixes { - edits := make(map[string][]protocol.TextEdit) - for uri, e := range fix.Edits { - edits[protocol.NewURI(uri)] = e - } - codeActions = append(codeActions, protocol.CodeAction{ + action := protocol.CodeAction{ Title: fix.Title, Kind: protocol.QuickFix, Diagnostics: []protocol.Diagnostic{diag}, - Edit: &protocol.WorkspaceEdit{ - Changes: &edits, - }, - }) + Edit: &protocol.WorkspaceEdit{}, + } + for uri, edits := range fix.Edits { + f, err := s.View().GetFile(ctx, uri) + if err != nil { + log.Error(ctx, "no file", err, telemetry.URI.Of(uri)) + continue + } + fh := s.Handle(ctx, f) + action.Edit.DocumentChanges = append(action.Edit.DocumentChanges, documentChanges(fh, edits)...) + } + codeActions = append(codeActions, action) } } return codeActions, nil } + +func documentChanges(fh source.FileHandle, edits []protocol.TextEdit) []protocol.TextDocumentEdit { + return []protocol.TextDocumentEdit{ + { + TextDocument: protocol.VersionedTextDocumentIdentifier{ + Version: fh.Identity().Version, + TextDocumentIdentifier: protocol.TextDocumentIdentifier{ + URI: protocol.NewURI(fh.Identity().URI), + }, + }, + Edits: edits, + }, + } +} diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 8e2615b5702..f20b967de86 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -310,17 +310,14 @@ func (r *runner) Import(t *testing.T, spn span.Span) { if err != nil { t.Fatal(err) } - var edits []protocol.TextEdit - for _, a := range actions { - if a.Title == "Organize Imports" { - edits = (*a.Edit.Changes)[string(uri)] + got := string(m.Content) + if len(actions) > 0 { + res, err := applyWorkspaceEdits(r, actions[0].Edit) + if err != nil { + t.Fatal(err) } + got = res[uri] } - sedits, err := source.FromProtocolEdits(m, edits) - if err != nil { - t.Error(err) - } - got := diff.ApplyEdits(string(m.Content), sedits) if goimported != got { t.Errorf("import failed for %s, expected:\n%v\ngot:\n%v", filename, goimported, got) } @@ -347,25 +344,18 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { Diagnostics: toProtocolDiagnostics(r.ctx, diagnostics[uri]), }, }) - if err != nil { - t.Error(err) - return - } - m, err := r.data.Mapper(f.URI()) if err != nil { t.Fatal(err) } - var edits []protocol.TextEdit - for _, a := range actions { - if a.Title == "Remove" { - edits = (*a.Edit.Changes)[string(uri)] - } + // TODO: This test should probably be able to handle multiple code actions. + if len(actions) > 1 { + t.Fatal("expected only 1 code action") } - sedits, err := source.FromProtocolEdits(m, edits) + res, err := applyWorkspaceEdits(r, actions[0].Edit) if err != nil { - t.Error(err) + t.Fatal(err) } - got := diff.ApplyEdits(string(m.Content), sedits) + got := res[uri] fixed := string(r.data.Golden("suggestedfix", filename, func() ([]byte, error) { return []byte(got), nil })) @@ -563,7 +553,7 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { t.Fatalf("failed for %v: %v", spn, err) } - workspaceEdits, err := r.server.Rename(r.ctx, &protocol.RenameParams{ + wedit, err := r.server.Rename(r.ctx, &protocol.RenameParams{ TextDocument: protocol.TextDocumentIdentifier{ URI: protocol.NewURI(uri), }, @@ -579,33 +569,26 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { } return } - - var res []string - for uri, edits := range *workspaceEdits.Changes { - m, err := r.data.Mapper(span.URI(uri)) - if err != nil { - t.Fatal(err) - } - sedits, err := source.FromProtocolEdits(m, edits) - if err != nil { - t.Error(err) - } - filename := filepath.Base(m.URI.Filename()) - contents := applyEdits(string(m.Content), sedits) - if len(*workspaceEdits.Changes) > 1 { - contents = fmt.Sprintf("%s:\n%s", filename, contents) - } - res = append(res, contents) + res, err := applyWorkspaceEdits(r, wedit) + if err != nil { + t.Fatal(err) } - - // Sort on filename - sort.Strings(res) + var orderedURIs []string + for uri := range res { + orderedURIs = append(orderedURIs, string(uri)) + } + sort.Strings(orderedURIs) var got string - for i, val := range res { + for i := 0; i < len(res); i++ { if i != 0 { got += "\n" } + uri := span.URI(orderedURIs[i]) + if len(res) > 1 { + got += filepath.Base(uri.Filename()) + ":\n" + } + val := res[uri] got += val } @@ -650,6 +633,24 @@ func (r *runner) PrepareRename(t *testing.T, src span.Span, want *source.Prepare } } +func applyWorkspaceEdits(r *runner, wedit *protocol.WorkspaceEdit) (map[span.URI]string, error) { + res := map[span.URI]string{} + for _, docEdits := range wedit.DocumentChanges { + uri := span.URI(docEdits.TextDocument.URI) + m, err := r.data.Mapper(uri) + if err != nil { + return nil, err + } + res[uri] = string(m.Content) + sedits, err := source.FromProtocolEdits(m, docEdits.Edits) + if err != nil { + return nil, err + } + res[uri] = applyEdits(res[uri], sedits) + } + return res, nil +} + func applyEdits(contents string, edits []diff.TextEdit) string { res := contents diff --git a/internal/lsp/rename.go b/internal/lsp/rename.go index 9ce36729a1a..9664b5c449a 100644 --- a/internal/lsp/rename.go +++ b/internal/lsp/rename.go @@ -23,16 +23,22 @@ func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*pr if err != nil { return nil, err } - edits, err := ident.Rename(ctx, view, params.NewName) + edits, err := ident.Rename(ctx, params.NewName) if err != nil { return nil, err } - changes := make(map[string][]protocol.TextEdit) + var docChanges []protocol.TextDocumentEdit for uri, e := range edits { - changes[protocol.NewURI(uri)] = e + f, err := view.GetFile(ctx, uri) + if err != nil { + return nil, err + } + fh := ident.Snapshot.Handle(ctx, f) + docChanges = append(docChanges, documentChanges(fh, e)...) } - - return &protocol.WorkspaceEdit{Changes: &changes}, nil + return &protocol.WorkspaceEdit{ + DocumentChanges: docChanges, + }, nil } func (s *Server) prepareRename(ctx context.Context, params *protocol.PrepareRenameParams) (*protocol.Range, error) { @@ -42,11 +48,15 @@ func (s *Server) prepareRename(ctx context.Context, params *protocol.PrepareRena if err != nil { return nil, err } + ident, err := source.Identifier(ctx, view, f, params.Position) + if err != nil { + return nil, nil // ignore errors + } // Do not return errors here, as it adds clutter. // Returning a nil result means there is not a valid rename. - item, err := source.PrepareRename(ctx, view, f, params.Position) + item, err := ident.PrepareRename(ctx) if err != nil { - return nil, nil + return nil, nil // ignore errors } // TODO(suzmue): return ident.Name as the placeholder text. return &item.Range, nil diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index fe4d38a4386..e85c62319f9 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -109,6 +109,7 @@ type diagnosticSet struct { func diagnostics(ctx context.Context, view View, pkg Package, reports map[span.URI][]Diagnostic) bool { ctx, done := trace.StartSpan(ctx, "source.diagnostics", telemetry.Package.Of(pkg.ID())) + _ = ctx // circumvent SA4006 defer done() diagSets := make(map[span.URI]*diagnosticSet) diff --git a/internal/lsp/source/implementation.go b/internal/lsp/source/implementation.go index 2793333a6a8..f82f646acc3 100644 --- a/internal/lsp/source/implementation.go +++ b/internal/lsp/source/implementation.go @@ -83,7 +83,7 @@ func Implementation(ctx context.Context, view View, f File, position protocol.Po } } if containingFile == nil { - return nil, fmt.Errorf("Failed to find file %q in package %v", file.Name(), pkg.PkgPath()) + return nil, fmt.Errorf("failed to find file %q in package %v", file.Name(), pkg.PkgPath()) } uri := containingFile.Identity().URI diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index d6b50f829a9..1717c66c75d 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -41,15 +41,10 @@ type PrepareItem struct { Text string } -func PrepareRename(ctx context.Context, view View, f File, pos protocol.Position) (*PrepareItem, error) { +func (i *IdentifierInfo) PrepareRename(ctx context.Context) (*PrepareItem, error) { ctx, done := trace.StartSpan(ctx, "source.PrepareRename") defer done() - i, err := Identifier(ctx, view, f, pos) - if err != nil { - return nil, err - } - // TODO(rstambler): We should handle this in a better way. // If the object declaration is nil, assume it is an import spec. if i.Declaration.obj == nil { @@ -77,7 +72,7 @@ func PrepareRename(ctx context.Context, view View, f File, pos protocol.Position } // Rename returns a map of TextEdits for each file modified when renaming a given identifier within a package. -func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string) (map[span.URI][]protocol.TextEdit, error) { +func (i *IdentifierInfo) Rename(ctx context.Context, newName string) (map[span.URI][]protocol.TextEdit, error) { ctx, done := trace.StartSpan(ctx, "source.Rename") defer done() @@ -90,7 +85,7 @@ func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string) if err != nil { return nil, err } - return ident.Rename(ctx, view, newName) + return ident.Rename(ctx, newName) } if i.Name == newName { return nil, errors.Errorf("old and new names are the same: %s", newName) @@ -117,7 +112,7 @@ func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string) r := renamer{ ctx: ctx, - fset: view.Session().Cache().FileSet(), + fset: i.Snapshot.View().Session().Cache().FileSet(), refs: refs, objsToUpdate: make(map[types.Object]bool), from: i.Name, @@ -147,7 +142,7 @@ func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string) for uri, edits := range changes { // These edits should really be associated with FileHandles for maximal correctness. // For now, this is good enough. - f, err := view.GetFile(ctx, uri) + f, err := i.Snapshot.View().GetFile(ctx, uri) if err != nil { return nil, err } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 8e550f6c302..4281635c774 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -663,7 +663,7 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { t.Error(err) return } - changes, err := ident.Rename(r.ctx, r.view, newText) + changes, err := ident.Rename(r.ctx, newText) if err != nil { renamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) { return []byte(err.Error()), nil @@ -747,7 +747,14 @@ func (r *runner) PrepareRename(t *testing.T, src span.Span, want *source.Prepare t.Fatal(err) } // Find the identifier at the position. - item, err := source.PrepareRename(ctx, r.view, f, srcRng.Start) + ident, err := source.Identifier(ctx, r.view, f, srcRng.Start) + if err != nil { + if want.Text != "" { // expected an ident. + t.Errorf("prepare rename failed for %v: got error: %v", src, err) + } + return + } + item, err := ident.PrepareRename(ctx) if err != nil { if want.Text != "" { // expected an ident. t.Errorf("prepare rename failed for %v: got error: %v", src, err) diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 842e5e24461..568151afe7d 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -20,13 +20,21 @@ import ( // FileIdentity uniquely identifies a file at a version from a FileSystem. type FileIdentity struct { - URI span.URI - Version string - Kind FileKind + URI span.URI + + // Version is the version of the file, as specified by the client. + Version float64 + + // Identifier represents a unique identifier for the file. + // It could be a file's modification time or its SHA1 hash if it is not on disk. + Identifier string + + // Kind is the file's kind. + Kind FileKind } func (identity FileIdentity) String() string { - return fmt.Sprintf("%s%s%s", identity.URI, identity.Version, identity.Kind) + return fmt.Sprintf("%s%f%s%s", identity.URI, identity.Version, identity.Identifier, identity.Kind) } // FileHandle represents a handle to a specific version of a single file from