[clangd] Extend the rename API.

[hokein]

Summary

several changes:

• return a structure result in rename API;
• prepareRename now returns more information (main-file occurrences)
• remove the duplicated detecting-touch-identifier code in prepareRename (which is implemented in rename API);

[sammccall]

clang-tools-extra/clangd/ClangdServer.cpp

408

the empty index is weird here - can we pass nullptr?
Currently nullptr leads to an error in the !crossfile case, but I think we can give rename(Index=nullptr, RenameOpts.CrossFile=true) the behavior we want.

(otherwise, if we need an empty index use MemIndex())

411

mention "the index is used to see if the local rename is complete"?

416

we're now returning the "dummy" string to the caller, so we should document it somewhere (or ideally just make it the empty string and document that)

clang-tools-extra/clangd/ClangdServer.h

277

nit: drop "may be incomplete as it"?

clang-tools-extra/clangd/refactor/Rename.cpp

500

nit: I'd find this more readable as a default construction followed by assignments to the fields

507

and again here - declare the whole struct first and fill it in?

clang-tools-extra/clangd/refactor/Rename.h

58

nit: I'd suggest RenameResult singular, consistent with CodeCompleteResult and ReferencesResult

60

Give this a real name... Target?

61

It's slightly awkward to have the half-populated state (may or may not contain cross-file results, can't tell without the query).

I'd consider redundantly including Edit LocalChanges and FileEdits GlobalChanges with the former always set and the latter empty when returned from preparerename.

[hokein]

address review comments.

[hokein]

clang-tools-extra/clangd/ClangdServer.cpp

416

make it empty string, and added document.

clang-tools-extra/clangd/refactor/Rename.h

61

It's slightly awkward to have the half-populated state (may or may not contain cross-file results, can't tell without the query).

I feel this is not too bad, the query is quite trivial, just Edits.size() > 1.

I'd consider redundantly including Edit LocalChanges and FileEdits GlobalChanges with the former always set and the latter empty when returned from preparerename.

This change seems nice to get changes for main-file, but I think GlobalChanges should include LocalChanges (otherwise, client side needs to combine these two when applying the final rename edits)? then we can't leave the later empty while keeping the former set.

Happy to do the change, but it looks like we don't have usage of LocalChanges so far. In prepareRename, we want main-file occurrences, rename will always return them regardless of single-file or cross-file rename, so I think Edits is efficient.

[sammccall]

LG, introducing a separate field for locally affected ranges is up to you.

clang-tools-extra/clangd/ClangdServer.h

277

This seems a little like spelling out too many implementation details, and "equal to invoke" isn't quite right grammatically. Maybe just...

"The returned result describes edits in the current file only (all occurrences of the target name are simply deleted)".

(Note that if we split out the field for file ranges it could just be vector<Range>, avoiding the need to explain the replacement with empty string)

278

nit: resulslt -> result

clang-tools-extra/clangd/refactor/Rename.h

61

It's slightly awkward to have the half-populated state (may or may not contain cross-file results, can't tell without the query).

I feel this is not too bad, the query is quite trivial, just Edits.size() > 1.

This isn't sufficient though: if Edits.size() == 1 then the results may be either complete or incomplete. (Sorry my wording above may have been poor, but this is the distinction I was referring to).

I'd consider redundantly including Edit LocalChanges and FileEdits GlobalChanges with the former always set and the latter empty when returned from preparerename.

This change seems nice to get changes for main-file, but I think GlobalChanges should include LocalChanges (otherwise, client side needs to combine these two when applying the final rename edits)? then we can't leave the later empty while keeping the former set.

Sure we can. // If the full set of changes is unknown, this field is empty.

Happy to do the change, but it looks like we don't have usage of LocalChanges so far. In prepareRename, we want main-file occurrences, rename will always return them regardless of single-file or cross-file rename, so I think Edits is efficient.

Yes, it's possible to implement correct behavior on top of the current API. It effectively reuses the same field with different semantics/contracts, and the client has enough information to know which contract is in place (and throw away the key in the expected singleton map).
However I think this is fragile and harder to understand than providing separate fields for the two contracts - using types to distinguish local vs global results makes the data harder to misuse. Given we expect this to be used by embedders, I think it's worth adding the second field.

[hokein]

address comment: add the LocalChanges field in RenameResult.

clang-tools-extra/clangd/refactor/Rename.h

61

ah, I see your points, agreed. Added the LocalChanges.

[sammccall]

LG, thanks!

clang-tools-extra/clangd/refactor/Rename.h

62

this could just be a vector<Range>, as we don't have a use case for the replacements and it avoids the issue of what to replace with. Up to you though.

[hokein]

change LocalChanges type to vector<Range>

clang-tools-extra/clangd/ClangdLSPServer.cpp

Server->workspaceSymbols(
           Sym.kind = adjustKindToCapability(Sym.kind, SupportedSymbolKinds);
 
         Reply(std::move(*Items));
       });
 }
 
 void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
                                       Callback<llvm::Optional<Range>> Reply) {
-  Server->prepareRename(Params.textDocument.uri.file(), Params.position,
-                        RenameOpts, std::move(Reply));
+  Server->prepareRename(
+      Params.textDocument.uri.file(), Params.position, RenameOpts,
+      [Reply = std::move(Reply)](llvm::Expected<RenameResults> Result) mutable {
+        if (!Result)
+          return Reply(Result.takeError());
+        return Reply(std::move(Result->R));
+      });
 }
 
 void ClangdLSPServer::onRename(const RenameParams &Params,
                                Callback<WorkspaceEdit> Reply) {
   Path File = std::string(Params.textDocument.uri.file());
   if (!DraftMgr.getDraft(File))
     return Reply(llvm::make_error<LSPError>(
         "onRename called for non-added file", ErrorCode::InvalidParams));
   Server->rename(
       File, Params.position, Params.newName, RenameOpts,
       [File, Params, Reply = std::move(Reply),
-       this](llvm::Expected<FileEdits> Edits) mutable {
-        if (!Edits)
-          return Reply(Edits.takeError());
-        if (auto Err = validateEdits(DraftMgr, *Edits))
+       this](llvm::Expected<RenameResults> R) mutable {
+        if (!R)
+          return Reply(R.takeError());
+        if (auto Err = validateEdits(DraftMgr, R->Edits))
           return Reply(std::move(Err));
         WorkspaceEdit Result;
         Result.changes.emplace();
-        for (const auto &Rep : *Edits) {
+        for (const auto &Rep : R->Edits) {
           (*Result.changes)[URI::createFile(Rep.first()).toString()] =
               Rep.second.asTextEdits();
         }
         Reply(Result);
       });
 }
 
 void ClangdLSPServer::onDocumentDidClose(

clang-tools-extra/clangd/ClangdServer.h

void formatFile(PathRef File, StringRef Code,
                   Callback<tooling::Replacements> CB);
 
   /// Run formatting after \p TriggerText was typed at \p Pos in \p File with
   /// content \p Code.
   void formatOnType(PathRef File, StringRef Code, Position Pos,
                     StringRef TriggerText, Callback<std::vector<TextEdit>> CB);
 
   /// Test the validity of a rename operation.
+  ///
+  /// The returned result may be incomplete as it only contains occurrences from
+  /// the current main file.
   void prepareRename(PathRef File, Position Pos,
                      const RenameOptions &RenameOpts,
-                     Callback<llvm::Optional<Range>> CB);
+                     Callback<RenameResults> CB);
 
   /// Rename all occurrences of the symbol at the \p Pos in \p File to
   /// \p NewName.
   /// If WantFormat is false, the final TextEdit will be not formatted,
   /// embedders could use this method to get all occurrences of the symbol (e.g.
   /// highlighting them in prepare stage).
   void rename(PathRef File, Position Pos, llvm::StringRef NewName,
-              const RenameOptions &Opts, Callback<FileEdits> CB);
+              const RenameOptions &Opts, Callback<RenameResults> CB);
 
   struct TweakRef {
     std::string ID;    /// ID to pass for applyTweak.
     std::string Title; /// A single-line message to show in the UI.
     Tweak::Intent Intent;
   };
   /// Enumerate the code tweaks available to the user at a specified point.
   void enumerateTweaks(PathRef File, Range Sel,

clang-tools-extra/clangd/ClangdServer.cpp

for (const tooling::Replacement &R :
       Result.push_back(replacementToEdit(Code, R));
     return CB(Result);
   };
   WorkScheduler.run("FormatOnType", File, std::move(Action));
 }
 
 void ClangdServer::prepareRename(PathRef File, Position Pos,
                                  const RenameOptions &RenameOpts,
-                                 Callback<llvm::Optional<Range>> CB) {
+                                 Callback<RenameResults> CB) {
   auto Action = [Pos, File = File.str(), CB = std::move(CB), RenameOpts,
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
-    auto &AST = InpAST->AST;
-    const auto &SM = AST.getSourceManager();
-    auto Loc = sourceLocationInMainFile(SM, Pos);
-    if (!Loc)
-      return CB(Loc.takeError());
-    const auto *TouchingIdentifier =
-        spelledIdentifierTouching(*Loc, AST.getTokens());
-    if (!TouchingIdentifier)
-      return CB(llvm::None); // no rename on non-identifiers.
-
-    auto Range = halfOpenToRange(
-        SM, CharSourceRange::getCharRange(TouchingIdentifier->location(),
-                                          TouchingIdentifier->endLocation()));
-
-    if (RenameOpts.AllowCrossFile)
-      // FIXME: we now assume cross-file rename always succeeds, revisit this.
-      return CB(Range);
-
-    // Performing the local rename isn't substantially more expensive than
-    // doing an AST-based check, so we just rename and throw away the results.
-    auto Changes = clangd::rename({Pos, "dummy", AST, File, Index, RenameOpts,
-                                   /*GetDirtyBuffer=*/nullptr});
-    if (!Changes) {
+    clangd::FileIndex EmptyIndex;
+    // prepareRename is latency-sensitive:
+    //  - for single-file rename, performing rename isn't substantially more
+    //    expensive than doing an AST-based check;
+    //  - for cross-file rename, we deliberately pass an empty index to save the
+    //    cost, thus the result may be incomplete as it only contains main-file
+    //    occurrences;
+    auto Results = clangd::rename(
+        {Pos, "dummy", InpAST->AST, File,
+         RenameOpts.AllowCrossFile ? &EmptyIndex : Index, RenameOpts});
+    if (!Results) {
       // LSP says to return null on failure, but that will result in a generic
       // failure message. If we send an LSP error response, clients can surface
       // the message to users (VSCode does).
-      return CB(Changes.takeError());
+      return CB(Results.takeError());
     }
-    return CB(Range);
+    return CB(*Results);
   };
   WorkScheduler.runWithAST("PrepareRename", File, std::move(Action));
 }
 
 void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
-                          const RenameOptions &Opts, Callback<FileEdits> CB) {
+                          const RenameOptions &Opts,
+                          Callback<RenameResults> CB) {
   // A snapshot of all file dirty buffers.
   llvm::StringMap<std::string> Snapshot = WorkScheduler.getAllFileContents();
   auto Action = [File = File.str(), NewName = NewName.str(), Pos, Opts,
                  CB = std::move(CB), Snapshot = std::move(Snapshot),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     // Tracks number of files edited per invocation.
     static constexpr trace::Metric RenameFiles("rename_files",
                                                trace::Metric::Distribution);
     if (!InpAST)
       return CB(InpAST.takeError());
     auto GetDirtyBuffer =
         [&Snapshot](PathRef AbsPath) -> llvm::Optional<std::string> {
       auto It = Snapshot.find(AbsPath);
       if (It == Snapshot.end())
         return llvm::None;
       return It->second;
     };
-    auto Edits = clangd::rename(
+    auto R = clangd::rename(
         {Pos, NewName, InpAST->AST, File, Index, Opts, GetDirtyBuffer});
-    if (!Edits)
-      return CB(Edits.takeError());
+    if (!R)
+      return CB(R.takeError());
 
     if (Opts.WantFormat) {
       auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
                                          *InpAST->Inputs.TFS);
       llvm::Error Err = llvm::Error::success();
-      for (auto &E : *Edits)
+      for (auto &E : R->Edits)
         Err =
             llvm::joinErrors(reformatEdit(E.getValue(), Style), std::move(Err));
 
       if (Err)
         return CB(std::move(Err));
     }
-    RenameFiles.record(Edits->size());
-    return CB(std::move(*Edits));
+    RenameFiles.record(R->Edits.size());
+    return CB(*R);
   };
   WorkScheduler.runWithAST("Rename", File, std::move(Action));
 }
 
 // May generate several candidate selections, due to SelectionTree ambiguity.
 // vector of pointers because GCC doesn't like non-copyable Selection.
 static llvm::Expected<std::vector<std::unique_ptr<Tweak::Selection>>>
 tweakSelection(const Range &Sel, const InputsAndAST &AST) {

clang-tools-extra/clangd/refactor/Rename.h

struct RenameInputs {
   RenameOptions Opts = {};
   // When set, used by the rename to get file content for all rename-related
   // files.
   // If there is no corresponding dirty buffer, we will use the file content
   // from disk.
   DirtyBufferGetter GetDirtyBuffer = nullptr;
 };
 
+struct RenameResults {
+  // The range of the symbol that the user can attempt to rename.
+  Range R;
+  FileEdits Edits;
+};
+
 /// Renames all occurrences of the symbol. The result edits are unformatted.
 /// If AllowCrossFile is false, returns an error if rename a symbol that's used
 /// in another file (per the index).
-llvm::Expected<FileEdits> rename(const RenameInputs &RInputs);
+llvm::Expected<RenameResults> rename(const RenameInputs &RInputs);
 
 /// Generates rename edits that replaces all given occurrences with the
 /// NewName.
 /// Exposed for testing only.
 /// REQUIRED: Occurrences is sorted and doesn't have duplicated ranges.
 llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
                                      llvm::StringRef InitialCode,
                                      std::vector<Range> Occurrences,

clang-tools-extra/clangd/refactor/Rename.cpp

if (impliesSimpleEdit(IndexedRest.front().start, LexedRest.front().start)) {
     PartialMatch.pop_back();
   }
   findNearMiss(PartialMatch, IndexedRest, LexedRest.drop_front(),
                LexedIndex + 1, Fuel, MatchedCB);
 }
 
 } // namespace
 
-llvm::Expected<FileEdits> rename(const RenameInputs &RInputs) {
+llvm::Expected<RenameResults> rename(const RenameInputs &RInputs) {
   trace::Span Tracer("Rename flow");
   const auto &Opts = RInputs.Opts;
   ParsedAST &AST = RInputs.AST;
   const SourceManager &SM = AST.getSourceManager();
   llvm::StringRef MainFileCode = SM.getBufferData(SM.getMainFileID());
   auto GetFileContent = [&RInputs,
                          &SM](PathRef AbsPath) -> llvm::Expected<std::string> {
     llvm::Optional<std::string> DirtyBuffer;
auto GetFileContent = [&RInputs,
     return (*Content)->getBuffer().str();
   };
   // Try to find the tokens adjacent to the cursor position.
   auto Loc = sourceLocationInMainFile(SM, RInputs.Pos);
   if (!Loc)
     return Loc.takeError();
   const syntax::Token *IdentifierToken =
       spelledIdentifierTouching(*Loc, AST.getTokens());
+
   // Renames should only triggered on identifiers.
   if (!IdentifierToken)
     return makeError(ReasonToReject::NoSymbolFound);
+  Range CurrentIdentifier = halfOpenToRange(
+      SM, CharSourceRange::getCharRange(IdentifierToken->location(),
+                                        IdentifierToken->endLocation()));
   // FIXME: Renaming macros is not supported yet, the macro-handling code should
   // be moved to rename tooling library.
   if (locateMacroAt(*IdentifierToken, AST.getPreprocessor()))
     return makeError(ReasonToReject::UnsupportedSymbol);
 
   auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
   if (DeclsUnderCursor.empty())
     return makeError(ReasonToReject::NoSymbolFound);
llvm::Expected<RenameResults> rename(const RenameInputs &RInputs) {
   //   - run index-based rename on other affected files;
   auto MainFileRenameEdit = renameWithinFile(AST, RenameDecl, RInputs.NewName);
   if (!MainFileRenameEdit)
     return MainFileRenameEdit.takeError();
 
   // return the main file edit if this is a within-file rename or the symbol
   // being renamed is function local.
   if (!Opts.AllowCrossFile || RenameDecl.getParentFunctionOrMethod()) {
-    return FileEdits(
-        {std::make_pair(RInputs.MainFilePath,
-                        Edit{MainFileCode, std::move(*MainFileRenameEdit)})});
+    return RenameResults{
+        CurrentIdentifier,
+        FileEdits({std::make_pair(
+            RInputs.MainFilePath,
+            Edit{MainFileCode, std::move(*MainFileRenameEdit)})})};
   }
 
-  FileEdits Results;
+  FileEdits Edits;
   // Renameable safely guards us that at this point we are renaming a local
   // symbol if we don't have index.
   if (RInputs.Index) {
     auto OtherFilesEdits = renameOutsideFile(
         RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index,
         Opts.LimitFiles == 0 ? std::numeric_limits<size_t>::max()
                              : Opts.LimitFiles,
         GetFileContent);
     if (!OtherFilesEdits)
       return OtherFilesEdits.takeError();
-    Results = std::move(*OtherFilesEdits);
+    Edits = std::move(*OtherFilesEdits);
   }
   // Attach the rename edits for the main file.
-  Results.try_emplace(RInputs.MainFilePath, MainFileCode,
-                      std::move(*MainFileRenameEdit));
-  return Results;
+  Edits.try_emplace(RInputs.MainFilePath, MainFileCode,
+  return RenameResults{CurrentIdentifier, Edits};
 }
 
 llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
                                      llvm::StringRef InitialCode,
                                      std::vector<Range> Occurrences,
                                      llvm::StringRef NewName) {
   trace::Span Tracer("BuildRenameEdit");
   SPAN_ATTACH(Tracer, "file_path", AbsFilePath);

clang-tools-extra/clangd/test/rename.test

 # CHECK-NEXT:      },
 # CHECK-NEXT:      "start": {
 # CHECK-NEXT:        "character": 4,
 # CHECK-NEXT:        "line": 0
 # CHECK-NEXT:      }
 # CHECK-NEXT:  }
 ---
 {"jsonrpc":"2.0","id":2,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2}}}
-#      CHECK:  "id": 2,
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:  "result": null
+#      CHECK:  "error": {
+# CHECK-NEXT:    "code": -32001,
+# CHECK-NEXT:    "message": "Cannot rename symbol: there is no symbol at the given location"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0"
 ---
 {"jsonrpc":"2.0","id":4,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
 #      CHECK:  "error": {
 # CHECK-NEXT:    "code": -32001,
 # CHECK-NEXT:    "message": "Cannot rename symbol: there is no symbol at the given location"
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "id": 4,
 # CHECK-NEXT:  "jsonrpc": "2.0"

clang-tools-extra/clangd/unittests/RenameTests.cpp

for (llvm::StringRef T : Tests) {
     auto TU = TestTU::withCode(Code.code());
     TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
     auto AST = TU.build();
     llvm::StringRef NewName = "abcde";
     for (const auto &RenamePos : Code.points()) {
       auto RenameResult =
           rename({RenamePos, NewName, AST, testPath(TU.Filename)});
       ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
-      ASSERT_EQ(1u, RenameResult->size());
-      EXPECT_EQ(applyEdits(std::move(*RenameResult)).front().second,
+      ASSERT_EQ(1u, RenameResult->Edits.size());
+      EXPECT_EQ(applyEdits(std::move(RenameResult->Edits)).front().second,
                 expectedResult(Code, NewName));
     }
   }
 }
 
 TEST(RenameTest, Renameable) {
   struct Case {
     const char *Code;
if (!WantRename) {
       assert(Case.ErrorMessage && "Error message must be set!");
       EXPECT_FALSE(Results)
           << "expected rename returned an error: " << T.code();
       auto ActualMessage = llvm::toString(Results.takeError());
       EXPECT_THAT(ActualMessage, testing::HasSubstr(Case.ErrorMessage));
     } else {
       EXPECT_TRUE(bool(Results)) << "rename returned an error: "
                                  << llvm::toString(Results.takeError());
-      ASSERT_EQ(1u, Results->size());
-      EXPECT_EQ(applyEdits(std::move(*Results)).front().second,
+      ASSERT_EQ(1u, Results->Edits.size());
+      EXPECT_EQ(applyEdits(std::move(Results->Edits)).front().second,
                 expectedResult(T, NewName));
     }
   }
 }
 
 TEST(RenameTest, MainFileReferencesOnly) {
   // filter out references not from main file.
   llvm::StringRef Test =
TU.AdditionalFiles["foo.inc"] = R"cpp(
       &foo;
     )cpp";
   auto AST = TU.build();
   llvm::StringRef NewName = "abcde";
 
   auto RenameResult =
       rename({Code.point(), NewName, AST, testPath(TU.Filename)});
   ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << Code.point();
-  ASSERT_EQ(1u, RenameResult->size());
-  EXPECT_EQ(applyEdits(std::move(*RenameResult)).front().second,
+  ASSERT_EQ(1u, RenameResult->Edits.size());
+  EXPECT_EQ(applyEdits(std::move(RenameResult->Edits)).front().second,
             expectedResult(Code, NewName));
 }
 
 TEST(RenameTest, ProtobufSymbolIsExcluded) {
   Annotations Code("Prot^obuf buf;");
   auto TU = TestTU::withCode(Code.code());
   TU.HeaderCode =
       R"cpp(// Generated by the protocol buffer compiler.  DO NOT EDIT!
       class Protobuf {};
       )cpp";
   TU.HeaderFilename = "protobuf.pb.h";
   auto AST = TU.build();
   auto Results = rename({Code.point(), "newName", AST, testPath(TU.Filename)});
   EXPECT_FALSE(Results);
   EXPECT_THAT(llvm::toString(Results.takeError()),
               testing::HasSubstr("not a supported kind"));
 }
 
+TEST(RenameTest, PrepareRename) {
+  Annotations FooH("void func();");
+  Annotations FooCC(R"cpp(
+    #include "foo.h"
+    void [[fu^nc]]() {}
+  )cpp");
+  std::string FooHPath = testPath("foo.h");
+  std::string FooCCPath = testPath("foo.cc");
+  MockFS FS;
+  FS.Files[FooHPath] = std::string(FooH.code());
+  FS.Files[FooCCPath] = std::string(FooCC.code());
+
+  auto ServerOpts = ClangdServer::optsForTest();
+  ServerOpts.BuildDynamicSymbolIndex = true;
+
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, ServerOpts);
+  runAddDocument(Server, FooHPath, FooH.code());
+  runAddDocument(Server, FooCCPath, FooCC.code());
+
+  auto Results =
+      runPrepareRename(Server, FooCCPath, FooCC.point(), {/*CrossFile=*/true});
+  // verify that for multi-file rename, we only return main-file occurrences.
+  ASSERT_TRUE(bool(Results)) << Results.takeError();
+  EXPECT_THAT(applyEdits(std::move(Results->Edits)),
+              UnorderedElementsAre(
+                  Pair(Eq(FooCCPath), Eq(expectedResult(FooCC, "dummy")))));
+
+  // single-file rename on global symbols, we should report an error.
+  Results =
+      runPrepareRename(Server, FooCCPath, FooCC.point(), {/*CrossFile=*/false});
+  EXPECT_FALSE(Results);
+  EXPECT_THAT(llvm::toString(Results.takeError()),
+              testing::HasSubstr("is used outside"));
+}
+
 TEST(CrossFileRenameTests, DirtyBuffer) {
   Annotations FooCode("class [[Foo]] {};");
   std::string FooPath = testPath("foo.cc");
   Annotations FooDirtyBuffer("class [[Foo]] {};\n// this is dirty buffer");
   Annotations BarCode("void [[Bar]]() {}");
   std::string BarPath = testPath("bar.cc");
   // Build the index, the index has "Foo" references from foo.cc and "Bar"
   // references from bar.cc.
auto Results = rename({MainCode.point(),
                          NewName,
                          AST,
                          MainFilePath,
                          Index.get(),
                          {/*CrossFile=*/true},
                          GetDirtyBuffer});
   ASSERT_TRUE(bool(Results)) << Results.takeError();
   EXPECT_THAT(
-      applyEdits(std::move(*Results)),
+      applyEdits(std::move(Results->Edits)),
       UnorderedElementsAre(
           Pair(Eq(FooPath), Eq(expectedResult(FooDirtyBuffer, NewName))),
           Pair(Eq(MainFilePath), Eq(expectedResult(MainCode, NewName)))));
 
   // Run rename on Bar, there is no dirty buffer for the affected file bar.cc,
   // so we should read file content from VFS.
   MainCode = Annotations("void [[Bar]]() { [[B^ar]](); }");
   TU = TestTU::withCode(MainCode.code());
   // Set a file "bar.cc" on disk.
   TU.AdditionalFiles["bar.cc"] = std::string(BarCode.code());
   AST = TU.build();
   Results = rename({MainCode.point(),
                     NewName,
                     AST,
                     MainFilePath,
                     Index.get(),
                     {/*CrossFile=*/true},
                     GetDirtyBuffer});
   ASSERT_TRUE(bool(Results)) << Results.takeError();
   EXPECT_THAT(
-      applyEdits(std::move(*Results)),
+      applyEdits(std::move(Results->Edits)),
       UnorderedElementsAre(
           Pair(Eq(BarPath), Eq(expectedResult(BarCode, NewName))),
           Pair(Eq(MainFilePath), Eq(expectedResult(MainCode, NewName)))));
 
   // Run rename on a pagination index which couldn't return all refs in one
   // request, we reject rename on this case.
   class PaginationIndex : public SymbolIndex {
     bool refs(const RefsRequest &Req,
TEST(CrossFileRenameTests, DeduplicateRefsFromIndex) {
   auto Results = rename({MainCode.point(),
                          NewName,
                          AST,
                          MainFilePath,
                          &DIndex,
                          {/*CrossFile=*/true}});
   ASSERT_TRUE(bool(Results)) << Results.takeError();
   EXPECT_THAT(
-      applyEdits(std::move(*Results)),
+      applyEdits(std::move(Results->Edits)),
       UnorderedElementsAre(
           Pair(Eq(BarPath), Eq(expectedResult(BarCode, NewName))),
           Pair(Eq(MainFilePath), Eq(expectedResult(MainCode, NewName)))));
 }
 
 TEST(CrossFileRenameTests, WithUpToDateIndex) {
   MockCompilationDatabase CDB;
   CDB.ExtraClangFlags = {"-xc++"};
for (const auto &T : Cases) {
 
     llvm::StringRef NewName = "NewName";
     for (const auto &RenamePos : FooH.points()) {
       EXPECT_THAT(Tracer.takeMetric("rename_files"), SizeIs(0));
       auto FileEditsList = llvm::cantFail(runRename(
           Server, FooHPath, RenamePos, NewName, {/*CrossFile=*/true}));
       EXPECT_THAT(Tracer.takeMetric("rename_files"), ElementsAre(2));
       EXPECT_THAT(
-          applyEdits(std::move(FileEditsList)),
+          applyEdits(std::move(FileEditsList.Edits)),
           UnorderedElementsAre(
               Pair(Eq(FooHPath), Eq(expectedResult(T.FooH, NewName))),
               Pair(Eq(FooCCPath), Eq(expectedResult(T.FooCC, NewName)))));
     }
   }
 }
 
 TEST(CrossFileRenameTests, CrossFileOnLocalSymbol) {
   // cross-file rename should work for function-local symbols, even there is no
   // index provided.
   Annotations Code("void f(int [[abc]]) { [[a^bc]] = 3; }");
   auto TU = TestTU::withCode(Code.code());
   auto Path = testPath(TU.Filename);
   auto AST = TU.build();
   llvm::StringRef NewName = "newName";
   auto Results = rename({Code.point(), NewName, AST, Path});
   ASSERT_TRUE(bool(Results)) << Results.takeError();
   EXPECT_THAT(
-      applyEdits(std::move(*Results)),
+      applyEdits(std::move(Results->Edits)),
       UnorderedElementsAre(Pair(Eq(Path), Eq(expectedResult(Code, NewName)))));
 }
 
 TEST(CrossFileRenameTests, BuildRenameEdits) {
   Annotations Code("[[😂]]");
   auto LSPRange = Code.range();
   llvm::StringRef FilePath = "/test/TestTU.cpp";
   llvm::StringRef NewName = "abc";

clang-tools-extra/clangd/unittests/SyncAPI.h

llvm::Expected<SignatureHelp> runSignatureHelp(ClangdServer &Server,
                                                PathRef File, Position Pos);
 
 llvm::Expected<std::vector<LocatedSymbol>>
 runLocateSymbolAt(ClangdServer &Server, PathRef File, Position Pos);
 
 llvm::Expected<std::vector<DocumentHighlight>>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
 
-llvm::Expected<FileEdits> runRename(ClangdServer &Server, PathRef File,
-                                    Position Pos, StringRef NewName,
-                                    const clangd::RenameOptions &RenameOpts);
+llvm::Expected<RenameResults>
+runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName,
+          const clangd::RenameOptions &RenameOpts);
+
+llvm::Expected<RenameResults>
+runPrepareRename(ClangdServer &Server, PathRef File, Position Pos,
 
 llvm::Expected<tooling::Replacements>
 runFormatFile(ClangdServer &Server, PathRef File, StringRef Code);
 
 SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query);
 SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req);
 RefSlab getRefs(const SymbolIndex &Index, SymbolID ID);
 

clang-tools-extra/clangd/unittests/SyncAPI.cpp

 
 llvm::Expected<std::vector<DocumentHighlight>>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos) {
   llvm::Optional<llvm::Expected<std::vector<DocumentHighlight>>> Result;
   Server.findDocumentHighlights(File, Pos, capture(Result));
   return std::move(*Result);
 }
 
-llvm::Expected<FileEdits> runRename(ClangdServer &Server, PathRef File,
+llvm::Expected<RenameResults> runRename(ClangdServer &Server, PathRef File,
                                         Position Pos, llvm::StringRef NewName,
                                         const RenameOptions &RenameOpts) {
-  llvm::Optional<llvm::Expected<FileEdits>> Result;
+  llvm::Optional<llvm::Expected<RenameResults>> Result;
   Server.rename(File, Pos, NewName, RenameOpts, capture(Result));
   return std::move(*Result);
 }
 
+llvm::Expected<RenameResults>
+runPrepareRename(ClangdServer &Server, PathRef File, Position Pos,
+                 const RenameOptions &RenameOpts) {
+  llvm::Optional<llvm::Expected<RenameResults>> Result;
+  Server.prepareRename(File, Pos, RenameOpts, capture(Result));
+  return std::move(*Result);
+}
+
 llvm::Expected<tooling::Replacements>
 runFormatFile(ClangdServer &Server, PathRef File, StringRef Code) {
   llvm::Optional<llvm::Expected<tooling::Replacements>> Result;
   Server.formatFile(File, Code, capture(Result));
   return std::move(*Result);
 }
 
 SymbolSlab runFuzzyFind(const SymbolIndex &Index, llvm::StringRef Query) {