Skip to content

Commit 97c0daf

Browse files
committed
[clangd] Return a placeholder string for the prepareRename request
This allows us to return the old name of an Objective-C method, which pre-populates the new name field in an editor.
1 parent 6a803cf commit 97c0daf

File tree

10 files changed

+87
-12
lines changed

10 files changed

+87
-12
lines changed

clang-tools-extra/clangd/ClangdLSPServer.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -844,14 +844,15 @@ void ClangdLSPServer::onWorkspaceSymbol(
844844
}
845845

846846
void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
847-
Callback<std::optional<Range>> Reply) {
847+
Callback<PrepareRenameResult> Reply) {
848848
Server->prepareRename(
849849
Params.textDocument.uri.file(), Params.position, /*NewName*/ std::nullopt,
850850
Opts.Rename,
851851
[Reply = std::move(Reply)](llvm::Expected<RenameResult> Result) mutable {
852852
if (!Result)
853853
return Reply(Result.takeError());
854-
return Reply(std::move(Result->Target));
854+
PrepareRenameResult PrepareResult{Result->Target, Result->OldName};
855+
return Reply(std::move(PrepareResult));
855856
});
856857
}
857858

clang-tools-extra/clangd/ClangdLSPServer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
134134
void onWorkspaceSymbol(const WorkspaceSymbolParams &,
135135
Callback<std::vector<SymbolInformation>>);
136136
void onPrepareRename(const TextDocumentPositionParams &,
137-
Callback<std::optional<Range>>);
137+
Callback<PrepareRenameResult>);
138138
void onRename(const RenameParams &, Callback<WorkspaceEdit>);
139139
void onHover(const TextDocumentPositionParams &,
140140
Callback<std::optional<Hover>>);

clang-tools-extra/clangd/ClangdServer.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -578,8 +578,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
578578
// prepareRename is latency-sensitive: we don't query the index, as we
579579
// only need main-file references
580580
auto Results =
581-
clangd::rename({Pos, NewName.value_or("__clangd_rename_placeholder"),
582-
InpAST->AST, File, /*FS=*/nullptr,
581+
clangd::rename({Pos, NewName, InpAST->AST, File, /*FS=*/nullptr,
583582
/*Index=*/nullptr, RenameOpts});
584583
if (!Results) {
585584
// LSP says to return null on failure, but that will result in a generic

clang-tools-extra/clangd/Protocol.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,10 @@ llvm::json::Value toJSON(const DocumentSymbol &S) {
905905
return std::move(Result);
906906
}
907907

908+
llvm::json::Value toJSON(const PrepareRenameResult &R) {
909+
return llvm::json::Object{{"range", R.range}, {"placeholder", R.placeholder}};
910+
}
911+
908912
llvm::json::Value toJSON(const WorkspaceEdit &WE) {
909913
llvm::json::Object Result;
910914
if (WE.changes) {

clang-tools-extra/clangd/Protocol.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,17 @@ struct CodeActionParams {
10041004
};
10051005
bool fromJSON(const llvm::json::Value &, CodeActionParams &, llvm::json::Path);
10061006

1007+
struct PrepareRenameResult {
1008+
/// The range of the string to rename.
1009+
Range range;
1010+
/// A placeholder text of the string content to be renamed.
1011+
///
1012+
/// This is usueful to populate the rename field with an Objective-C selector
1013+
/// name (eg. `performAction:with:`) when renaming Objective-C methods.
1014+
std::string placeholder;
1015+
};
1016+
llvm::json::Value toJSON(const PrepareRenameResult &WE);
1017+
10071018
/// The edit should either provide changes or documentChanges. If the client
10081019
/// can handle versioned document edits and if documentChanges are present,
10091020
/// the latter are preferred over changes.

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -811,7 +811,20 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
811811
if (!Name)
812812
return makeError(ReasonToReject::UnsupportedSymbol);
813813
SymbolName OldSymbolName(Name);
814-
SymbolName NewSymbolName(RInputs.NewName, AST.getLangOpts());
814+
SymbolName NewSymbolName(ArrayRef<StringRef>{});
815+
if (std::optional<StringRef> NewName = RInputs.NewName) {
816+
NewSymbolName = SymbolName(*NewName, AST.getLangOpts());
817+
} else {
818+
// If no new name is given, we are perfoming a pseudo rename for the
819+
// prepareRename request to check if the rename is possible. Construct a
820+
// new symbol name that has as many name pieces as the old name and is thus
821+
// a valid new name.
822+
std::vector<std::string> NewNamePieces;
823+
for (StringRef Piece : OldSymbolName.getNamePieces()) {
824+
NewNamePieces.push_back(Piece.str() + "__clangd_rename_placeholder");
825+
}
826+
NewSymbolName = SymbolName(NewNamePieces);
827+
}
815828
if (OldSymbolName == NewSymbolName)
816829
return makeError(ReasonToReject::SameName);
817830
auto Invalid = checkName(RenameDecl, NewSymbolName);
@@ -858,6 +871,7 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
858871
}
859872
RenameResult Result;
860873
Result.Target = CurrentIdentifier;
874+
Result.OldName = RenameDecl.getNameAsString();
861875
Edit MainFileEdits = Edit(MainFileCode, std::move(*MainFileRenameEdit));
862876
for (const TextEdit &TE : MainFileEdits.asTextEdits())
863877
Result.LocalChanges.push_back(TE.range);

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,12 @@ struct RenameOptions {
3535
};
3636

3737
struct RenameInputs {
38-
Position Pos; // the position triggering the rename
39-
llvm::StringRef NewName;
38+
/// The position triggering the rename
39+
Position Pos;
40+
41+
/// The new name to give to the symbol or `nullopt` to perform a fake rename
42+
/// that checks if rename is possible.
43+
std::optional<llvm::StringRef> NewName;
4044

4145
ParsedAST &AST;
4246
llvm::StringRef MainFilePath;
@@ -52,12 +56,14 @@ struct RenameInputs {
5256
};
5357

5458
struct RenameResult {
55-
// The range of the symbol that the user can attempt to rename.
59+
/// The range of the symbol that the user can attempt to rename.
5660
Range Target;
57-
// Rename occurrences for the current main file.
61+
/// The current name of the declaration at the cursor.
62+
std::string OldName;
63+
/// Rename occurrences for the current main file.
5864
std::vector<Range> LocalChanges;
59-
// Complete edits for the rename, including LocalChanges.
60-
// If the full set of changes is unknown, this field is empty.
65+
/// Complete edits for the rename, including LocalChanges.
66+
/// If the full set of changes is unknown, this field is empty.
6167
FileEdits GlobalChanges;
6268
};
6369

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1476,6 +1476,7 @@ TEST(RenameTest, PrepareRename) {
14761476
/*NewName=*/std::nullopt, {});
14771477
// Verify that for multi-file rename, we only return main-file occurrences.
14781478
ASSERT_TRUE(bool(Results)) << Results.takeError();
1479+
ASSERT_EQ(Results->OldName, "func");
14791480
// We don't know the result is complete in prepareRename (passing a nullptr
14801481
// index internally), so GlobalChanges should be empty.
14811482
EXPECT_TRUE(Results->GlobalChanges.empty());
@@ -1507,6 +1508,39 @@ TEST(RenameTest, PrepareRename) {
15071508
}
15081509
}
15091510

1511+
TEST(RenameTest, PrepareRenameObjC) {
1512+
Annotations Input(R"cpp(
1513+
@interface Foo
1514+
- (int)performA^ction:(int)action w^ith:(int)value;
1515+
@end
1516+
@implementation Foo
1517+
- (int)performA^ction:(int)action w^ith:(int)value {
1518+
return [self ^performAction^:action ^w^ith^:value];
1519+
}
1520+
@end
1521+
)cpp");
1522+
std::string Path = testPath("foo.m");
1523+
MockFS FS;
1524+
FS.Files[Path] = std::string(Input.code());
1525+
1526+
auto ServerOpts = ClangdServer::optsForTest();
1527+
ServerOpts.BuildDynamicSymbolIndex = true;
1528+
1529+
trace::TestTracer Tracer;
1530+
MockCompilationDatabase CDB;
1531+
CDB.ExtraClangFlags = {"-xobjective-c"};
1532+
ClangdServer Server(CDB, FS, ServerOpts);
1533+
runAddDocument(Server, Path, Input.code());
1534+
1535+
for (Position Point : Input.points()) {
1536+
auto Results = runPrepareRename(Server, Path, Point,
1537+
/*NewName=*/std::nullopt, {});
1538+
// Verify that for multi-file rename, we only return main-file occurrences.
1539+
ASSERT_TRUE(bool(Results)) << Results.takeError();
1540+
ASSERT_EQ(Results->OldName, "performAction:with:");
1541+
}
1542+
}
1543+
15101544
TEST(CrossFileRenameTests, DirtyBuffer) {
15111545
Annotations FooCode("class [[Foo]] {};");
15121546
std::string FooPath = testPath("foo.cc");

clang/include/clang/Tooling/Refactoring/Rename/SymbolName.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class SymbolName {
3636
public:
3737
/// Create a new \c SymbolName with the specified pieces.
3838
explicit SymbolName(ArrayRef<StringRef> NamePieces);
39+
explicit SymbolName(ArrayRef<std::string> NamePieces);
3940

4041
explicit SymbolName(const DeclarationName &Name);
4142

clang/lib/Tooling/Refactoring/SymbolName.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ SymbolName::SymbolName(ArrayRef<StringRef> NamePieces) {
4141
this->NamePieces.push_back(Piece.str());
4242
}
4343

44+
SymbolName::SymbolName(ArrayRef<std::string> NamePieces) {
45+
for (const auto &Piece : NamePieces)
46+
this->NamePieces.push_back(Piece);
47+
}
48+
4449
std::optional<std::string> SymbolName::getSinglePiece() const {
4550
if (getNamePieces().size() == 1) {
4651
return NamePieces.front();

0 commit comments

Comments
 (0)